[PATCH 0/3 v4] Staging: rtl8192u: Fix coding style issues at ieee80211_crypt_wep.c

[PATCH 0/3 v4] Staging: rtl8192u: Fix coding style issues at ieee80211_crypt_wep.c

[Pedro Marzo Perez]

The checkpatch.pl script reports several errors at file ieee80211_crypt_wep.c,
this patch fixes them.

Pedro Marzo Perez (3):
  Simplify error check code at prism2_wep_init
  Remove two useless lines at ieee80211_wep_null
  Correct include indentation and openning braces at new line

 .../rtl8192u/ieee80211/ieee80211_crypt_wep.c       | 43 ++++++----------------
 1 file changed, 12 insertions(+), 31 deletions(-)

-- 
1.9.1

[PATCH 1/3 v4] Staging: rtl8192u: Simplify error check code at prism2_wep_init

[Pedro Marzo Perez]

Simplify prism2_wep_init error check code employing goto when a failure is detected.
Removed pr_debug which was given a checkpatch.pl error because of literal string
 splitted across two lines of code, it was seldom going to be printed anyway.

Signed-off-by: Pedro Marzo Perez <marzo.pedro@gmail.com>
---
 .../rtl8192u/ieee80211/ieee80211_crypt_wep.c       | 32 ++++++----------------
 1 file changed, 9 insertions(+), 23 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c
index 0a17f84..bd789d1 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c
@@ -43,38 +43,24 @@ static void *prism2_wep_init(int keyidx)
 
 	priv = kzalloc(sizeof(*priv), GFP_ATOMIC);
 	if (priv == NULL)
-		goto fail;
+		return NULL;
 	priv->key_idx = keyidx;
 
 	priv->tx_tfm = crypto_alloc_blkcipher("ecb(arc4)", 0, CRYPTO_ALG_ASYNC);
-	if (IS_ERR(priv->tx_tfm)) {
-		pr_debug("ieee80211_crypt_wep: could not allocate "
-		       "crypto API arc4\n");
-		priv->tx_tfm = NULL;
-		goto fail;
-	}
+	if (IS_ERR(priv->tx_tfm))
+		goto free_priv;
 	priv->rx_tfm = crypto_alloc_blkcipher("ecb(arc4)", 0, CRYPTO_ALG_ASYNC);
-	if (IS_ERR(priv->rx_tfm)) {
-		pr_debug("ieee80211_crypt_wep: could not allocate "
-		       "crypto API arc4\n");
-		priv->rx_tfm = NULL;
-		goto fail;
-	}
+	if (IS_ERR(priv->rx_tfm))
+		goto free_tx;
 
 	/* start WEP IV from a random value */
 	get_random_bytes(&priv->iv, 4);
 
 	return priv;
-
-fail:
-	if (priv) {
-		if (priv->tx_tfm)
-			crypto_free_blkcipher(priv->tx_tfm);
-		if (priv->rx_tfm)
-			crypto_free_blkcipher(priv->rx_tfm);
-		kfree(priv);
-	}
-
+free_tx:
+	crypto_free_blkcipher(priv->tx_tfm);
+free_priv:
+	kfree(priv);
 	return NULL;
 }
 
-- 
1.9.1

[PATCH 2/3 v4] Staging: rtl8192u: Remove two useless lines at ieee80211_wep_null

[Pedro Marzo Perez]

Remove two lines at ieee80211_wep_null which checkpatch.pl reported as errors.
The first one because it has a C99 comment style and the second one because it is a void
return which is useless.

Signed-off-by: Pedro Marzo Perez <marzo.pedro@gmail.com>
---
 drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c
index bd789d1..94622cc 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c
@@ -279,6 +279,4 @@ void __exit ieee80211_crypto_wep_exit(void)
 
 void ieee80211_wep_null(void)
 {
-//	printk("============>%s()\n", __func__);
-	return;
 }
-- 
1.9.1

Re: [PATCH 2/3 v4] Staging: rtl8192u: Remove two useless lines at ieee80211_wep_null

[Greg KH]

