Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <624a11a3-2461-6c8f-155d-e76e5e3e471e@gmail.com>
Date:   Sat, 1 Apr 2023 23:20:36 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.9.0
Subject: Re: [PATCH 3/5] staging: rtl8192e: avoid camel-case
 <dot11RSNAStatsCCMPFormatErrors>
To:     Khadija Kamran <kamrankhadijadj@gmail.com>,
        outreachy@lists.linux.dev
Cc:     gregkh@linuxfoundation.org, linux-staging@lists.linux.dev,
        linux-kernel@vger.kernel.org, Deepak R Varma <drv@mailo.com>
References: <cover.1680375200.git.kamrankhadijadj@gmail.com>
 <7ed9ed4df19b1ab818fc4b3ec43083e1a051c38e.1680375200.git.kamrankhadijadj@gmail.com>
Content-Language: en-US
From:   Philipp Hortmann <philipp.g.hortmann@gmail.com>
In-Reply-To: <7ed9ed4df19b1ab818fc4b3ec43083e1a051c38e.1680375200.git.kamrankhadijadj@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 4/1/23 20:58, Khadija Kamran wrote:
> Linux kernel coding-style does not suggest mixed-case names. This issue
> is reported by checkpatch.
> 
> Fix variable name by changing it from camel-case to lower-case. In order
> to describe the variable name better, add a comment above it.
> 

I think you are changing from camel case to snake case.

> Suggested-by: Deepak R Varma <drv@mailo.com>
> Signed-off-by: Khadija Kamran <kamrankhadijadj@gmail.com>
> ---
>   drivers/staging/rtl8192e/rtllib_crypt_ccmp.c | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/rtl8192e/rtllib_crypt_ccmp.c b/drivers/staging/rtl8192e/rtllib_crypt_ccmp.c
> index 8d3067a6ccca..e8a3bc94efdc 100644
> --- a/drivers/staging/rtl8192e/rtllib_crypt_ccmp.c
> +++ b/drivers/staging/rtl8192e/rtllib_crypt_ccmp.c
> @@ -34,7 +34,8 @@ struct rtllib_ccmp_data {
>   	u8 tx_pn[CCMP_PN_LEN];
>   	u8 rx_pn[CCMP_PN_LEN];
>   
> -	u32 dot11RSNAStatsCCMPFormatErrors;
> +	/* .11 RSNA stats for CCMP */
> +	u32 ccmp_format_errors;
>   	u32 dot11RSNAStatsCCMPReplays;
>   	u32 dot11RSNAStatsCCMPDecryptErrors;


Is it obvious that CCMPFormatErrors is a part of dot11RSNAStats? OK. You 
added a comment. Often for me the issue is that the comment is only 
valid to one line or to more?

I would prefer something like: dot11rsna_stats_ccmp_format_err
This is keeping the information the author gave variable and I would 
understand better and see it in every place.

>   
> @@ -226,7 +227,7 @@ static int rtllib_ccmp_decrypt(struct sk_buff *skb, int hdr_len, void *priv)
>   	u8 pn[6];
>   
>   	if (skb->len < hdr_len + CCMP_HDR_LEN + CCMP_MIC_LEN) {
> -		key->dot11RSNAStatsCCMPFormatErrors++;
> +		key->ccmp_format_errors++;
>   		return -1;
>   	}
>   
> @@ -238,7 +239,7 @@ static int rtllib_ccmp_decrypt(struct sk_buff *skb, int hdr_len, void *priv)
>   			pr_debug("CCMP: received packet without ExtIV flag from %pM\n",
>   				 hdr->addr2);
>   		}
> -		key->dot11RSNAStatsCCMPFormatErrors++;
> +		key->ccmp_format_errors++;
>   		return -2;
>   	}
>   	keyidx >>= 6;
> @@ -374,7 +375,7 @@ static void rtllib_ccmp_print_stats(struct seq_file *m, void *priv)
>   		   "key[%d] alg=CCMP key_set=%d tx_pn=%pM rx_pn=%pM format_errors=%d replays=%d decrypt_errors=%d\n",
>   		   ccmp->key_idx, ccmp->key_set,
>   		   ccmp->tx_pn, ccmp->rx_pn,
> -		   ccmp->dot11RSNAStatsCCMPFormatErrors,
> +		   ccmp->ccmp_format_errors,
>   		   ccmp->dot11RSNAStatsCCMPReplays,
>   		   ccmp->dot11RSNAStatsCCMPDecryptErrors);
>   }