| Message ID | 4a82fe70-b07c-4878-bd31-6ae07b61f522@web.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | clk: at91: pmc: Use common error handling code in pmc_register_ops() | expand |
On 17/09/2024 at 14:34, Markus Elfring wrote: > From: Markus Elfring <elfring@users.sourceforge.net> > Date: Tue, 17 Sep 2024 14:28:22 +0200 > > Add a jump target so that a bit of exception handling can be better reused > at the end of this function implementation. > > This issue was detected by using the Coccinelle software. > > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net> Bad track record and no real benefit from the patch: NACK, sorry. Regards, Nicolas > --- > drivers/clk/at91/pmc.c | 18 ++++++++++-------- > 1 file changed, 10 insertions(+), 8 deletions(-) > > diff --git a/drivers/clk/at91/pmc.c b/drivers/clk/at91/pmc.c > index 5aa9c1f1c886..040b70e1ffbc 100644 > --- a/drivers/clk/at91/pmc.c > +++ b/drivers/clk/at91/pmc.c > @@ -162,20 +162,18 @@ static int __init pmc_register_ops(void) > if (!np) > return -ENODEV; > > - if (!of_device_is_available(np)) { > - of_node_put(np); > - return -ENODEV; > - } > + if (!of_device_is_available(np)) > + goto put_node; > + > of_node_put(np); > > np = of_find_compatible_node(NULL, NULL, "atmel,sama5d2-securam"); > if (!np) > return -ENODEV; > > - if (!of_device_is_available(np)) { > - of_node_put(np); > - return -ENODEV; > - } > + if (!of_device_is_available(np)) > + goto put_node; > + > of_node_put(np); > > at91_pmc_backup_suspend = of_iomap(np, 0); > @@ -187,6 +185,10 @@ static int __init pmc_register_ops(void) > register_syscore_ops(&pmc_syscore_ops); > > return 0; > + > +put_node: > + of_node_put(np); > + return -ENODEV; > } > /* This has to happen before arch_initcall because of the tcb_clksrc driver */ > postcore_initcall(pmc_register_ops); > -- > 2.46.0 >
>> Add a jump target so that a bit of exception handling can be better reused >> at the end of this function implementation. … > Bad track record and no real benefit from the patch: > NACK, sorry. … >> +++ b/drivers/clk/at91/pmc.c >> @@ -162,20 +162,18 @@ static int __init pmc_register_ops(void) >> if (!np) >> return -ENODEV; >> >> - if (!of_device_is_available(np)) { >> - of_node_put(np); >> - return -ENODEV; >> - } >> + if (!of_device_is_available(np)) >> + goto put_node; >> + >> of_node_put(np); >> >> np = of_find_compatible_node(NULL, NULL, "atmel,sama5d2-securam"); >> if (!np) >> return -ENODEV; >> >> - if (!of_device_is_available(np)) { >> - of_node_put(np); >> - return -ENODEV; >> - } >> + if (!of_device_is_available(np)) >> + goto put_node; >> + >> of_node_put(np); >> >> at91_pmc_backup_suspend = of_iomap(np, 0); >> @@ -187,6 +185,10 @@ static int __init pmc_register_ops(void) >> register_syscore_ops(&pmc_syscore_ops); >> >> return 0; >> + >> +put_node: >> + of_node_put(np); >> + return -ENODEV; >> } … * Do you really disagree to advice from related information sources? 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? * How do you think about to increase the application of scope-based resource management? Regards, Markus
diff --git a/drivers/clk/at91/pmc.c b/drivers/clk/at91/pmc.c index 5aa9c1f1c886..040b70e1ffbc 100644 --- a/drivers/clk/at91/pmc.c +++ b/drivers/clk/at91/pmc.c @@ -162,20 +162,18 @@ static int __init pmc_register_ops(void) if (!np) return -ENODEV; - if (!of_device_is_available(np)) { - of_node_put(np); - return -ENODEV; - } + if (!of_device_is_available(np)) + goto put_node; + of_node_put(np); np = of_find_compatible_node(NULL, NULL, "atmel,sama5d2-securam"); if (!np) return -ENODEV; - if (!of_device_is_available(np)) { - of_node_put(np); - return -ENODEV; - } + if (!of_device_is_available(np)) + goto put_node; + of_node_put(np); at91_pmc_backup_suspend = of_iomap(np, 0); @@ -187,6 +185,10 @@ static int __init pmc_register_ops(void) register_syscore_ops(&pmc_syscore_ops); return 0; + +put_node: + of_node_put(np); + return -ENODEV; } /* This has to happen before arch_initcall because of the tcb_clksrc driver */ postcore_initcall(pmc_register_ops);