[PATCH] MIPS: Octeon: Delete an error message for a failed memory allocation in octeon_irq_init_gpio()

[PATCH] MIPS: Octeon: Delete an error message for a failed memory allocation in octeon_irq_init_gpio()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 23 May 2017 20:00:06 +0200

Omit an extra message for a memory allocation failure in this function.

This issue was detected by using the Coccinelle software.

Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 arch/mips/cavium-octeon/octeon-irq.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/mips/cavium-octeon/octeon-irq.c b/arch/mips/cavium-octeon/octeon-irq.c
index c1eb1ff7c800..050c08ece5b6 100644
--- a/arch/mips/cavium-octeon/octeon-irq.c
+++ b/arch/mips/cavium-octeon/octeon-irq.c
@@ -1615,7 +1615,6 @@ static int __init octeon_irq_init_gpio(
 		irq_domain_add_linear(
 			gpio_node, 16, &octeon_irq_domain_gpio_ops, gpiod);
 	} else {
-		pr_warn("Cannot allocate memory for GPIO irq_domain.\n");
 		return -ENOMEM;
 	}
 
-- 
2.13.0

Re: [PATCH] MIPS: Octeon: Delete an error message for a failed memory allocation in octeon_irq_init_gpio()

[Joe Perches]

On Tue, 2017-05-23 at 20:10 +0200, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 23 May 2017 20:00:06 +0200
> 
> Omit an extra message for a memory allocation failure in this function.
> 
> This issue was detected by using the Coccinelle software.
> 
> Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  arch/mips/cavium-octeon/octeon-irq.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/mips/cavium-octeon/octeon-irq.c b/arch/mips/cavium-octeon/octeon-irq.c
> index c1eb1ff7c800..050c08ece5b6 100644
> --- a/arch/mips/cavium-octeon/octeon-irq.c
> +++ b/arch/mips/cavium-octeon/octeon-irq.c
> @@ -1615,7 +1615,6 @@ static int __init octeon_irq_init_gpio(
>  		irq_domain_add_linear(
>  			gpio_node, 16, &octeon_irq_domain_gpio_ops, gpiod);
>  	} else {
> -		pr_warn("Cannot allocate memory for GPIO irq_domain.\n");
>  		return -ENOMEM;
>  	}

You really should reverse the test here and
unindent the first block.

Again:  Don't be mindless.
        Take the time to improve the code.
---
diff --git a/arch/mips/cavium-octeon/octeon-irq.c b/arch/mips/cavium-octeon/octe
index c1eb1ff7c800..2bdc750f2f2d 100644
--- a/arch/mips/cavium-octeon/octeon-irq.c
+++ b/arch/mips/cavium-octeon/octeon-irq.c
@@ -1609,15 +1609,13 @@ static int __init octeon_irq_init_gpio(
 	}
 
 	gpiod = kzalloc(sizeof(*gpiod), GFP_KERNEL);