On Thu, May 21, 2015 at 02:25:18AM +0200, Pedro Marzo Perez wrote:
> Remove two lines at ieee80211_wep_null which checkpatch.pl reported as errors.
> The first one because it has a C99 comment style and the second one because it is a void
> return which is useless.
> 
> Signed-off-by: Pedro Marzo Perez <marzo.pedro@gmail.com>
> ---
>  drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c
> index bd789d1..94622cc 100644
> --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c
> +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c
> @@ -279,6 +279,4 @@ void __exit ieee80211_crypto_wep_exit(void)
>  
>  void ieee80211_wep_null(void)
>  {
> -//	printk("============>%s()\n", __func__);
> -	return;
>  }

Please just delete the function.

Re: [PATCH 2/3 v4] Staging: rtl8192u: Remove two useless lines at ieee80211_wep_null

[pmarzo]

On Sun, 2015-05-31 at 10:39 +0900, Greg KH wrote:
> On Thu, May 21, 2015 at 02:25:18AM +0200, Pedro Marzo Perez wrote:
> > Remove two lines at ieee80211_wep_null which checkpatch.pl reported as errors.
> > The first one because it has a C99 comment style and the second one because it is a void
> > return which is useless.
> > 
> > Signed-off-by: Pedro Marzo Perez <marzo.pedro@gmail.com>
> > ---
> >  drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c | 2 --
> >  1 file changed, 2 deletions(-)
> > 
> > diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c
> > index bd789d1..94622cc 100644
> > --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c
> > +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c
> > @@ -279,6 +279,4 @@ void __exit ieee80211_crypto_wep_exit(void)
> >  
> >  void ieee80211_wep_null(void)
> >  {
> > -//	printk("============>%s()\n", __func__);
> > -	return;
> >  }
> 
> Please just delete the function.

It is not so easy, the function is exported at file
drivers/staging/rtl8192u/ieee80211/ieee80211.h and used at
drivers/staging/rtl8192u/ieee80211/ieee80211_module.c line 179 with
several other "null" functions, it seems a dirty trick to force module
loading:
/* These function were added to load crypte module autoly */
    ieee80211_tkip_null();
    ieee80211_wep_null();
    ieee80211_ccmp_null();

