[PATCH v3] staging: wlan-ng: Rename 'foo' to 'rc' in p80211conv.c

[PATCH v3] staging: wlan-ng: Rename 'foo' to 'rc' in p80211conv.c

[Felix N. Kimbu]

Rename identifer 'foo' to 'rc' Suggested-by Alison Schofield in functions
skb_p80211_to_ether() and skb_ether_to_p80211().

Fix indentation necessitated by above rename Suggested-by Dan Carpenter
and Philipp Hortmann.

This change adds intuitive meaning to the idenfier, adhering to best
practices and coding style.

Also, driver has been included in the subject Suggested-by Philipp Hortmann.

Signed-off-by: Felix N. Kimbu <felixkimbu1@gmail.com>
---
Changes since v2:
- Fix wrong indentation introduced in v1
- Correct subject to include driver from v1

Changes since v1:
- Rename identifier 'foo' to 'decrypt_check'

 drivers/staging/wlan-ng/p80211conv.c | 30 ++++++++++++++--------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/wlan-ng/p80211conv.c b/drivers/staging/wlan-ng/p80211conv.c
index 8336435eccc2..7401a6cacb7f 100644
--- a/drivers/staging/wlan-ng/p80211conv.c
+++ b/drivers/staging/wlan-ng/p80211conv.c
@@ -93,7 +93,7 @@ int skb_ether_to_p80211(struct wlandevice *wlandev, u32 ethconv,
 	struct wlan_ethhdr e_hdr;
 	struct wlan_llc *e_llc;
 	struct wlan_snap *e_snap;
-	int foo;
+	int rc;
 
 	memcpy(&e_hdr, skb->data, sizeof(e_hdr));
 
@@ -185,14 +185,14 @@ int skb_ether_to_p80211(struct wlandevice *wlandev, u32 ethconv,
 		p80211_wep->data = kmalloc(skb->len, GFP_ATOMIC);
 		if (!p80211_wep->data)
 			return -ENOMEM;
-		foo = wep_encrypt(wlandev, skb->data, p80211_wep->data,
-				  skb->len,
-				  wlandev->hostwep & HOSTWEP_DEFAULTKEY_MASK,
-				  p80211_wep->iv, p80211_wep->icv);
-		if (foo) {
+		rc = wep_encrypt(wlandev, skb->data, p80211_wep->data,
+				 skb->len,
+				 wlandev->hostwep & HOSTWEP_DEFAULTKEY_MASK,
+				 p80211_wep->iv, p80211_wep->icv);
+		if (rc) {
 			netdev_warn(wlandev->netdev,
 				    "Host en-WEP failed, dropping frame (%d).\n",
-				    foo);
+				    rc);
 			kfree(p80211_wep->data);
 			return 2;
 		}
@@ -265,7 +265,7 @@ int skb_p80211_to_ether(struct wlandevice *wlandev, u32 ethconv,
 	struct wlan_llc *e_llc;
 	struct wlan_snap *e_snap;
 
-	int foo;
+	int rc;
 
 	payload_length = skb->len - WLAN_HDR_A3_LEN - WLAN_CRC_LEN;
 	payload_offset = WLAN_HDR_A3_LEN;
@@ -305,15 +305,15 @@ int skb_p80211_to_ether(struct wlandevice *wlandev, u32 ethconv,
 				   "WEP frame too short (%u).\n", skb->len);
 			return 1;
 		}
-		foo = wep_decrypt(wlandev, skb->data + payload_offset + 4,
-				  payload_length - 8, -1,
-				  skb->data + payload_offset,
-				  skb->data + payload_offset +
-				  payload_length - 4);
-		if (foo) {
+		rc = wep_decrypt(wlandev, skb->data + payload_offset + 4,
+				 payload_length - 8, -1,
+				 skb->data + payload_offset,
+				 skb->data + payload_offset +
+				 payload_length - 4);
+		if (rc) {
 			/* de-wep failed, drop skb. */
 			netdev_dbg(netdev, "Host de-WEP failed, dropping frame (%d).\n",
-				   foo);
+				   rc);
 			wlandev->rx.decrypt_err++;
 			return 2;
 		}
-- 
2.34.1

Re: [PATCH v3] staging: wlan-ng: Rename 'foo' to 'rc' in p80211conv.c

[Alison Schofield]

On Tue, Mar 12, 2024 at 06:11:53PM +0100, Felix N. Kimbu wrote:

Hi Felix,
Thanks for sending this v3 as a new patch. Now, I'm going to ask
you to create a v4 to straighten out a bit more of the patch
format.

The commit message below includes some revision history that only
belongs below the '---',  Your commit message will be with this
patch when it is committed, and the changelog will fall away.
The changelog benefits reviewers today, and it also is always
retrievable in Lore for folks curious about the patches journey.

So, I'll make some edits directly below indicating what I think
v4 should look like.

Caveat 1: I didn't test the patch. I expect it applies, compiles,
and passes checkpatch.  Double check that all on v4.

Caveat 2: I don't know if GregKH is going to accept this trivial
patch. That's OK.  By the time we get though v4 you will have 
applied a bunch of patching practices that you can take forward
to your next patches.

> Rename identifer 'foo' to 'rc' Suggested-by Alison Schofield in functions
> skb_p80211_to_ether() and skb_ether_to_p80211().
> 

Replace above with:
Rename identifier 'foo' to 'rc' in skb_p80211_to_ether() and
skb_ether_to_p80211() to match the common kernel coding style.

(Try to get the spell checker running with checkpatch to catch things
like the misspelled 'identifer' above.)


Delete from here:
> Fix indentation necessitated by above rename Suggested-by Dan Carpenter
> and Philipp Hortmann.
> 
> This change adds intuitive meaning to the idenfier, adhering to best
> practices and coding style.
> 
to here.

> 
> Signed-off-by: Felix N. Kimbu <felixkimbu1@gmail.com>
> ---

(The names in parens is optional. Folks often do it to make
it easier for reviewers to find what they last commented, and
also it is a bit of a nod, appreciating your reviewers)

Changes in v4:
- Remove changelog comments from commit log (AlisonS)

Changes in v3:
- Create a proper new patch revision (AlisonS)
- Use 'rc' instead of 'decrypt_check' (AlisonS)

Changes in v2:
- Fix wrong indentation introduced in v1 (DanC)
- Correct subject to include driver (PhilippH)


> 
>  drivers/staging/wlan-ng/p80211conv.c | 30 ++++++++++++++--------------
>  1 file changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/staging/wlan-ng/p80211conv.c b/drivers/staging/wlan-ng/p80211conv.c
> index 8336435eccc2..7401a6cacb7f 100644
> --- a/drivers/staging/wlan-ng/p80211conv.c
> +++ b/drivers/staging/wlan-ng/p80211conv.c
> @@ -93,7 +93,7 @@ int skb_ether_to_p80211(struct wlandevice *wlandev, u32 ethconv,
>  	struct wlan_ethhdr e_hdr;
>  	struct wlan_llc *e_llc;
>  	struct wlan_snap *e_snap;
> -	int foo;
> +	int rc;
>  
>  	memcpy(&e_hdr, skb->data, sizeof(e_hdr));
>  
> @@ -185,14 +185,14 @@ int skb_ether_to_p80211(struct wlandevice *wlandev, u32 ethconv,
>  		p80211_wep->data = kmalloc(skb->len, GFP_ATOMIC);
>  		if (!p80211_wep->data)
>  			return -ENOMEM;
> -		foo = wep_encrypt(wlandev, skb->data, p80211_wep->data,
> -				  skb->len,
> -				  wlandev->hostwep & HOSTWEP_DEFAULTKEY_MASK,
> -				  p80211_wep->iv, p80211_wep->icv);
> -		if (foo) {
> +		rc = wep_encrypt(wlandev, skb->data, p80211_wep->data,
> +				 skb->len,
> +				 wlandev->hostwep & HOSTWEP_DEFAULTKEY_MASK,
> +				 p80211_wep->iv, p80211_wep->icv);
> +		if (rc) {
>  			netdev_warn(wlandev->netdev,
>  				    "Host en-WEP failed, dropping frame (%d).\n",
> -				    foo);
> +				    rc);
>  			kfree(p80211_wep->data);
>  			return 2;
>  		}
> @@ -265,7 +265,7 @@ int skb_p80211_to_ether(struct wlandevice *wlandev, u32 ethconv,
>  	struct wlan_llc *e_llc;
>  	struct wlan_snap *e_snap;
>  
> -	int foo;
> +	int rc;
>  
>  	payload_length = skb->len - WLAN_HDR_A3_LEN - WLAN_CRC_LEN;
>  	payload_offset = WLAN_HDR_A3_LEN;
> @@ -305,15 +305,15 @@ int skb_p80211_to_ether(struct wlandevice *wlandev, u32 ethconv,
>  				   "WEP frame too short (%u).\n", skb->len);
>  			return 1;
>  		}
> -		foo = wep_decrypt(wlandev, skb->data + payload_offset + 4,
> -				  payload_length - 8, -1,
> -				  skb->data + payload_offset,
> -				  skb->data + payload_offset +
> -				  payload_length - 4);
> -		if (foo) {
> +		rc = wep_decrypt(wlandev, skb->data + payload_offset + 4,
> +				 payload_length - 8, -1,
> +				 skb->data + payload_offset,
> +				 skb->data + payload_offset +
> +				 payload_length - 4);
> +		if (rc) {
>  			/* de-wep failed, drop skb. */
>  			netdev_dbg(netdev, "Host de-WEP failed, dropping frame (%d).\n",
> -				   foo);
> +				   rc);
>  			wlandev->rx.decrypt_err++;
>  			return 2;
>  		}
> -- 
> 2.34.1
> 
> 

Re: [PATCH v3] staging: wlan-ng: Rename 'foo' to 'rc' in p80211conv.c

[Felix Kimbu]

On Tue, Mar 12, 2024 at 9:25 PM Alison Schofield
<alison.schofield@intel.com> wrote:

Greetings Alison,
Thanks for the review. I will work and resubmit the v4 as guided.
Regards

>
> On Tue, Mar 12, 2024 at 06:11:53PM +0100, Felix N. Kimbu wrote:
>
> Hi Felix,
> Thanks for sending this v3 as a new patch. Now, I'm going to ask
> you to create a v4 to straighten out a bit more of the patch
> format.
>
> The commit message below includes some revision history that only
> belongs below the '---',  Your commit message will be with this
> patch when it is committed, and the changelog will fall away.
> The changelog benefits reviewers today, and it also is always
> retrievable in Lore for folks curious about the patches journey.
>
> So, I'll make some edits directly below indicating what I think
> v4 should look like.
>
> Caveat 1: I didn't test the patch. I expect it applies, compiles,
> and passes checkpatch.  Double check that all on v4.
>
> Caveat 2: I don't know if GregKH is going to accept this trivial
> patch. That's OK.  By the time we get though v4 you will have
> applied a bunch of patching practices that you can take forward
> to your next patches.
>
> > Rename identifer 'foo' to 'rc' Suggested-by Alison Schofield in functions
> > skb_p80211_to_ether() and skb_ether_to_p80211().
> >
>
> Replace above with:
> Rename identifier 'foo' to 'rc' in skb_p80211_to_ether() and
> skb_ether_to_p80211() to match the common kernel coding style.
>
> (Try to get the spell checker running with checkpatch to catch things
> like the misspelled 'identifer' above.)
>
>
> Delete from here:
> > Fix indentation necessitated by above rename Suggested-by Dan Carpenter
> > and Philipp Hortmann.
> >
> > This change adds intuitive meaning to the idenfier, adhering to best
> > practices and coding style.
> >
> to here.
>
> >
> > Signed-off-by: Felix N. Kimbu <felixkimbu1@gmail.com>
> > ---
>
> (The names in parens is optional. Folks often do it to make
> it easier for reviewers to find what they last commented, and
> also it is a bit of a nod, appreciating your reviewers)
>
> Changes in v4:
> - Remove changelog comments from commit log (AlisonS)
>
> Changes in v3:
> - Create a proper new patch revision (AlisonS)
> - Use 'rc' instead of 'decrypt_check' (AlisonS)
>
> Changes in v2:
> - Fix wrong indentation introduced in v1 (DanC)
> - Correct subject to include driver (PhilippH)
>
>