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 <[email protected]>
>> Signed-off-by: Maninder Singh <[email protected]>
>> ---
> 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 <[email protected]>
> +
> + * argv.c (expandargv): Fix memory leak for buffer allocated to read
> file contents.
> +
> 2021-02-01 Martin Sebor <[email protected]>
>
> * 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
>
