[programs] fix the memory leak #368
Conversation
[programs/util.h:359]: (error) Common realloc mistake: 'buf' nulled but not freed upon failure Found by https://github.com/bryongloden/cppcheck
| buf = (char*)realloc(buf, newListSize); | ||
| bufend = buf + newListSize; | ||
| if (!buf) return NULL; | ||
| buftmp = (char*)realloc(buf, newListSize); |
There was a problem hiding this comment.
prefer declaring variables in narrowest scope.
buftmp should be declared here, instead of line 347.
It can be made const too.
It also does not need to be char* since it's not accessed anyway.
So I would lean for :
void* const buftmp = realloc(buf, newListSize);
and corresponding cast at line 361
| if (!buf) return NULL; | ||
| } else { | ||
| free(buf); | ||
| /* And handle error */ |
There was a problem hiding this comment.
Which error should be handled here ?
return NULL; ? ( ping @inikep)
The patch seems incomplete here.
| if (buftmp != NULL) { | ||
| buf = buftmp; | ||
| bufend = buf + newListSize; | ||
| if (!buf) return NULL; |
There was a problem hiding this comment.
if buf = buftmp and buftmp != NULL, then !buf is never true.
This test should be removed
|
Good catch @bryongloden ! |
ghost
added
the
|
Yes, a good catch. I forgot about |
|
The path is implemented at inikep@6173931 |
|
Fixed by #370 |
Passing one pointer into
realloc()and assigning the result directly into that same pointer variable can cause a memory leak if the reallocation fails, because the original allocation will still exist. The correct way to do this is to use a temporary pointer variable.