| Message ID | 20240825185311.109835-1-krzysztof.kozlowski@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [1/3] net: hisilicon: hip04: fix OF node leak in probe() | expand |
> Driver is leaking OF node reference from > of_parse_phandle_with_fixed_args() in probe(). * Is there a need to improve such a change description another bit? + Imperative mood * Tags like “Fixes” and “Cc” * Can a corresponding cover letter help? Regards, Markus
On Sun, Aug 25, 2024 at 09:21:31PM +0200, Markus Elfring wrote: > > Driver is leaking OF node reference from > > of_parse_phandle_with_fixed_args() in probe(). > > * Is there a need to improve such a change description another bit? > > + Imperative mood > > * Tags like “Fixes” and “Cc” I think it would be helpful if these were either explicitly targeted for net-next without Fixes tags (the assumed state of affairs as-is). Subject: [Patch x/n net-next] ... Or targeted at net, with Fixes tags Subject: [Patch x/n net] ... I guess that in theory these are fixes, as resource leaks could occur. But perhaps that is more theory that practice. I am unsure. > * Can a corresponding cover letter help? I agree that would be nice, it's the usual practice for Networking patchsets with more than one patch. The above aside, I looked over the code changes and I agree they are correct.
On 27/08/2024 16:40, Simon Horman wrote: > On Sun, Aug 25, 2024 at 09:21:31PM +0200, Markus Elfring wrote: >>> Driver is leaking OF node reference from >>> of_parse_phandle_with_fixed_args() in probe(). >> >> * Is there a need to improve such a change description another bit? >> >> + Imperative mood >> >> * Tags like “Fixes” and “Cc” > > I think it would be helpful if these were either explicitly targeted for > net-next without Fixes tags (the assumed state of affairs as-is). > > Subject: [Patch x/n net-next] ... > > Or targeted at net, with Fixes tags > > Subject: [Patch x/n net] ... > > I guess that in theory these are fixes, as resource leaks could occur. > But perhaps that is more theory that practice. I am unsure. I'll resend with net-next. Best regards, Krzysztof
On Tue, Aug 27, 2024 at 04:41:50PM +0200, Krzysztof Kozlowski wrote: > On 27/08/2024 16:40, Simon Horman wrote: > > On Sun, Aug 25, 2024 at 09:21:31PM +0200, Markus Elfring wrote: > >>> Driver is leaking OF node reference from > >>> of_parse_phandle_with_fixed_args() in probe(). > >> > >> * Is there a need to improve such a change description another bit? > >> > >> + Imperative mood > >> > >> * Tags like “Fixes” and “Cc” > > > > I think it would be helpful if these were either explicitly targeted for > > net-next without Fixes tags (the assumed state of affairs as-is). > > > > Subject: [Patch x/n net-next] ... > > > > Or targeted at net, with Fixes tags > > > > Subject: [Patch x/n net] ... > > > > I guess that in theory these are fixes, as resource leaks could occur. > > But perhaps that is more theory that practice. I am unsure. > > I'll resend with net-next. Thanks, much appreciated.
diff --git a/drivers/net/ethernet/hisilicon/hip04_eth.c b/drivers/net/ethernet/hisilicon/hip04_eth.c index b91e7a06b97f..beb815e5289b 100644 --- a/drivers/net/ethernet/hisilicon/hip04_eth.c +++ b/drivers/net/ethernet/hisilicon/hip04_eth.c @@ -947,6 +947,7 @@ static int hip04_mac_probe(struct platform_device *pdev) priv->tx_coalesce_timer.function = tx_done; priv->map = syscon_node_to_regmap(arg.np); + of_node_put(arg.np); if (IS_ERR(priv->map)) { dev_warn(d, "no syscon hisilicon,hip04-ppe\n"); ret = PTR_ERR(priv->map);
Driver is leaking OF node reference from of_parse_phandle_with_fixed_args() in probe(). Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> --- drivers/net/ethernet/hisilicon/hip04_eth.c | 1 + 1 file changed, 1 insertion(+)