Received-SPF: pass (google.com: domain of linux-kernel+bounces-100758-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Tue, 12 Mar 2024 13:25:09 -0700
From: Alison Schofield <alison.schofield@intel.com>
To: "Felix N. Kimbu" <felixkimbu1@gmail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org,
	outreachy@lists.linux.dev
Subject: Re: [PATCH v3] staging: wlan-ng: Rename 'foo' to 'rc' in p80211conv.c
Message-ID: <ZfC6JeVBa9weWBSU@aschofie-mobl2>
References: <ZfCM2RyeEpxooKz+@MOLeToid>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZfCM2RyeEpxooKz+@MOLeToid>

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
> 
>