On Sun, Mar 29, 2015 at 02:59:24PM +0000, noname_AT_inventati.org wrote:
> On Sun, Mar 29, 2015 at 03:45:50PM +0200, Silvan Jegen wrote:
> > Heyho!
> >
> > On Wed, Mar 25, 2015 at 07:15:08PM +0000, noname_AT_inventati.org wrote:
> > > There are two issues with buffer_alloc function(s).
> > >
> > > First function is located in buffer.c:
> > > http://repo.or.cz/w/vis.git/blob/HEAD:/buffer.c
> > >
> > > The problem is on line 15:
> > > http://repo.or.cz/w/vis.git/blob/HEAD:/buffer.c#l15
> > >
> > > If realloc fails, buf->data is set to NULL, but original buf->data is
> > > not freed, see realloc(3). The result of realloc should be stored in
> > > separate variable and buffer_free should be called to free buf->data and
> > > set size and len to 0 if realloc fails.
> >
> > This is true. In a memory-overcommitting system like Linux, however,
> > this should not be a problem in practice since realloc should never fail.
>
> Then NULL check is a dead code and should be removed.
I tend to agree that one should commit to either handling out-of-memory
errors correctly in all cases or to just ignore them. The current code
kind-of handles the out-of-memory case but leaks memory.
The original author of the code should decide which approach to take.
Since he started handling the out-of-memory error, sending a patch to
check for a realloc failure and free the memory in that case before
returning *may* be appreciated.
Received on Sun Mar 29 2015 - 17:39:47 CEST