Re: [PATCH] elfload: use g_new instead of malloc

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1441 bytes --]

On 10/2/20 3:58 AM, Markus Armbruster wrote:

>>>> @@ -890,9 +886,8 @@ static abi_ulong load_elf_interp(struct elfhdr * interp_elf_ex,
>>>>          }
>>>>          if (retval < 0) {
>>>>                  perror("load_elf_interp");
>>>> +                g_free(elf_phdata);
>>>>                  exit(-1);
>>>> -                free (elf_phdata);
>>>> -                return retval;
>>>
>>> Deleting return looks wrong.
>>
>> Why? There is an exit(-1) right in front of it, so this is dead code...
>> well, maybe that should be done in a separate patch, or at least
>> mentioned in the patch description, though.
> 
> You're right; I missed the exit(-1).
> 
> Following the unpleasant odour spread by exit(-1)...  Aha, the
> function's behavior on error is inconsistent: sometimes it returns zero,
> sometimes it exits.

Eradicating exit(-1) (which is indistinguishable from exit(255), and
generally not what you want, unless your program is designed to
specifically invoke the immediate-exit semantics of xargs) is also a
worthwhile cleanup project.  But I agree with the advice for separate
patches for separate bugs.

> 
> Since the problem is bigger than just one dead return, I recommend
> leaving it to a separate patch, and keeping this one focused on g_new().
> 
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]