[PATCH] Staging: wlan-ng: hfa384x_usb: fixed an 'else' statement coding style issue

[PATCH] Staging: wlan-ng: hfa384x_usb: fixed an 'else' statement coding style issue

[Eduardo Barretto]

Fixed a coding style issue

Signed-off-by: Eduardo Barretto <edusbarretto@gmail.com>
---
 drivers/staging/wlan-ng/hfa384x_usb.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/wlan-ng/hfa384x_usb.c b/drivers/staging/wlan-ng/hfa384x_usb.c
index 55d2f56..d388f2c 100644
--- a/drivers/staging/wlan-ng/hfa384x_usb.c
+++ b/drivers/staging/wlan-ng/hfa384x_usb.c
@@ -4123,12 +4123,11 @@ static int hfa384x_isgood_pdrcode(u16 pdrcode)
 			pr_debug("Encountered unknown PDR#=0x%04x, assuming it's ok.\n",
 				 pdrcode);
 			return 1;
-		} else {
-			/* bad code */
-			pr_debug("Encountered unknown PDR#=0x%04x, (>=0x1000), assuming it's bad.\n",
-				 pdrcode);
-			return 0;
 		}
+		/* bad code */
+		pr_debug("Encountered unknown PDR#=0x%04x, (>=0x1000), assuming it's bad.\n",
+			 pdrcode);
+		return 0;
 	}
 	return 0;		/* avoid compiler warnings */
 }
-- 
2.1.3

Re: [PATCH] Staging: wlan-ng: hfa384x_usb: fixed an 'else' statement coding style issue

[Joe Perches]

On Mon, 2014-12-15 at 00:02 -0200, Eduardo Barretto wrote:
> Fixed a coding style issue
[]
> diff --git a/drivers/staging/wlan-ng/hfa384x_usb.c b/drivers/staging/wlan-ng/hfa384x_usb.c
[]
> @@ -4123,12 +4123,11 @@ static int hfa384x_isgood_pdrcode(u16 pdrcode)
>  			pr_debug("Encountered unknown PDR#=0x%04x, assuming it's ok.\n",
>  				 pdrcode);
>  			return 1;
> -		} else {
> -			/* bad code */
> -			pr_debug("Encountered unknown PDR#=0x%04x, (>=0x1000), assuming it's bad.\n",
> -				 pdrcode);
> -			return 0;
>  		}
> +		/* bad code */
> +		pr_debug("Encountered unknown PDR#=0x%04x, (>=0x1000), assuming it's bad.\n",
> +			 pdrcode);
> +		return 0;
>  	}
>  	return 0;		/* avoid compiler warnings */
>  }

While this patch isn't necessary, if any change is
done, it might better to not have two consecutive
return 0; uses.

Re: [PATCH] Staging: wlan-ng: hfa384x_usb: fixed an 'else' statement coding style issue

[Eduardo Barretto]

Thank you for the quick feedback. 
It was my first patch to the kernel and I wanted to be sure it would get right to the community.
I'll be making a version two with the consideration you brought me.

Thank you again,
Best regards,

Eduardo Barretto

On Sun, Dec 14, 2014 at 06:11:30PM -0800, Joe Perches wrote:
> On Mon, 2014-12-15 at 00:02 -0200, Eduardo Barretto wrote:
> > Fixed a coding style issue
> []
> > diff --git a/drivers/staging/wlan-ng/hfa384x_usb.c b/drivers/staging/wlan-ng/hfa384x_usb.c
> []
> > @@ -4123,12 +4123,11 @@ static int hfa384x_isgood_pdrcode(u16 pdrcode)
> >  			pr_debug("Encountered unknown PDR#=0x%04x, assuming it's ok.\n",
> >  				 pdrcode);
> >  			return 1;
> > -		} else {
> > -			/* bad code */
> > -			pr_debug("Encountered unknown PDR#=0x%04x, (>=0x1000), assuming it's bad.\n",
> > -				 pdrcode);
> > -			return 0;
> >  		}
> > +		/* bad code */
> > +		pr_debug("Encountered unknown PDR#=0x%04x, (>=0x1000), assuming it's bad.\n",
> > +			 pdrcode);
> > +		return 0;
> >  	}
> >  	return 0;		/* avoid compiler warnings */
> >  }
> 
> While this patch isn't necessary, if any change is
> done, it might better to not have two consecutive
> return 0; uses.
> 
> 

Re: [PATCH] Staging: wlan-ng: hfa384x_usb: fixed an 'else' statement coding style issue

[Joe Perches]

On Mon, 2014-12-15 at 18:53 -0200, Eduardo Barretto wrote:
> Thank you for the quick feedback. 
> It was my first patch to the kernel and I wanted to be sure it would get right to the community.
> I'll be making a version two with the consideration you brought me.

the code today is:

{
	switch (prdcode) {
	case [...]
		return 1;
	default:
		if (prdcode < 0x1000) {
			printk(msg1);
			return 1;
		else
			printk(msg2);
			return 0;
	}
	return 0;	/* avoid compiler noise */
}

I think this code does not needs changing.
I think more modern compilers don't even warn
when the last return 0; isn't there.

If it were to be changed, I'd probably write it like:

{
	switch (prdcode) {
	case [...]
		return 1;
	default:
		if (prdcode < 0x1000) {
			printk(msg1);
			return 1;
		}
		break;
	}

	printk(msg2);
	return 0;
}

but I wouldn't bother.