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

Reza Arbab arbab at linux.ibm.com
Wed Jan 4 08:19:18 AEDT 2023


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().

> 	}
>
> 	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:

out_closedir:
	closedir(dir);
	return rc;

> }

-- 
Reza Arbab


More information about the Skiboot mailing list