[U-Boot] [PATCH 4/8] fdtgrep: Fix logic of free() in do_fdtgrep()

[Simon Glass]

Hi Heinrich,

On 9 June 2018 at 13:44, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> On 06/09/2018 08:22 PM, Simon Glass wrote:
>> This loop never actually exits, but the way the code is written this is
>> not obvious. Add an explicit error check.
>>
>> Reported-by: Coverity (CID: 131280)
>>
>> Signed-off-by: Simon Glass <sjg at chromium.org>
>> ---
>>
>>  tools/fdtgrep.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/fdtgrep.c b/tools/fdtgrep.c
>> index f2b8b71ed7..c4563e2289 100644
>> --- a/tools/fdtgrep.c
>> +++ b/tools/fdtgrep.c
>> @@ -801,7 +801,7 @@ static int do_fdtgrep(struct display_info *disp, const char *filename)
>>        * The first pass will count the regions, but if it is too many,
>>        * we do another pass to actually record them.
>>        */
>> -     for (i = 0; i < 3; i++) {
>> +     for (i = 0; i < 2; i++) {
>>               region = malloc(count * sizeof(struct fdt_region));
>>               if (!region) {
>>                       fprintf(stderr, "Out of memory for %d regions\n",
>
> Can't we call fdtgrep_find_regions() with max_regions = 0 and region =
> NULL to do the counting and get rid of the loop? That may be a bit
> slower but the code will be much easier to read.

Yes I think that would be possible. It would require a change to
fdtgrep_first_region since it assumes there is space for its region.

It seems like a good idea to me, but is beyond the scope of this coverity fix.

Regards,
Simon