On Sun, Mar 29, 2015 at 02:59:24PM +0000, non...@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, non...@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.