-	if (gpiod) {
-		/* gpio domain host_data is the base hwirq number. */
-		gpiod->base_hwirq = base_hwirq;
-		irq_domain_add_linear(
-			gpio_node, 16, &octeon_irq_domain_gpio_ops, gpiod);
-	} else {
-		pr_warn("Cannot allocate memory for GPIO irq_domain.\n");
+	if (!gpiod)
 		return -ENOMEM;
-	}
+
+	/* gpio domain host_data is the base hwirq number. */
+	gpiod->base_hwirq = base_hwirq;
+	irq_domain_add_linear(gpio_node, 16,
+			      &octeon_irq_domain_gpio_ops, gpiod);
 
 	/*
 	 * Clear the OF_POPULATED flag that was set by of_irq_init()

Re: [PATCH] MIPS: Octeon: Delete an error message for a failed memory allocation in octeon_irq_init_gpio()

[David Daney]

On 05/23/2017 11:55 AM, Joe Perches wrote:
> On Tue, 2017-05-23 at 20:10 +0200, SF Markus Elfring wrote:
>> From: Markus Elfring <elfring@users.sourceforge.net>
>> Date: Tue, 23 May 2017 20:00:06 +0200
>>
>> Omit an extra message for a memory allocation failure in this function.
>>
>> This issue was detected by using the Coccinelle software.
>>
>> Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
>> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
>> ---
>>   arch/mips/cavium-octeon/octeon-irq.c | 1 -
>>   1 file changed, 1 deletion(-)
>>
>> diff --git a/arch/mips/cavium-octeon/octeon-irq.c b/arch/mips/cavium-octeon/octeon-irq.c
>> index c1eb1ff7c800..050c08ece5b6 100644
>> --- a/arch/mips/cavium-octeon/octeon-irq.c
>> +++ b/arch/mips/cavium-octeon/octeon-irq.c
>> @@ -1615,7 +1615,6 @@ static int __init octeon_irq_init_gpio(
>>   		irq_domain_add_linear(
>>   			gpio_node, 16, &octeon_irq_domain_gpio_ops, gpiod);
>>   	} else {
>> -		pr_warn("Cannot allocate memory for GPIO irq_domain.\n");
>>   		return -ENOMEM;
>>   	}
> 
> You really should reverse the test here and
> unindent the first block.
> 
> Again:  Don't be mindless.
>          Take the time to improve the code.

Quite right.

For  Markus Elfring's version: NAK

For Joe's version:

Acked-by: David Daney <david.daney@cavium.com>



> ---
> diff --git a/arch/mips/cavium-octeon/octeon-irq.c b/arch/mips/cavium-octeon/octe
> index c1eb1ff7c800..2bdc750f2f2d 100644
> --- a/arch/mips/cavium-octeon/octeon-irq.c
> +++ b/arch/mips/cavium-octeon/octeon-irq.c
> @@ -1609,15 +1609,13 @@ static int __init octeon_irq_init_gpio(
>   	}
>   
>   	gpiod = kzalloc(sizeof(*gpiod), GFP_KERNEL);
> -	if (gpiod) {
> -		/* gpio domain host_data is the base hwirq number. */
> -		gpiod->base_hwirq = base_hwirq;
> -		irq_domain_add_linear(
> -			gpio_node, 16, &octeon_irq_domain_gpio_ops, gpiod);
> -	} else {
> -		pr_warn("Cannot allocate memory for GPIO irq_domain.\n");
> +	if (!gpiod)
>   		return -ENOMEM;
> -	}
> +
> +	/* gpio domain host_data is the base hwirq number. */
> +	gpiod->base_hwirq = base_hwirq;
> +	irq_domain_add_linear(gpio_node, 16,
> +			      &octeon_irq_domain_gpio_ops, gpiod);
>   
>   	/*
>   	 * Clear the OF_POPULATED flag that was set by of_irq_init()
> 

Re: [PATCH] MIPS: Octeon: Delete an error message for a failed memory allocation in octeon_irq_init_gpio()

[David Daney]

Steven,

Take Joe Perches' patch, put a proper change log on it and test it.

Hopefully Joe will agree to a SoB and we will do that instead of the 
Markus Elfring thing.

Thanks,
David


On 05/23/2017 11:55 AM, Joe Perches wrote:
> On Tue, 2017-05-23 at 20:10 +0200, SF Markus Elfring wrote:
>> From: Markus Elfring <elfring@users.sourceforge.net>
>> Date: Tue, 23 May 2017 20:00:06 +0200
>>
>> Omit an extra message for a memory allocation failure in this function.
>>
>> This issue was detected by using the Coccinelle software.
>>
>> Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
>> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
>> ---
>>   arch/mips/cavium-octeon/octeon-irq.c | 1 -
>>   1 file changed, 1 deletion(-)
>>
>> diff --git a/arch/mips/cavium-octeon/octeon-irq.c b/arch/mips/cavium-octeon/octeon-irq.c
>> index c1eb1ff7c800..050c08ece5b6 100644
>> --- a/arch/mips/cavium-octeon/octeon-irq.c
>> +++ b/arch/mips/cavium-octeon/octeon-irq.c
>> @@ -1615,7 +1615,6 @@ static int __init octeon_irq_init_gpio(
>>   		irq_domain_add_linear(
>>   			gpio_node, 16, &octeon_irq_domain_gpio_ops, gpiod);
>>   	} else {
>> -		pr_warn("Cannot allocate memory for GPIO irq_domain.\n");
>>   		return -ENOMEM;
>>   	}
> 
> You really should reverse the test here and
> unindent the first block.
> 
> Again:  Don't be mindless.
>          Take the time to improve the code.
> ---
> diff --git a/arch/mips/cavium-octeon/octeon-irq.c b/arch/mips/cavium-octeon/octe
> index c1eb1ff7c800..2bdc750f2f2d 100644
> --- a/arch/mips/cavium-octeon/octeon-irq.c
> +++ b/arch/mips/cavium-octeon/octeon-irq.c
> @@ -1609,15 +1609,13 @@ static int __init octeon_irq_init_gpio(
>   	}
>   
>   	gpiod = kzalloc(sizeof(*gpiod), GFP_KERNEL);
> -	if (gpiod) {
> -		/* gpio domain host_data is the base hwirq number. */
> -		gpiod->base_hwirq = base_hwirq;
> -		irq_domain_add_linear(
> -			gpio_node, 16, &octeon_irq_domain_gpio_ops, gpiod);
> -	} else {
> -		pr_warn("Cannot allocate memory for GPIO irq_domain.\n");
> +	if (!gpiod)
>   		return -ENOMEM;
> -	}
> +
> +	/* gpio domain host_data is the base hwirq number. */
> +	gpiod->base_hwirq = base_hwirq;
> +	irq_domain_add_linear(gpio_node, 16,
> +			      &octeon_irq_domain_gpio_ops, gpiod);
>   
>   	/*
>   	 * Clear the OF_POPULATED flag that was set by of_irq_init()
> 

Re: [PATCH] MIPS: Octeon: Delete an error message for a failed memory allocation in octeon_irq_init_gpio()

[SF Markus Elfring]

>> +++ b/arch/mips/cavium-octeon/octeon-irq.c
>> @@ -1615,7 +1615,6 @@ static int __init octeon_irq_init_gpio(
>>  		irq_domain_add_linear(
>>  			gpio_node, 16, &octeon_irq_domain_gpio_ops, gpiod);
>>  	} else {
>> -		pr_warn("Cannot allocate memory for GPIO irq_domain.\n");
>>  		return -ENOMEM;
>>  	}
> 
> You really should reverse the test here and
> unindent the first block.

Thanks for your improved source code transformation.

I am curious if I will stumble on a similar change possibility once more
for remaining update candidates in other software areas.

Regards,
Markus

Re: [PATCH] MIPS: Octeon: Delete an error message for a failed memory allocation in octeon_irq_init_gpio()

[Joe Perches]

On Wed, 2017-05-24 at 18:01 +0200, SF Markus Elfring wrote:
> I am curious if I will stumble on a similar change possibility once more
> for remaining update candidates in other software areas.

Only if you keep your eyes open to the possibilities.

Re: [PATCH] MIPS: Octeon: Delete an error message for a failed memory allocation in octeon_irq_init_gpio()

[Geert Uytterhoeven]

On Wed, May 24, 2017 at 9:43 PM, Joe Perches <joe@perches.com> wrote:
> On Wed, 2017-05-24 at 18:01 +0200, SF Markus Elfring wrote:
>> I am curious if I will stumble on a similar change possibility once more
>> for remaining update candidates in other software areas.
>
> Only if you keep your eyes open to the possibilities.

It's like a puzzle: how to fold the if/else trees and forests to achieve
the longest linear path and the smallest indentation ;-)

And... goto (to the end/cleanup phase!) is your friend...

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