* [PATCH 0/3 v3] Staging: rtl8192u: Fix coding style issues at ieee80211_crypt_wep.c @ 2015-05-18 23:32 Pedro Marzo Perez 2015-05-18 23:32 ` [PATCH 1/3 v3] Staging: rtl8192u: Simplify error check code at prism2_wep_init Pedro Marzo Perez ` (2 more replies) 0 siblings, 3 replies; 11+ messages in thread From: Pedro Marzo Perez @ 2015-05-18 23:32 UTC (permalink / raw) To: gregkh, navyasri.tech, dilekuzulmez, joe, haticeerturk27 Cc: devel, linux-kernel The checkpatch.pl script reports several errors and warnings at file ieee80211_crypt_wep.c, this series of patches fixes them. Pedro Marzo Perez (3): ieee80211_crypt_wep: Simplify error check code at prism2_wep_init ieee80211_crypt_wep: Remove two useless lines at ieee80211_wep_null ieee80211_crypt_wep: Correct include indentation and openning braces at new line .../rtl8192u/ieee80211/ieee80211_crypt_wep.c | 33 ++++++++-------------- 1 file changed, 12 insertions(+), 21 deletions(-) -- 1.9.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/3 v3] Staging: rtl8192u: Simplify error check code at prism2_wep_init 2015-05-18 23:32 [PATCH 0/3 v3] Staging: rtl8192u: Fix coding style issues at ieee80211_crypt_wep.c Pedro Marzo Perez @ 2015-05-18 23:32 ` Pedro Marzo Perez 2015-05-19 5:34 ` Sudip Mukherjee ` (2 more replies) 2015-05-18 23:32 ` [PATCH 2/3 v3] Staging: rtl8192u: Remove two useless lines at ieee80211_wep_null Pedro Marzo Perez 2015-05-18 23:32 ` [PATCH 3/3 v3] Staging: rtl8192u: Correct include indentation and openning braces at new line Pedro Marzo Perez 2 siblings, 3 replies; 11+ messages in thread From: Pedro Marzo Perez @ 2015-05-18 23:32 UTC (permalink / raw) To: gregkh, navyasri.tech, dilekuzulmez, joe, haticeerturk27 Cc: devel, linux-kernel Merge two pr_debug lines with literal strings splitted across several lines into one single line, simplifying prism2_wep_init error check code. Signed-off-by: Pedro Marzo Perez <marzo.pedro@gmail.com> --- .../rtl8192u/ieee80211/ieee80211_crypt_wep.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c index 0a17f84..388ed3c 100644 --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c @@ -9,6 +9,8 @@ * more details. */ +#define pr_fmt(fmt) "ieee80211_crypt_wep: " fmt + #include <linux/module.h> #include <linux/init.h> #include <linux/slab.h> @@ -43,20 +45,16 @@ 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; } 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; } @@ -67,14 +65,12 @@ static void *prism2_wep_init(int keyidx) 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); - } - + pr_debug("could not allocate crypto API arc4\n"); + if (priv->tx_tfm) + crypto_free_blkcipher(priv->tx_tfm); + if (priv->rx_tfm) + crypto_free_blkcipher(priv->rx_tfm); + kfree(priv); return NULL; } -- 1.9.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3 v3] Staging: rtl8192u: Simplify error check code at prism2_wep_init 2015-05-18 23:32 ` [PATCH 1/3 v3] Staging: rtl8192u: Simplify error check code at prism2_wep_init Pedro Marzo Perez @ 2015-05-19 5:34 ` Sudip Mukherjee 2015-05-19 22:49 ` pmarzo 2015-05-19 21:46 ` Dan Carpenter 2015-05-31 1:36 ` Greg KH 2 siblings, 1 reply; 11+ messages in thread From: Sudip Mukherjee @ 2015-05-19 5:34 UTC (permalink / raw) To: Pedro Marzo Perez Cc: gregkh, navyasri.tech, dilekuzulmez, joe, haticeerturk27, devel, linux-kernel, dan.carpenter On Tue, May 19, 2015 at 01:32:22AM +0200, Pedro Marzo Perez wrote: > Merge two pr_debug lines with literal strings splitted across several lines > into one single line, simplifying prism2_wep_init error check code. I would have split this patch into three. 1) introduce pr_fmt and remove "ieee80211_crypt_wep" from pr_debug 2) return NULL instead of that goto fail. 3) combine two pr_debug into a single at the fail block. But Dan can say if its ok. (I am still taking ideas from Dan about my patches) regards sudip > > Signed-off-by: Pedro Marzo Perez <marzo.pedro@gmail.com> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3 v3] Staging: rtl8192u: Simplify error check code at prism2_wep_init 2015-05-19 5:34 ` Sudip Mukherjee @ 2015-05-19 22:49 ` pmarzo 0 siblings, 0 replies; 11+ messages in thread From: pmarzo @ 2015-05-19 22:49 UTC (permalink / raw) To: Sudip Mukherjee Cc: gregkh, navyasri.tech, dilekuzulmez, joe, haticeerturk27, devel, linux-kernel, dan.carpenter On Tue, 2015-05-19 at 11:04 +0530, Sudip Mukherjee wrote: > On Tue, May 19, 2015 at 01:32:22AM +0200, Pedro Marzo Perez wrote: > > Merge two pr_debug lines with literal strings splitted across several lines > > into one single line, simplifying prism2_wep_init error check code. > > I would have split this patch into three. > 1) introduce pr_fmt and remove "ieee80211_crypt_wep" from pr_debug > 2) return NULL instead of that goto fail. > 3) combine two pr_debug into a single at the fail block. > I firmly agree with the "one logical change-one patch" rule but this seems to me a bit overkilling.Anyway I am a completely newbie sending kernel patches so you are probably right. > But Dan can say if its ok. (I am still taking ideas from Dan about my > patches) Sure. Dan, we need your wisdom! > > regards > sudip thanku for your comments sudip! regards pedro > > > > Signed-off-by: Pedro Marzo Perez <marzo.pedro@gmail.com> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3 v3] Staging: rtl8192u: Simplify error check code at prism2_wep_init 2015-05-18 23:32 ` [PATCH 1/3 v3] Staging: rtl8192u: Simplify error check code at prism2_wep_init Pedro Marzo Perez 2015-05-19 5:34 ` Sudip Mukherjee @ 2015-05-19 21:46 ` Dan Carpenter 2015-05-20 8:26 ` pmarzo 2015-05-31 1:36 ` Greg KH 2 siblings, 1 reply; 11+ messages in thread From: Dan Carpenter @ 2015-05-19 21:46 UTC (permalink / raw) To: Pedro Marzo Perez Cc: gregkh, navyasri.tech, dilekuzulmez, joe, haticeerturk27, devel, linux-kernel I was planning to leave this one for Greg but since you asked me to comment... This patch is ok as one patch. The subject is "clean up prism2_wep_init()" and that's one thing. Breaking it up into tiny patches would probably make it harder to review. On Tue, May 19, 2015 at 01:32:22AM +0200, Pedro Marzo Perez wrote: > Merge two pr_debug lines with literal strings splitted across several lines > into one single line, simplifying prism2_wep_init error check code. > > Signed-off-by: Pedro Marzo Perez <marzo.pedro@gmail.com> > --- > .../rtl8192u/ieee80211/ieee80211_crypt_wep.c | 22 +++++++++------------- > 1 file changed, 9 insertions(+), 13 deletions(-) > > diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c > index 0a17f84..388ed3c 100644 > --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c > +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c > @@ -9,6 +9,8 @@ > * more details. > */ > > +#define pr_fmt(fmt) "ieee80211_crypt_wep: " fmt Greg doesn't like pr_fmt() in driver code, use dev_dbg() and friends instead. I don't like debug output generally so I say just delete it. Especially in this case the debug output is pretty useless. > + > #include <linux/module.h> > #include <linux/init.h> > #include <linux/slab.h> > @@ -43,20 +45,16 @@ 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; This error handling is overly complicated. It's supposed to be you climb up a mountain to go hang gliding (return zero), but if you hit an error on the way to the talk you walk backwards the way you came to the bottom of the mountain. You see all the landmarks in reverse order and it makes you have a sad face. allocate priv allocate tx allocate rx return success; <-- made it to the top free tx <- walking back down. :( free priv It should be: 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; If this allocation succeeds then we've made it to the top. No need to call crypto_free_blkcipher(priv->rx_tfm) in this function. > @@ -67,14 +65,12 @@ static void *prism2_wep_init(int keyidx) > 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); > - } In the original code there isn't a nice error path down the mountain, it's all mixed together and not in any particular order. > - > + pr_debug("could not allocate crypto API arc4\n"); > + if (priv->tx_tfm) > + crypto_free_blkcipher(priv->tx_tfm); > + if (priv->rx_tfm) > + crypto_free_blkcipher(priv->rx_tfm); > + kfree(priv); > return NULL; free_tx: crypto_free_blkcipher(priv->tx_tfm); free_priv: kfree(priv); return NULL; regards, dan carpenter ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3 v3] Staging: rtl8192u: Simplify error check code at prism2_wep_init 2015-05-19 21:46 ` Dan Carpenter @ 2015-05-20 8:26 ` pmarzo 2015-05-20 9:03 ` Dan Carpenter 0 siblings, 1 reply; 11+ messages in thread From: pmarzo @ 2015-05-20 8:26 UTC (permalink / raw) To: Dan Carpenter Cc: gregkh, navyasri.tech, dilekuzulmez, joe, haticeerturk27, devel, linux-kernel On mié, 2015-05-20 at 00:46 +0300, Dan Carpenter wrote: > I was planning to leave this one for Greg but since you asked me to > comment... > > This patch is ok as one patch. The subject is "clean up > prism2_wep_init()" and that's one thing. Breaking it up into tiny > patches would probably make it harder to review. > > On Tue, May 19, 2015 at 01:32:22AM +0200, Pedro Marzo Perez wrote: > > Merge two pr_debug lines with literal strings splitted across several lines > > into one single line, simplifying prism2_wep_init error check code. > > > > Signed-off-by: Pedro Marzo Perez <marzo.pedro@gmail.com> > > --- > > .../rtl8192u/ieee80211/ieee80211_crypt_wep.c | 22 +++++++++------------- > > 1 file changed, 9 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c > > index 0a17f84..388ed3c 100644 > > --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c > > +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c > > @@ -9,6 +9,8 @@ > > * more details. > > */ > > > > +#define pr_fmt(fmt) "ieee80211_crypt_wep: " fmt > > Greg doesn't like pr_fmt() in driver code, use dev_dbg() and friends > instead. I don't like debug output generally so I say just delete it. > Especially in this case the debug output is pretty useless. dev_dbg needs the device parameter, I don't see a way to get the device pointer within this function. Anyway I think you are right, this debug output is not very useful, if Greg agrees I will delete the line. I am curious, why you dont like debug output? I usually have to adapt the kernel to run in ARM based boards and I find debug output really useful. > > + > > #include <linux/module.h> > > #include <linux/init.h> > > #include <linux/slab.h> > > @@ -43,20 +45,16 @@ 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; > > This error handling is overly complicated. It's supposed to be you > climb up a mountain to go hang gliding (return zero), but if you hit an > error on the way to the talk you walk backwards the way you came to the > bottom of the mountain. You see all the landmarks in reverse order and > it makes you have a sad face. Agree, it is still too complicated. The patch simplifies the original code but it is still not completely right, and it does not follow kernel error check style if error => goto > > allocate priv > allocate tx > allocate rx > > return success; <-- made it to the top > > free tx <- walking back down. :( > free priv > > It should be: > > 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; > > If this allocation succeeds then we've made it to the top. No need to > call crypto_free_blkcipher(priv->rx_tfm) in this function. Agree, this check is useless, will never be called > > > @@ -67,14 +65,12 @@ static void *prism2_wep_init(int keyidx) > > 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); > > - } > > In the original code there isn't a nice error path down the mountain, > it's all mixed together and not in any particular order. > > > - > > + pr_debug("could not allocate crypto API arc4\n"); > > + if (priv->tx_tfm) > > + crypto_free_blkcipher(priv->tx_tfm); > > + if (priv->rx_tfm) > > + crypto_free_blkcipher(priv->rx_tfm); > > + kfree(priv); > > return NULL; > > > free_tx: > crypto_free_blkcipher(priv->tx_tfm); > free_priv: > kfree(priv); > > return NULL; > I will send an V4 patch, I am glad you don't charge a fee per patch submitted and reviewed :-) Anyway I will wait a couple of days to Greg's comments, so hopefully V4 will be the last. > regards, > dan carpenter ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3 v3] Staging: rtl8192u: Simplify error check code at prism2_wep_init 2015-05-20 8:26 ` pmarzo @ 2015-05-20 9:03 ` Dan Carpenter 0 siblings, 0 replies; 11+ messages in thread From: Dan Carpenter @ 2015-05-20 9:03 UTC (permalink / raw) To: pmarzo Cc: devel, gregkh, haticeerturk27, linux-kernel, joe, dilekuzulmez, navyasri.tech On Wed, May 20, 2015 at 10:26:30AM +0200, pmarzo wrote: > On mié, 2015-05-20 at 00:46 +0300, Dan Carpenter wrote: > > I was planning to leave this one for Greg but since you asked me to > > comment... > > > > This patch is ok as one patch. The subject is "clean up > > prism2_wep_init()" and that's one thing. Breaking it up into tiny > > patches would probably make it harder to review. > > > > On Tue, May 19, 2015 at 01:32:22AM +0200, Pedro Marzo Perez wrote: > > > Merge two pr_debug lines with literal strings splitted across several lines > > > into one single line, simplifying prism2_wep_init error check code. > > > > > > Signed-off-by: Pedro Marzo Perez <marzo.pedro@gmail.com> > > > --- > > > .../rtl8192u/ieee80211/ieee80211_crypt_wep.c | 22 +++++++++------------- > > > 1 file changed, 9 insertions(+), 13 deletions(-) > > > > > > diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c > > > index 0a17f84..388ed3c 100644 > > > --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c > > > +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c > > > @@ -9,6 +9,8 @@ > > > * more details. > > > */ > > > > > > +#define pr_fmt(fmt) "ieee80211_crypt_wep: " fmt > > > > Greg doesn't like pr_fmt() in driver code, use dev_dbg() and friends > > instead. I don't like debug output generally so I say just delete it. > > Especially in this case the debug output is pretty useless. > dev_dbg needs the device parameter, I don't see a way to get the device > pointer within this function. Anyway I think you are right, this debug > output is not very useful, if Greg agrees I will delete the line. Greg doesn't care. Don't wait for Greg. He is a busy guy. Greg is very predictable so I can normally tell you which patches will be merged and which will not. > I am curious, why you dont like debug output? I usually have to adapt > the kernel to run in ARM based boards and I find debug output really > useful. These debug statements are never going to be printed in real life. They are a waste of RAM for no reason. They make the code more complicated which is why we are debating "how should we handle this thing?" They make the code slightly messier. They can introduce NULL deref bugs in code which never gets tested. It's not that all debug code is bad, but so often people do it without thinking because they think it's something they must do for every function. They think they are doing the right thing without realizing that sometimes they are making the code worse. Low level functions like crypto_alloc_blkcipher() should have debug code and stack dumps on error. It mostly does I think. I think it assumes that /lib/modules/ is not corrupt... regards, dan carpenter ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3 v3] Staging: rtl8192u: Simplify error check code at prism2_wep_init 2015-05-18 23:32 ` [PATCH 1/3 v3] Staging: rtl8192u: Simplify error check code at prism2_wep_init Pedro Marzo Perez 2015-05-19 5:34 ` Sudip Mukherjee 2015-05-19 21:46 ` Dan Carpenter @ 2015-05-31 1:36 ` Greg KH 2 siblings, 0 replies; 11+ messages in thread From: Greg KH @ 2015-05-31 1:36 UTC (permalink / raw) To: Pedro Marzo Perez Cc: navyasri.tech, dilekuzulmez, joe, haticeerturk27, devel, linux-kernel On Tue, May 19, 2015 at 01:32:22AM +0200, Pedro Marzo Perez wrote: > Merge two pr_debug lines with literal strings splitted across several lines > into one single line, simplifying prism2_wep_init error check code. > > Signed-off-by: Pedro Marzo Perez <marzo.pedro@gmail.com> > --- > .../rtl8192u/ieee80211/ieee80211_crypt_wep.c | 22 +++++++++------------- > 1 file changed, 9 insertions(+), 13 deletions(-) > > diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c > index 0a17f84..388ed3c 100644 > --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c > +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c > @@ -9,6 +9,8 @@ > * more details. > */ > > +#define pr_fmt(fmt) "ieee80211_crypt_wep: " fmt Like Dan said, don't do this in a driver. It's a driver, you almost always have access to the device being operated on, so use the dev_* functions. This is a network driver, so use the netdev_* functions, which are even better. But never the pr_* functions, that's not ok. Please fix up and resend. thanks, greg k-h ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/3 v3] Staging: rtl8192u: Remove two useless lines at ieee80211_wep_null 2015-05-18 23:32 [PATCH 0/3 v3] Staging: rtl8192u: Fix coding style issues at ieee80211_crypt_wep.c Pedro Marzo Perez 2015-05-18 23:32 ` [PATCH 1/3 v3] Staging: rtl8192u: Simplify error check code at prism2_wep_init Pedro Marzo Perez @ 2015-05-18 23:32 ` Pedro Marzo Perez 2015-05-31 1:37 ` Greg KH 2015-05-18 23:32 ` [PATCH 3/3 v3] Staging: rtl8192u: Correct include indentation and openning braces at new line Pedro Marzo Perez 2 siblings, 1 reply; 11+ messages in thread From: Pedro Marzo Perez @ 2015-05-18 23:32 UTC (permalink / raw) To: gregkh, navyasri.tech, dilekuzulmez, joe, haticeerturk27 Cc: devel, linux-kernel Removed 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 388ed3c..2ce7b54 100644 --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c @@ -289,6 +289,4 @@ void __exit ieee80211_crypto_wep_exit(void) void ieee80211_wep_null(void) { -// printk("============>%s()\n", __func__); - return; } -- 1.9.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3 v3] Staging: rtl8192u: Remove two useless lines at ieee80211_wep_null 2015-05-18 23:32 ` [PATCH 2/3 v3] Staging: rtl8192u: Remove two useless lines at ieee80211_wep_null Pedro Marzo Perez @ 2015-05-31 1:37 ` Greg KH 0 siblings, 0 replies; 11+ messages in thread From: Greg KH @ 2015-05-31 1:37 UTC (permalink / raw) To: Pedro Marzo Perez Cc: navyasri.tech, dilekuzulmez, joe, haticeerturk27, devel, linux-kernel On Tue, May 19, 2015 at 01:32:23AM +0200, Pedro Marzo Perez wrote: > Removed 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 388ed3c..2ce7b54 100644 > --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c > +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c > @@ -289,6 +289,4 @@ void __exit ieee80211_crypto_wep_exit(void) > > void ieee80211_wep_null(void) > { > -// printk("============>%s()\n", __func__); > - return; > } You now have a function that does nothing, why not just delete it entirely? ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/3 v3] Staging: rtl8192u: Correct include indentation and openning braces at new line 2015-05-18 23:32 [PATCH 0/3 v3] Staging: rtl8192u: Fix coding style issues at ieee80211_crypt_wep.c Pedro Marzo Perez 2015-05-18 23:32 ` [PATCH 1/3 v3] Staging: rtl8192u: Simplify error check code at prism2_wep_init Pedro Marzo Perez 2015-05-18 23:32 ` [PATCH 2/3 v3] Staging: rtl8192u: Remove two useless lines at ieee80211_wep_null Pedro Marzo Perez @ 2015-05-18 23:32 ` Pedro Marzo Perez 2 siblings, 0 replies; 11+ messages in thread From: Pedro Marzo Perez @ 2015-05-18 23:32 UTC (permalink / raw) To: gregkh, navyasri.tech, dilekuzulmez, joe, haticeerturk27 Cc: devel, linux-kernel 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 2ce7b54..4be468c 100644 --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c @@ -21,7 +21,7 @@ #include "ieee80211.h" #include <linux/crypto.h> - #include <linux/scatterlist.h> +#include <linux/scatterlist.h> #include <linux/crc32.h> MODULE_AUTHOR("Jouni Malinen"); @@ -138,9 +138,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); @@ -197,8 +195,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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2015-05-31 2:10 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-05-18 23:32 [PATCH 0/3 v3] Staging: rtl8192u: Fix coding style issues at ieee80211_crypt_wep.c Pedro Marzo Perez 2015-05-18 23:32 ` [PATCH 1/3 v3] Staging: rtl8192u: Simplify error check code at prism2_wep_init Pedro Marzo Perez 2015-05-19 5:34 ` Sudip Mukherjee 2015-05-19 22:49 ` pmarzo 2015-05-19 21:46 ` Dan Carpenter 2015-05-20 8:26 ` pmarzo 2015-05-20 9:03 ` Dan Carpenter 2015-05-31 1:36 ` Greg KH 2015-05-18 23:32 ` [PATCH 2/3 v3] Staging: rtl8192u: Remove two useless lines at ieee80211_wep_null Pedro Marzo Perez 2015-05-31 1:37 ` Greg KH 2015-05-18 23:32 ` [PATCH 3/3 v3] Staging: rtl8192u: Correct include indentation and openning braces at new line Pedro Marzo Perez
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.