[Skiboot] [PATCH] opal-prd: Fix missing cleanups in chip_init

[Miaoqian Lin]

On 2023/1/4 5:19, Reza Arbab wrote:
> Hello,
>
> Sorry for the late reply. Your message got lost in the list's "pending moderator approval" queue.
>
> On Mon, Sep 19, 2022 at 12:46:10PM +0400, Miaoqian Lin wrote:
>> diff --git a/external/opal-prd/opal-prd.c b/external/opal-prd/opal-prd.c
>> index 1c610da4c0b6..ed47260d23bd 100644
>> --- a/external/opal-prd/opal-prd.c
>> +++ b/external/opal-prd/opal-prd.c
>> @@ -1290,22 +1290,28 @@ static int chip_init(void)
>>                   dirent->d_name);
>>         if (rc < 0) {
>>             pr_log(LOG_ERR, "FW: Failed to create chip-id path");
>> +            closedir(dir);
>>             return -1;
>>         }
>>
>>         rc = open_and_read(path, &buf, &len);
>>         if (rc) {
>>             pr_log(LOG_ERR, "FW; Failed to read chipid");
>> +            closedir(dir);
>> +            free(path);
>>             return -1;
>>         }
>>         chipid = buf;
>>         chips[nr_chips++] = be32toh(*chipid);
>> +        free(path);
>
> Doesn't chipid/buf also need to be freed here? It is allocated by open_and_read().
>
The memory allocated by open_and_read() is stored as chipid/buf. It is stored in 'chips' array,

which is a global variable and used in other places. I don't think we need to release it here.

>>     }
>>
>>     pr_log(LOG_DEBUG, "FW: Chip init");
>>     for (i = 0; i < nr_chips; i++)
>>         pr_log(LOG_DEBUG, "FW: Chip 0x%lx", chips[i]);
>>
>> +    closedir(dir);
>> +
>>     return 0;
>
> I think it would be more readable to replace the other closedir()+return pairs with 'goto' and a jump label here at the end:
>
Thanks for your review, I'll send v2 to fix this.
> out_closedir:
>     closedir(dir);
>     return rc;
>
>> }
>