2023-03-28 22:05:55

by Alexandre Belloni

[permalink] [raw]
Subject: Re: [PATCH resent] clk: at91: sama7g5: Add two jump labels in sama7g5_pmc_setup()

On 28/03/2023 21:24:00+0200, Markus Elfring wrote:
> >> The label “err_free” was used to jump to another pointer check despite of
> >> the detail in the implementation of the function “sama7g5_pmc_setup”
> >> that it was determined already that the corresponding variable contained
> >> a null pointer (because of a failed memory allocation).
> >>
> >> * Thus use additional labels.
> >>
> >> * Delete an extra pointer check at the end which became unnecessary
> >>    with this refactoring.
> >>
> >> This issue was detected by using the Coccinelle software.
> >
> > Fine, but I'm sorry that it complexity the function for no real value.
>
> Under which circumstances can advice be taken better into account
> also from another information source?
> 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#MEM12C.Considerusingagotochainwhenleavingafunctiononerrorwhenusingandreleasingresources-CompliantSolution%28POSIX,GotoChain%29
>
>
>
> > Other clk drivers have the same pattern so I want them to all stay the same.
> > This is a NACK, sorry about that.
>
> I am curious if other contributors (or code reviewers) would like to influence
> the software situation a bit more.
>

I agree with Nicolas here, this is useless churn.


--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com