linux-kernel - Re: [PATCH v2 04/20] pinctrl: starfive: Use scope based of_node

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <96beb071-ec2c-475f-8fa2-d519a1b1dc28@web.de>
Date: Mon, 27 May 2024 16:25:18 +0200
From: Markus Elfring <Markus.Elfring@....de>
To: Peng Fan <peng.fan@....com>, soc@...nel.org,
 Alexandre Belloni <alexandre.belloni@...tlin.com>,
 Alexandre Torgue <alexandre.torgue@...s.st.com>,
 Alim Akhtar <alim.akhtar@...sung.com>,
 Andrew Jeffery <andrew@...econstruct.com.au>,
 Angelo Gioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
 Baolin Wang <baolin.wang@...ux.alibaba.com>,
 Chester Lin <chester62515@...il.com>, Chunyan Zhang <zhang.lyra@...il.com>,
 Claudiu Beznea <claudiu.beznea@...on.dev>,
 Damien Le Moal <dlemoal@...nel.org>, Dan Carpenter
 <dan.carpenter@...aro.org>, Dong Aisheng <aisheng.dong@....com>,
 Dvorkin Dmitry <dvorkin@...bo.com>, Emil Renner Berthing <kernel@...il.dk>,
 Fabio Estevam <festevam@...il.com>,
 Geert Uytterhoeven <geert+renesas@...der.be>,
 Ghennadi Procopciuc <ghennadi.procopciuc@....nxp.com>,
 Hal Feng <hal.feng@...rfivetech.com>, Heiko Stübner
 <heiko@...ech.de>, Jacky Bai <ping.bai@....com>,
 Jianlong Huang <jianlong.huang@...rfivetech.com>,
 Joel Stanley <joel@....id.au>, Jonathan Hunter <jonathanh@...dia.com>,
 Krzysztof Kozlowski <krzk@...nel.org>,
 Linus Walleij <linus.walleij@...aro.org>,
 Ludovic Desroches <ludovic.desroches@...rochip.com>,
 Matthias Brugger <matthias.bgg@...il.com>,
 Matthias Brugger <mbrugger@...e.com>,
 Maxime Coquelin <mcoquelin.stm32@...il.com>,
 Nicolas Ferre <nicolas.ferre@...rochip.com>, Orson Zhai
 <orsonzhai@...il.com>, Patrice Chotard <patrice.chotard@...s.st.com>,
 Sascha Hauer <s.hauer@...gutronix.de>, Sean Wang <sean.wang@...nel.org>,
 Shawn Guo <shawnguo@...nel.org>,
 Shiraz Hashim <shiraz.linux.kernel@...il.com>,
 Stephen Warren <swarren@...dotorg.org>,
 Sylwester Nawrocki <s.nawrocki@...sung.com>,
 Thierry Reding <thierry.reding@...il.com>, Tony Lindgren <tony@...mide.com>,
 Viresh Kumar <vireshk@...nel.org>, Wells Lu <wellslutw@...il.com>
Cc: LKML <linux-kernel@...r.kernel.org>, linux-gpio@...r.kernel.org,
 linux-tegra@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-stm32@...md-mailman.stormreply.com, linux-samsung-soc@...r.kernel.org,
 linux-renesas-soc@...r.kernel.org, linux-rockchip@...ts.infradead.org,
 linux-riscv@...ts.infradead.org, linux-mediatek@...ts.infradead.org,
 imx@...ts.linux.dev, linux-aspeed@...ts.ozlabs.org,
 openbmc@...ts.ozlabs.org, kernel@...gutronix.de,
 Peng Fan <peng.fan@....nxp.com>,
 Emil Renner Berthing <emil.renner.berthing@...onical.com>
Subject: Re: [PATCH v2 04/20] pinctrl: starfive: Use scope based of_node_put()
 cleanups

> Use scope based of_node_put() cleanup to simplify code.

I see opportunities to improve affected function implementations another bit.


…
> +++ b/drivers/pinctrl/starfive/pinctrl-starfive-jh7100.c
…
> @@ -543,18 +540,18 @@ static int starfive_dt_node_to_map(struct pinctrl_dev *pctldev,
>  			pins = devm_kcalloc(dev, npins, sizeof(*pins), GFP_KERNEL);
>  			if (!pins) {
>  				ret = -ENOMEM;
> -				goto put_child;
> +				goto free_map;
>  			}
>
>  			pinmux = devm_kcalloc(dev, npins, sizeof(*pinmux), GFP_KERNEL);
>  			if (!pinmux) {
>  				ret = -ENOMEM;
> -				goto put_child;
> +				goto free_map;
>  			}
…
> @@ -623,8 +620,6 @@ static int starfive_dt_node_to_map(struct pinctrl_dev *pctldev,
>  	mutex_unlock(&sfp->mutex);
>  	return 0;
>
> -put_child:
> -	of_node_put(child);
>  free_map:
>  	pinctrl_utils_free_map(pctldev, map, nmaps);
>  	mutex_unlock(&sfp->mutex);
…
> +++ b/drivers/pinctrl/starfive/pinctrl-starfive-jh7110.c
…
> @@ -175,18 +175,18 @@ static int jh7110_dt_node_to_map(struct pinctrl_dev *pctldev,
>  		pins = devm_kcalloc(dev, npins, sizeof(*pins), GFP_KERNEL);
>  		if (!pins) {
>  			ret = -ENOMEM;
> -			goto put_child;
> +			goto free_map;
>  		}
>
>  		pinmux = devm_kcalloc(dev, npins, sizeof(*pinmux), GFP_KERNEL);
>  		if (!pinmux) {
>  			ret = -ENOMEM;
> -			goto put_child;
> +			goto free_map;
>  		}
…
> @@ -233,8 +233,6 @@ static int jh7110_dt_node_to_map(struct pinctrl_dev *pctldev,
>  	*num_maps = nmaps;
>  	return 0;
>
> -put_child:
> -	of_node_put(child);
>  free_map:
>  	pinctrl_utils_free_map(pctldev, map, nmaps);
>  	mutex_unlock(&sfp->mutex);


1. Exception handling is repeated a few times also according to memory allocation failures.
   How do you think about to use a corresponding label like “e_nomem”
   so that another bit of duplicate source code can be avoided?
   https://wiki.sei.cmu.edu/confluence/display/c/MEM12-C.+Consider+using+a+goto+chain+when+leaving+a+function+on+error+when+using+and+releasing+resources

2. Will development interests grow for the usage of a statement like “guard(mutex)(&sfp->mutex);”?


Regards,
Markus