[U-Boot] [PATCH 5/8] fdtgrep: Separate out checking of two allocations

[Simon Glass]

Hi Heinrich,

On 9 June 2018 at 13:46, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> On 06/09/2018 08:22 PM, Simon Glass wrote:
>> The current code might succeed on the first allocation and fail on the
>> second. Separate the checks to avoid this problem.
>>
>> Of course, free() will never fail and the chances that (when allocating
>> two small areas) one will succeed and one will fail are just as remote.
>> But this keeps coverity happy.
>>
>> Reported-by: Coverity (CID: 131226)
>>
>> Signed-off-by: Simon Glass <sjg at chromium.org>
>> ---
>>
>>  tools/fdtgrep.c | 11 +++++++----
>>  1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/fdtgrep.c b/tools/fdtgrep.c
>> index c4563e2289..5593b42203 100644
>> --- a/tools/fdtgrep.c
>> +++ b/tools/fdtgrep.c
>> @@ -133,11 +133,11 @@ static int value_add(struct display_info *disp, struct value_node **headp,
>>       }
>>
>>       str = strdup(str);
>> +     if (!str)
>> +             goto err_mem;
>>       node = malloc(sizeof(*node));
>> -     if (!str || !node) {
>> -             fprintf(stderr, "Out of memory\n");
>> -             return -1;
>> -     }
>> +     if (!node)
>> +             goto err_mem;
>>       node->next = *headp;
>>       node->type = type;
>>       node->include = include;
>> @@ -145,6 +145,9 @@ static int value_add(struct display_info *disp, struct value_node **headp,
>>       *headp = node;
>>
>
> free(str) is missing here.

If we free the string then the node will be invalid. That is the point
of the strdup(), so that the node can have a copy of the string.

>
>
>>       return 0;
>> +err_mem:
>
> free(str) is missing here.

But the strdup() failed, so str is NULL and there is nothing to free.

>
> Best regards
>
> Heinrich
>
>> +     fprintf(stderr, "Out of memory\n");
>> +     return -1;
>>  }
>>
>>  static bool util_is_printable_string(const void *data, int len)
>>
>

Regards,
Simon