pinctrl: sirf: Use common error handling code in sirfsoc_dt_node_to_map()

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()
Related show

Commit Message

SF Markus Elfring Oct. 30, 2017, 6:36 p.m.
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.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/pinctrl/sirf/pinctrl-sirf.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

Comments

Geert Uytterhoeven Oct. 30, 2017, 7:15 p.m. | #1
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
SF Markus Elfring Oct. 30, 2017, 7:32 p.m. | #2
>> 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
Geert Uytterhoeven Oct. 30, 2017, 7:34 p.m. | #3
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
SF Markus Elfring Oct. 30, 2017, 7:41 p.m. | #4
>> 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
Dan Carpenter Nov. 1, 2017, 10:33 a.m. | #5
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

Patch

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,