Hi Martin, >> Dynamic memory referenced by 'buffer' was allocated using xmalloc but fails >> to free it >> when jump to 'error' label. >> >> Issue as per static analysis tool. > >The change looks okay to me although I can't approve it. Since GCC >is a regression fixing stage, unless the leak is a recent regression >the fix is (strictly speaking) out of scope for GCC 11. Either >a libiberty or a global maintainer might be comfortable approving it >regardless.
OK >That said, rather than adding another call to free, I would suggest >to consider initializing buffer to null and moving the existing call >to free the buffer under the error: label. > We thought same, but then it will add un-necessary call to free in case of earlier 3 goto cases and thus impact code's readability (going for free without allocating). if (fseek (f, 0L, SEEK_END) == -1) goto error; pos = ftell (f); if (pos == -1) goto error; if (fseek (f, 0L, SEEK_SET) == -1) goto error; thats why we tried with current change. Other option was to create one more label in case of free. Thanks, Maninder Singh >Martin > >> >> Signed-off-by: Ayush Mittal <ayus...@samsung.com> >> Signed-off-by: Maninder Singh <maninder...@samsung.com> >> --- > libiberty/ChangeLog | 4 ++++ > libiberty/argv.c | 5 ++++- > 2 files changed, 8 insertions(+), 1 deletion(-) > > diff --git a/libiberty/ChangeLog b/libiberty/ChangeLog > index e472553..96cacba 100644 > --- a/libiberty/ChangeLog > +++ b/libiberty/ChangeLog > @@ -1,3 +1,7 @@ > +2021-02-18 Ayush Mittal <ayus...@samsung.com> > + > + * argv.c (expandargv): Fix memory leak for buffer allocated to read > file contents. > + > 2021-02-01 Martin Sebor <mse...@redhat.com> > > * dyn-string.c (dyn_string_insert_cstr): Use memcpy instead of > strncpy > diff --git a/libiberty/argv.c b/libiberty/argv.c > index cd97f90..7199c7d 100644 > --- a/libiberty/argv.c > +++ b/libiberty/argv.c > @@ -442,7 +442,10 @@ expandargv (int *argcp, char ***argvp) > due to CR/LF->CR translation when reading text files. > That does not in-and-of itself indicate failure. */ > && ferror (f)) > - goto error; > + { > + free(buffer); > + goto error; > + } > /* Add a NUL terminator. */ > buffer[len] = '\0'; > /* If the file is empty or contains only whitespace, buildargv would >