I guess all this stuff should be rewritten to force the autoload
properly but I don't have the knowledge to fix all this.
Sorry Greg :-(

Re: [PATCH 2/3 v4] Staging: rtl8192u: Remove two useless lines at ieee80211_wep_null

[Greg KH]

On Tue, Jun 02, 2015 at 12:19:12AM +0200, pmarzo wrote:
> On Sun, 2015-05-31 at 10:39 +0900, Greg KH wrote:
> > On Thu, May 21, 2015 at 02:25:18AM +0200, Pedro Marzo Perez wrote:
> > > Remove two lines at ieee80211_wep_null which checkpatch.pl reported as errors.
> > > The first one because it has a C99 comment style and the second one because it is a void
> > > return which is useless.
> > > 
> > > Signed-off-by: Pedro Marzo Perez <marzo.pedro@gmail.com>
> > > ---
> > >  drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c | 2 --
> > >  1 file changed, 2 deletions(-)
> > > 
> > > diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c
> > > index bd789d1..94622cc 100644
> > > --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c
> > > +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c
> > > @@ -279,6 +279,4 @@ void __exit ieee80211_crypto_wep_exit(void)
> > >  
> > >  void ieee80211_wep_null(void)
> > >  {
> > > -//	printk("============>%s()\n", __func__);
> > > -	return;
> > >  }
> > 
> > Please just delete the function.
> 
> It is not so easy, the function is exported at file
> drivers/staging/rtl8192u/ieee80211/ieee80211.h and used at
> drivers/staging/rtl8192u/ieee80211/ieee80211_module.c line 179 with
> several other "null" functions, it seems a dirty trick to force module
> loading:
> /* These function were added to load crypte module autoly */
>     ieee80211_tkip_null();
>     ieee80211_wep_null();
>     ieee80211_ccmp_null();
> 
> I guess all this stuff should be rewritten to force the autoload
> properly but I don't have the knowledge to fix all this.
> Sorry Greg :-(

Ugh, that's a mess.  Ok, I'll take this patch again, please resend it.

thanks,

greg k-h
> 

Re: [PATCH 2/3 v4] Staging: rtl8192u: Remove two useless lines at ieee80211_wep_null

[pmarzo]

On mar, 2015-06-02 at 14:21 +0900, Greg KH wrote:
> On Tue, Jun 02, 2015 at 12:19:12AM +0200, pmarzo wrote:
> > On Sun, 2015-05-31 at 10:39 +0900, Greg KH wrote:
> > > On Thu, May 21, 2015 at 02:25:18AM +0200, Pedro Marzo Perez wrote:
> > > > Remove two lines at ieee80211_wep_null which checkpatch.pl reported as errors.
> > > > The first one because it has a C99 comment style and the second one because it is a void
> > > > return which is useless.
> > > > 
> > > > Signed-off-by: Pedro Marzo Perez <marzo.pedro@gmail.com>
> > > > ---
> > > >  drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c | 2 --
> > > >  1 file changed, 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c
> > > > index bd789d1..94622cc 100644
> > > > --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c
> > > > +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c
> > > > @@ -279,6 +279,4 @@ void __exit ieee80211_crypto_wep_exit(void)
> > > >  
> > > >  void ieee80211_wep_null(void)
> > > >  {
> > > > -//	printk("============>%s()\n", __func__);
> > > > -	return;
> > > >  }
> > > 
> > > Please just delete the function.
> > 
> > It is not so easy, the function is exported at file
> > drivers/staging/rtl8192u/ieee80211/ieee80211.h and used at
> > drivers/staging/rtl8192u/ieee80211/ieee80211_module.c line 179 with
> > several other "null" functions, it seems a dirty trick to force module
> > loading:
> > /* These function were added to load crypte module autoly */
> >     ieee80211_tkip_null();
> >     ieee80211_wep_null();
> >     ieee80211_ccmp_null();
> > 
> > I guess all this stuff should be rewritten to force the autoload
> > properly but I don't have the knowledge to fix all this.
> > Sorry Greg :-(
> 
> Ugh, that's a mess.  Ok, I'll take this patch again, please resend it.
> 
> thanks,
> 
> greg k-h
> > 

Ok, no problem, just not sure what do I have to resend.

Should I resend the whole serie of three patches 1/3 2/3 3/3? 
I received an e-mail confirming the acceptance of patch 3/3 on your
staging tree, no news about patch 1/3 yet, so it may confuse you if I
resend all of them.

Should I just send this patch outside the serie as a standalone patch?

regards, Pedro.

Re: [PATCH 2/3 v4] Staging: rtl8192u: Remove two useless lines at ieee80211_wep_null

[Greg KH]

On Tue, Jun 02, 2015 at 10:08:19AM +0200, pmarzo wrote:
> On mar, 2015-06-02 at 14:21 +0900, Greg KH wrote:
> > On Tue, Jun 02, 2015 at 12:19:12AM +0200, pmarzo wrote:
> > > On Sun, 2015-05-31 at 10:39 +0900, Greg KH wrote:
> > > > On Thu, May 21, 2015 at 02:25:18AM +0200, Pedro Marzo Perez wrote:
> > > > > Remove two lines at ieee80211_wep_null which checkpatch.pl reported as errors.
> > > > > The first one because it has a C99 comment style and the second one because it is a void
> > > > > return which is useless.
> > > > > 
> > > > > Signed-off-by: Pedro Marzo Perez <marzo.pedro@gmail.com>
> > > > > ---
> > > > >  drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c | 2 --
> > > > >  1 file changed, 2 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c
> > > > > index bd789d1..94622cc 100644
> > > > > --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c
> > > > > +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c
> > > > > @@ -279,6 +279,4 @@ void __exit ieee80211_crypto_wep_exit(void)
> > > > >  
> > > > >  void ieee80211_wep_null(void)
> > > > >  {
> > > > > -//	printk("============>%s()\n", __func__);
> > > > > -	return;
> > > > >  }
> > > > 
> > > > Please just delete the function.
> > > 
> > > It is not so easy, the function is exported at file
> > > drivers/staging/rtl8192u/ieee80211/ieee80211.h and used at
> > > drivers/staging/rtl8192u/ieee80211/ieee80211_module.c line 179 with
> > > several other "null" functions, it seems a dirty trick to force module
> > > loading:
> > > /* These function were added to load crypte module autoly */
> > >     ieee80211_tkip_null();
> > >     ieee80211_wep_null();
> > >     ieee80211_ccmp_null();
> > > 
> > > I guess all this stuff should be rewritten to force the autoload
> > > properly but I don't have the knowledge to fix all this.
> > > Sorry Greg :-(
> > 
> > Ugh, that's a mess.  Ok, I'll take this patch again, please resend it.
> > 
> > thanks,
> > 
> > greg k-h
> > > 
> 
> Ok, no problem, just not sure what do I have to resend.
> 
> Should I resend the whole serie of three patches 1/3 2/3 3/3? 
> I received an e-mail confirming the acceptance of patch 3/3 on your
> staging tree, no news about patch 1/3 yet, so it may confuse you if I
> resend all of them.

Resend whatever I have not yet applied.

thanks,

greg k-h

Re: [PATCH 2/3 v4] Staging: rtl8192u: Remove two useless lines at ieee80211_wep_null

[pmarzo]

On mar, 2015-06-02 at 17:25 +0900, Greg KH wrote:
> On Tue, Jun 02, 2015 at 10:08:19AM +0200, pmarzo wrote:
> > On mar, 2015-06-02 at 14:21 +0900, Greg KH wrote:
> > > On Tue, Jun 02, 2015 at 12:19:12AM +0200, pmarzo wrote:
> > > > On Sun, 2015-05-31 at 10:39 +0900, Greg KH wrote:
> > > > > On Thu, May 21, 2015 at 02:25:18AM +0200, Pedro Marzo Perez wrote:
> > > > > > Remove two lines at ieee80211_wep_null which checkpatch.pl reported as errors.
> > > > > > The first one because it has a C99 comment style and the second one because it is a void
> > > > > > return which is useless.
> > > > > > 
> > > > > > Signed-off-by: Pedro Marzo Perez <marzo.pedro@gmail.com>
> > > > > > ---
> > > > > >  drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c | 2 --
> > > > > >  1 file changed, 2 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c
> > > > > > index bd789d1..94622cc 100644
> > > > > > --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c
> > > > > > +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c
> > > > > > @@ -279,6 +279,4 @@ void __exit ieee80211_crypto_wep_exit(void)
> > > > > >  
> > > > > >  void ieee80211_wep_null(void)
> > > > > >  {
> > > > > > -//	printk("============>%s()\n", __func__);
> > > > > > -	return;
> > > > > >  }
> > > > > 
> > > > > Please just delete the function.
> > > > 
> > > > It is not so easy, the function is exported at file
> > > > drivers/staging/rtl8192u/ieee80211/ieee80211.h and used at
> > > > drivers/staging/rtl8192u/ieee80211/ieee80211_module.c line 179 with
> > > > several other "null" functions, it seems a dirty trick to force module
> > > > loading:
> > > > /* These function were added to load crypte module autoly */
> > > >     ieee80211_tkip_null();
> > > >     ieee80211_wep_null();
> > > >     ieee80211_ccmp_null();
> > > > 
> > > > I guess all this stuff should be rewritten to force the autoload
> > > > properly but I don't have the knowledge to fix all this.
> > > > Sorry Greg :-(
> > > 
> > > Ugh, that's a mess.  Ok, I'll take this patch again, please resend it.
> > > 
> > > thanks,
> > > 
> > > greg k-h
> > > > 
> > 
> > Ok, no problem, just not sure what do I have to resend.
> > 
> > Should I resend the whole serie of three patches 1/3 2/3 3/3? 
> > I received an e-mail confirming the acceptance of patch 3/3 on your
> > staging tree, no news about patch 1/3 yet, so it may confuse you if I
> > resend all of them.
> 
> Resend whatever I have not yet applied.
> 
> thanks,
> 
> greg k-h

Ok, I will download your staging tree and regenerate patches 1/3 and 2/3
with that git tree. That would be v5 1/2 and 2/2 new patches.
Just one (probably very stupid) question, why do you need me to resend
the patches? I mean, both of them apply cleanly to your staging tree
with patch 3/3 already merged.

regards, Pedro.

Re: [PATCH 2/3 v4] Staging: rtl8192u: Remove two useless lines at ieee80211_wep_null

[Greg KH]

On Tue, Jun 02, 2015 at 03:35:05PM +0200, pmarzo wrote:
> Ok, I will download your staging tree and regenerate patches 1/3 and 2/3
> with that git tree. That would be v5 1/2 and 2/2 new patches.
> Just one (probably very stupid) question, why do you need me to resend
> the patches? I mean, both of them apply cleanly to your staging tree
> with patch 3/3 already merged.

Because I don't have them in my inbox anywhere.

I average about 1000 emails a day, not including high-volume mailing
lists (lkml, linux-fsdev, etc.)  Once I deal with an email, I delete it
as it does me no good to keep them around for no reason.

thanks,

greg k-h

Re: [PATCH 2/3 v4] Staging: rtl8192u: Remove two useless lines at ieee80211_wep_null

[pmarzo]

On mar, 2015-06-02 at 22:40 +0900, Greg KH wrote:
> On Tue, Jun 02, 2015 at 03:35:05PM +0200, pmarzo wrote:
> > Ok, I will download your staging tree and regenerate patches 1/3 and 2/3
> > with that git tree. That would be v5 1/2 and 2/2 new patches.
> > Just one (probably very stupid) question, why do you need me to resend
> > the patches? I mean, both of them apply cleanly to your staging tree
> > with patch 3/3 already merged.
> 
> Because I don't have them in my inbox anywhere.
> 
> I average about 1000 emails a day, not including high-volume mailing
> lists (lkml, linux-fsdev, etc.)  Once I deal with an email, I delete it
> as it does me no good to keep them around for no reason.
> 
> thanks,
> 
> greg k-h

1000 emails!! that's really amazing, as Dan Carpenter said you are a
really busy person. Now I understand why your answers are so brief.
I've got 33 e-mails this morning but instead of thinking "uggh what a
bad morning" I am actually thinking "I am really lucky, Greg still has
967 more to read!" :-)

regards, Pedro.

[PATCH 3/3 v4] Staging: rtl8192u: Correct include indentation and openning braces at new line

[Pedro Marzo Perez]

Opening braces should never be in a new line.
Correct include indentation.

Signed-off-by: Pedro Marzo Perez <marzo.pedro@gmail.com>
---
 drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c
index 94622cc..681611d 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c
@@ -19,7 +19,7 @@
 #include "ieee80211.h"
 
 #include <linux/crypto.h>
-    #include <linux/scatterlist.h>
+#include <linux/scatterlist.h>
 #include <linux/crc32.h>
 
 MODULE_AUTHOR("Jouni Malinen");
@@ -128,9 +128,7 @@ static int prism2_wep_encrypt(struct sk_buff *skb, int hdr_len, void *priv)
 	/* Copy rest of the WEP key (the secret part) */
 	memcpy(key + 3, wep->key, wep->key_len);
 
-	if (!tcb_desc->bHwSec)
-	{
-
+	if (!tcb_desc->bHwSec) {
 		/* Append little-endian CRC32 and encrypt it to produce ICV */
 		crc = ~crc32_le(~0, pos, len);
 		icv = skb_put(skb, 4);
@@ -187,8 +185,7 @@ static int prism2_wep_decrypt(struct sk_buff *skb, int hdr_len, void *priv)
 	/* Apply RC4 to data and compute CRC32 over decrypted data */
 	plen = skb->len - hdr_len - 8;
 
-	if (!tcb_desc->bHwSec)
-	{
+	if (!tcb_desc->bHwSec) {
 		crypto_blkcipher_setkey(wep->rx_tfm, key, klen);
 		sg_init_one(&sg, pos, plen+4);
 
-- 
1.9.1