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

Reza Arbab arbab at linux.ibm.com
Thu Jan 5 03:47:12 AEDT 2023


On Thu, Jan 05, 2023 at 12:08:45AM +0800, Miaoqian Lin wrote:
>On 2023/1/4 5:19, Reza Arbab wrote:
>> 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.

Unless I'm misunderstanding, chipid/buf is dereferenced, so the value it 
points to is copied by assignment to an element in the 'chips' array.  
After that, the memory is no longer needed.

-- 
Reza Arbab


More information about the Skiboot mailing list