Thomas Huth <th...@redhat.com> writes:
> On 02/10/2020 07.05, Markus Armbruster wrote:
>> Elena Afanasova <eafanas...@gmail.com> writes:
>>
>>> Signed-off-by: Elena Afanasova <eafanas...@gmail.com>
>>> ---
>>> bsd-user/elfload.c | 92 +++++++++++++++-------------------------------
>>> 1 file changed, 30 insertions(+), 62 deletions(-)
>>>
>>> diff --git a/bsd-user/elfload.c b/bsd-user/elfload.c
>>> index 32378af7b2..e10ca54eb7 100644
>>> --- a/bsd-user/elfload.c
>>> +++ b/bsd-user/elfload.c
>>> @@ -867,18 +867,14 @@ static abi_ulong load_elf_interp(struct elfhdr *
>>> interp_elf_ex,
>>> if (sizeof(struct elf_phdr) * interp_elf_ex->e_phnum >
>>> TARGET_PAGE_SIZE)
>>> return ~(abi_ulong)0UL;
>>>
>>> - elf_phdata = (struct elf_phdr *)
>>> - malloc(sizeof(struct elf_phdr) * interp_elf_ex->e_phnum);
>>> -
>>> - if (!elf_phdata)
>>> - return ~((abi_ulong)0UL);
>>> + elf_phdata = g_new(struct elf_phdr, interp_elf_ex->e_phnum);
>>>
>>> /*
>>> * If the size of this structure has changed, then punt, since
>>> * we will be doing the wrong thing.
>>> */
>>> if (interp_elf_ex->e_phentsize != sizeof(struct elf_phdr)) {
>>> - free(elf_phdata);
>>> + g_free(elf_phdata);
>>> return ~((abi_ulong)0UL);
>>> }
>>>
>>> @@ -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.
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().
