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
signature.asc
Description: OpenPGP digital signature
