| Message ID | 80911d4b-d819-335c-106e-70d7672e645b@users.sourceforge.net |
|---|---|
| State | New |
| Headers | show |
| Series | pinctrl: sirf: Use common error handling code in sirfsoc_dt_node_to_map() | expand |
Hi Markus, On Mon, Oct 30, 2017 at 7:36 PM, SF Markus Elfring <elfring@users.sourceforge.net> wrote: > From: Markus Elfring <elfring@users.sourceforge.net> > Date: Mon, 30 Oct 2017 19:26:56 +0100 > > Add a jump target so that a bit of exception handling can be better reused > at the end of this function. > > This issue was detected by using the Coccinelle software. Does Coccinelle have a threshold for how much cleanup can be shared? > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net> > --- > drivers/pinctrl/sirf/pinctrl-sirf.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) Again, breaking the flow for the reviewer isn't worth it here, IMHO. > diff --git a/drivers/pinctrl/sirf/pinctrl-sirf.c b/drivers/pinctrl/sirf/pinctrl-sirf.c > index d64add0b84cc..5ec2f37f5180 100644 > --- a/drivers/pinctrl/sirf/pinctrl-sirf.c > +++ b/drivers/pinctrl/sirf/pinctrl-sirf.c > @@ -89,16 +89,12 @@ static int sirfsoc_dt_node_to_map(struct pinctrl_dev *pctldev, > /* calculate number of maps required */ > for_each_child_of_node(np_config, np) { > ret = of_property_read_string(np, "sirf,function", &function); > - if (ret < 0) { > - of_node_put(np); > - return ret; > - } > + if (ret < 0) > + goto put_node; > > ret = of_property_count_strings(np, "sirf,pins"); > - if (ret < 0) { > - of_node_put(np); > - return ret; > - } > + if (ret < 0) > + goto put_node; > > count += ret; > } > @@ -125,6 +121,10 @@ static int sirfsoc_dt_node_to_map(struct pinctrl_dev *pctldev, > *num_maps = count; > > return 0; > + > +put_node: > + of_node_put(np); > + return ret; > } Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Add a jump target so that a bit of exception handling can be better reused >> at the end of this function. >> >> This issue was detected by using the Coccinelle software. > > Does Coccinelle have a threshold for how much cleanup can be shared? Are you looking for a limitation of such source code search patterns? Regards, Markus -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Oct 30, 2017 at 8:32 PM, SF Markus Elfring <elfring@users.sourceforge.net> wrote: >>> Add a jump target so that a bit of exception handling can be better reused >>> at the end of this function. >>> >>> This issue was detected by using the Coccinelle software. >> >> Does Coccinelle have a threshold for how much cleanup can be shared? > > Are you looking for a limitation of such source code search patterns? Exactly. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Are you looking for a limitation of such source code search patterns? > > Exactly. Would you like to explain your view a bit more? Regards, Markus -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Really, the of_node_put() is connected to the loop and not the function so the original code is the right way to do it. Otherwise if we added more error handling to the function then normal kernel error handling wouldn't work naturally. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/pinctrl/sirf/pinctrl-sirf.c b/drivers/pinctrl/sirf/pinctrl-sirf.c index d64add0b84cc..5ec2f37f5180 100644 --- a/drivers/pinctrl/sirf/pinctrl-sirf.c +++ b/drivers/pinctrl/sirf/pinctrl-sirf.c @@ -89,16 +89,12 @@ static int sirfsoc_dt_node_to_map(struct pinctrl_dev *pctldev, /* calculate number of maps required */ for_each_child_of_node(np_config, np) { ret = of_property_read_string(np, "sirf,function", &function); - if (ret < 0) { - of_node_put(np); - return ret; - } + if (ret < 0) + goto put_node; ret = of_property_count_strings(np, "sirf,pins"); - if (ret < 0) { - of_node_put(np); - return ret; - } + if (ret < 0) + goto put_node; count += ret; } @@ -125,6 +121,10 @@ static int sirfsoc_dt_node_to_map(struct pinctrl_dev *pctldev, *num_maps = count; return 0; + +put_node: + of_node_put(np); + return ret; } static void sirfsoc_dt_free_map(struct pinctrl_dev *pctldev,