diff mbox series

[05/17] staging: vt6655: changed variable name: pvRTS

Message ID 47da976cd02d262cebe520b21a0bf2451de6731b.1666740522.git.tanjubrunostar0@gmail.com
State New
Headers
Series staging: vt6655: a series of checkpatch fixes on the file: rxtx.c |

Commit Message

Tanjuate Brunostar Oct. 25, 2022, 11:37 p.m. UTC 
  change variable names pvRTS to meet the
        linux coding standard, as it says to avoid using camelCase naming
        style. Cought by checkpatch

Signed-off-by: Tanjuate Brunostar <tanjubrunostar0@gmail.com>
---
 drivers/staging/vt6655/rxtx.c | 56 +++++++++++++++++------------------
 1 file changed, 28 insertions(+), 28 deletions(-)

Comments

Greg KH Oct. 26, 2022, 1:51 p.m. UTC | #1 
On Tue, Oct 25, 2022 at 11:37:01PM +0000, Tanjuate Brunostar wrote:

Philipp has pointed out most of this already, but I'll just be specific
and say what isn't ok in all of these patches:

> 	change variable names pvRTS to meet the

"name" not "name"

>         linux coding standard, as it says to avoid using camelCase naming

"Linux" not "linux"

>         style. Cought by checkpatch

Why is this all indented?

Please do not do that, look at existing accepted changes in the git log
and match up what they look like.

But worst of all, you didn't really fix the variable name at all.  You
just appeased a tool that was trying to say "don't use camelCase, use
sane names".

> Signed-off-by: Tanjuate Brunostar <tanjubrunostar0@gmail.com>
> ---
>  drivers/staging/vt6655/rxtx.c | 56 +++++++++++++++++------------------
>  1 file changed, 28 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/staging/vt6655/rxtx.c b/drivers/staging/vt6655/rxtx.c
> index 2cac8f3882df..e97cba014adf 100644
> --- a/drivers/staging/vt6655/rxtx.c
> +++ b/drivers/staging/vt6655/rxtx.c
> @@ -87,7 +87,7 @@ static const unsigned short w_fb_opt_1[2][5] = {
>  /*---------------------  Static Functions  --------------------------*/
>  static void s_v_fill_rts_head(struct vnt_private *p_device,
>  			      unsigned char by_pkt_type,
> -			      void *pvRTS,
> +			      void *pv_rts,

"pvRTS" is using Hungarian Notation.  Look it up on Wikipedia for what
it means, and why people used to use it.

For us, we don't need that at all as the type of the variable is obvious
in the code and the compiler checks it.

So "pvRTS" is trying to say "this is a pointer to void called "RTS".

We don't care about the "describe the variable type in the name" thing,
so it should just be called "RTS", or better yet, "rts", right?

But then, step back.  Why is this a void pointer at all?  This is really
a structure of type struct vnt_rts_g_fb.  So why isn't that being passed
here instead?

So try to work on both, fixing up the names to be sane, and then,
getting rid of the void * stuff, to better reflect how data is flowing
around and what type that data is in.

thanks,

greg k-h
Tanjuate Brunostar Oct. 27, 2022, 6:12 a.m. UTC | #2 
On Wed, Oct 26, 2022 at 2:50 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Tue, Oct 25, 2022 at 11:37:01PM +0000, Tanjuate Brunostar wrote:
>
> Philipp has pointed out most of this already, but I'll just be specific
> and say what isn't ok in all of these patches:
>
> >       change variable names pvRTS to meet the
>
> "name" not "name"
>
> >         linux coding standard, as it says to avoid using camelCase naming
>
> "Linux" not "linux"
>
> >         style. Cought by checkpatch
>
> Why is this all indented?
>
> Please do not do that, look at existing accepted changes in the git log
> and match up what they look like.
>
> But worst of all, you didn't really fix the variable name at all.  You
> just appeased a tool that was trying to say "don't use camelCase, use
> sane names".
>
> > Signed-off-by: Tanjuate Brunostar <tanjubrunostar0@gmail.com>
> > ---
> >  drivers/staging/vt6655/rxtx.c | 56 +++++++++++++++++------------------
> >  1 file changed, 28 insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/staging/vt6655/rxtx.c b/drivers/staging/vt6655/rxtx.c
> > index 2cac8f3882df..e97cba014adf 100644
> > --- a/drivers/staging/vt6655/rxtx.c
> > +++ b/drivers/staging/vt6655/rxtx.c
> > @@ -87,7 +87,7 @@ static const unsigned short w_fb_opt_1[2][5] = {
> >  /*---------------------  Static Functions  --------------------------*/
> >  static void s_v_fill_rts_head(struct vnt_private *p_device,
> >                             unsigned char by_pkt_type,
> > -                           void *pvRTS,
> > +                           void *pv_rts,
>
> "pvRTS" is using Hungarian Notation.  Look it up on Wikipedia for what
> it means, and why people used to use it.
>
> For us, we don't need that at all as the type of the variable is obvious
> in the code and the compiler checks it.
>
> So "pvRTS" is trying to say "this is a pointer to void called "RTS".
>
> We don't care about the "describe the variable type in the name" thing,
> so it should just be called "RTS", or better yet, "rts", right?
>
> But then, step back.  Why is this a void pointer at all?  This is really
> a structure of type struct vnt_rts_g_fb.  So why isn't that being passed
> here instead?
>
> So try to work on both, fixing up the names to be sane, and then,
> getting rid of the void * stuff, to better reflect how data is flowing
> around and what type that data is in.
>
> thanks,
>
> greg k-h

I see. thank you for the pointers

Tanju
diff mbox series

Patch

diff --git a/drivers/staging/vt6655/rxtx.c b/drivers/staging/vt6655/rxtx.c
index 2cac8f3882df..e97cba014adf 100644
--- a/drivers/staging/vt6655/rxtx.c
+++ b/drivers/staging/vt6655/rxtx.c
@@ -87,7 +87,7 @@  static const unsigned short w_fb_opt_1[2][5] = {
 /*---------------------  Static Functions  --------------------------*/
 static void s_v_fill_rts_head(struct vnt_private *p_device,
 			      unsigned char by_pkt_type,
-			      void *pvRTS,
+			      void *pv_rts,
 			      unsigned int	cbFrameLength,
 			      bool bNeedAck,
 			      bool bDisCRC,
@@ -99,7 +99,7 @@  static void s_vGenerateTxParameter(struct vnt_private *p_device,
 				   unsigned char by_pkt_type,
 				   struct vnt_tx_fifo_head *,
 				   void *pvRrvTime,
-				   void *pvRTS,
+				   void *pv_rts,
 				   void *pvCTS,
 				   unsigned int	cbFrameSize,
 				   bool bNeedACK,
@@ -622,7 +622,7 @@  static __le16 s_uFillDataHead(struct vnt_private *p_device,
 
 static void s_v_fill_rts_head(struct vnt_private *p_device,
 			      unsigned char by_pkt_type,
-			      void *pvRTS,
+			      void *pv_rts,
 			      unsigned int cbFrameLength,
 			      bool bNeedAck,
 			      bool bDisCRC,
@@ -632,7 +632,7 @@  static void s_v_fill_rts_head(struct vnt_private *p_device,
 {
 	unsigned int uRTSFrameLen = 20;
 
-	if (!pvRTS)
+	if (!pv_rts)
 		return;
 
 	if (bDisCRC) {
@@ -648,7 +648,7 @@  static void s_v_fill_rts_head(struct vnt_private *p_device,
 	 */
 	if (by_pkt_type == PK_TYPE_11GB || by_pkt_type == PK_TYPE_11GA) {
 		if (byFBOption == AUTO_FB_NONE) {
-			struct vnt_rts_g *buf = pvRTS;
+			struct vnt_rts_g *buf = pv_rts;
 			/* Get SignalField, ServiceField & Length */
 			vnt_get_phy_field(p_device, uRTSFrameLen,
 					  p_device->byTopCCKBasicRate,
@@ -683,7 +683,7 @@  static void s_v_fill_rts_head(struct vnt_private *p_device,
 			ether_addr_copy(buf->data.ra, hdr->addr1);
 			ether_addr_copy(buf->data.ta, hdr->addr2);
 		} else {
-			struct vnt_rts_g_fb *buf = pvRTS;
+			struct vnt_rts_g_fb *buf = pv_rts;
 			/* Get SignalField, ServiceField & Length */
 			vnt_get_phy_field(p_device, uRTSFrameLen,
 					  p_device->byTopCCKBasicRate,
@@ -739,7 +739,7 @@  static void s_v_fill_rts_head(struct vnt_private *p_device,
 		} /* if (byFBOption == AUTO_FB_NONE) */
 	} else if (by_pkt_type == PK_TYPE_11A) {
 		if (byFBOption == AUTO_FB_NONE) {
-			struct vnt_rts_ab *buf = pvRTS;
+			struct vnt_rts_ab *buf = pv_rts;
 			/* Get SignalField, ServiceField & Length */
 			vnt_get_phy_field(p_device, uRTSFrameLen,
 					  p_device->byTopOFDMBasicRate,
@@ -759,7 +759,7 @@  static void s_v_fill_rts_head(struct vnt_private *p_device,
 			ether_addr_copy(buf->data.ra, hdr->addr1);
 			ether_addr_copy(buf->data.ta, hdr->addr2);
 		} else {
-			struct vnt_rts_a_fb *buf = pvRTS;
+			struct vnt_rts_a_fb *buf = pv_rts;
 			/* Get SignalField, ServiceField & Length */
 			vnt_get_phy_field(p_device, uRTSFrameLen,
 					  p_device->byTopOFDMBasicRate,
@@ -790,7 +790,7 @@  static void s_v_fill_rts_head(struct vnt_private *p_device,
 			ether_addr_copy(buf->data.ta, hdr->addr2);
 		}
 	} else if (by_pkt_type == PK_TYPE_11B) {
-		struct vnt_rts_ab *buf = pvRTS;
+		struct vnt_rts_ab *buf = pv_rts;
 		/* Get SignalField, ServiceField & Length */
 		vnt_get_phy_field(p_device, uRTSFrameLen,
 				  p_device->byTopCCKBasicRate,
@@ -918,7 +918,7 @@  static void s_vFillCTSHead(struct vnt_private *p_device,
  *      pTxDataHead     - Transmit Data Buffer
  *      pTxBufHead      - pTxBufHead
  *      pvRrvTime        - pvRrvTime
- *      pvRTS            - RTS Buffer
+ *      pv_rts            - RTS Buffer
  *      pCTS            - CTS Buffer
  *      cbFrameSize     - Transmit Data Length (Hdr+Payload+FCS)
  *      bNeedACK        - If need ACK
@@ -935,7 +935,7 @@  static void s_vGenerateTxParameter(struct vnt_private *p_device,
 				   unsigned char by_pkt_type,
 				   struct vnt_tx_fifo_head *tx_buffer_head,
 				   void *pvRrvTime,
-				   void *pvRTS,
+				   void *pv_rts,
 				   void *pvCTS,
 				   unsigned int cbFrameSize,
 				   bool bNeedACK,
@@ -961,7 +961,7 @@  static void s_vGenerateTxParameter(struct vnt_private *p_device,
 		return;
 
 	if (by_pkt_type == PK_TYPE_11GB || by_pkt_type == PK_TYPE_11GA) {
-		if (pvRTS) { /* RTS_need */
+		if (pv_rts) { /* RTS_need */
 			/* Fill RsvTime */
 			struct vnt_rrv_time_rts *buf = pvRrvTime;
 
@@ -977,7 +977,7 @@  static void s_vGenerateTxParameter(struct vnt_private *p_device,
 								p_device->byTopCCKBasicRate,
 								bNeedACK);
 
-			s_v_fill_rts_head(p_device, by_pkt_type, pvRTS, cbFrameSize, bNeedACK,
+			s_v_fill_rts_head(p_device, by_pkt_type, pv_rts, cbFrameSize, bNeedACK,
 					  bDisCRC, psEthHeader, wCurrentRate, byFBOption);
 		} else {/* RTS_needless, PCF mode */
 			struct vnt_rrv_time_cts *buf = pvRrvTime;
@@ -995,7 +995,7 @@  static void s_vGenerateTxParameter(struct vnt_private *p_device,
 				       bDisCRC, wCurrentRate, byFBOption);
 		}
 	} else if (by_pkt_type == PK_TYPE_11A) {
-		if (pvRTS) {/* RTS_need, non PCF mode */
+		if (pv_rts) {/* RTS_need, non PCF mode */
 			struct vnt_rrv_time_ab *buf = pvRrvTime;
 
 			buf->rts_rrv_time = get_rtscts_time(p_device, 2, by_pkt_type, cbFrameSize,
@@ -1004,16 +1004,16 @@  static void s_vGenerateTxParameter(struct vnt_private *p_device,
 							      wCurrentRate, bNeedACK);
 
 			/* Fill RTS */
-			s_v_fill_rts_head(p_device, by_pkt_type, pvRTS, cbFrameSize, bNeedACK,
+			s_v_fill_rts_head(p_device, by_pkt_type, pv_rts, cbFrameSize, bNeedACK,
 					  bDisCRC, psEthHeader, wCurrentRate, byFBOption);
-		} else if (!pvRTS) {/* RTS_needless, non PCF mode */
+		} else if (!pv_rts) {/* RTS_needless, non PCF mode */
 			struct vnt_rrv_time_ab *buf = pvRrvTime;
 
 			buf->rrv_time = vnt_rxtx_rsvtime_le16(p_device, PK_TYPE_11A, cbFrameSize,
 							      wCurrentRate, bNeedACK);
 		}
 	} else if (by_pkt_type == PK_TYPE_11B) {
-		if (pvRTS) {/* RTS_need, non PCF mode */
+		if (pv_rts) {/* RTS_need, non PCF mode */
 			struct vnt_rrv_time_ab *buf = pvRrvTime;
 
 			buf->rts_rrv_time = get_rtscts_time(p_device, 0, by_pkt_type, cbFrameSize,
@@ -1022,7 +1022,7 @@  static void s_vGenerateTxParameter(struct vnt_private *p_device,
 							      wCurrentRate, bNeedACK);
 
 			/* Fill RTS */
-			s_v_fill_rts_head(p_device, by_pkt_type, pvRTS, cbFrameSize, bNeedACK,
+			s_v_fill_rts_head(p_device, by_pkt_type, pv_rts, cbFrameSize, bNeedACK,
 					  bDisCRC, psEthHeader, wCurrentRate, byFBOption);
 		} else { /* RTS_needless, non PCF mode */
 			struct vnt_rrv_time_ab *buf = pvRrvTime;
@@ -1061,7 +1061,7 @@  static unsigned int s_cbFillTxBufHead(struct vnt_private *p_device,
 	unsigned int cbHeaderLength = 0;
 	void *pvRrvTime = NULL;
 	struct vnt_mic_hdr *pMICHDR = NULL;
-	void *pvRTS = NULL;
+	void *pv_rts = NULL;
 	void *pvCTS = NULL;
 	void *pvTxDataHd = NULL;
 	unsigned short wTxBufSize;   /* FFinfo size */
@@ -1104,7 +1104,7 @@  static unsigned int s_cbFillTxBufHead(struct vnt_private *p_device,
 				pvRrvTime = (void *)(pbyTxBufferAddr + wTxBufSize);
 				pMICHDR = (struct vnt_mic_hdr *)(pbyTxBufferAddr + wTxBufSize +
 								 sizeof(struct vnt_rrv_time_rts));
-				pvRTS = (void *)(pbyTxBufferAddr + wTxBufSize +
+				pv_rts = (void *)(pbyTxBufferAddr + wTxBufSize +
 						 sizeof(struct vnt_rrv_time_rts) + cbMICHDR);
 				pvCTS = NULL;
 				pvTxDataHd = (void *)(pbyTxBufferAddr + wTxBufSize +
@@ -1117,7 +1117,7 @@  static unsigned int s_cbFillTxBufHead(struct vnt_private *p_device,
 				pvRrvTime = (void *)(pbyTxBufferAddr + wTxBufSize);
 				pMICHDR = (struct vnt_mic_hdr *)(pbyTxBufferAddr + wTxBufSize +
 								 sizeof(struct vnt_rrv_time_cts));
-				pvRTS = NULL;
+				pv_rts = NULL;
 				pvCTS = (void *)(pbyTxBufferAddr + wTxBufSize +
 						  sizeof(struct vnt_rrv_time_cts) + cbMICHDR);
 				pvTxDataHd = (void *)(pbyTxBufferAddr + wTxBufSize +
@@ -1133,7 +1133,7 @@  static unsigned int s_cbFillTxBufHead(struct vnt_private *p_device,
 				pvRrvTime = (void *)(pbyTxBufferAddr + wTxBufSize);
 				pMICHDR = (struct vnt_mic_hdr *)(pbyTxBufferAddr + wTxBufSize +
 								 sizeof(struct vnt_rrv_time_rts));
-				pvRTS = (void *)(pbyTxBufferAddr + wTxBufSize +
+				pv_rts = (void *)(pbyTxBufferAddr + wTxBufSize +
 						 sizeof(struct vnt_rrv_time_rts) + cbMICHDR);
 				pvCTS = NULL;
 				pvTxDataHd = (void *)(pbyTxBufferAddr + wTxBufSize +
@@ -1146,7 +1146,7 @@  static unsigned int s_cbFillTxBufHead(struct vnt_private *p_device,
 				pvRrvTime = (void *)(pbyTxBufferAddr + wTxBufSize);
 				pMICHDR = (struct vnt_mic_hdr *)(pbyTxBufferAddr + wTxBufSize +
 								 sizeof(struct vnt_rrv_time_cts));
-				pvRTS = NULL;
+				pv_rts = NULL;
 				pvCTS = (void *)(pbyTxBufferAddr + wTxBufSize +
 						 sizeof(struct vnt_rrv_time_cts) + cbMICHDR);
 				pvTxDataHd = (void  *)(pbyTxBufferAddr + wTxBufSize +
@@ -1164,7 +1164,7 @@  static unsigned int s_cbFillTxBufHead(struct vnt_private *p_device,
 				pvRrvTime = (void *)(pbyTxBufferAddr + wTxBufSize);
 				pMICHDR = (struct vnt_mic_hdr *)(pbyTxBufferAddr + wTxBufSize +
 								 sizeof(struct vnt_rrv_time_ab));
-				pvRTS = (void *)(pbyTxBufferAddr + wTxBufSize +
+				pv_rts = (void *)(pbyTxBufferAddr + wTxBufSize +
 						 sizeof(struct vnt_rrv_time_ab) + cbMICHDR);
 				pvCTS = NULL;
 				pvTxDataHd = (void *)(pbyTxBufferAddr + wTxBufSize +
@@ -1177,7 +1177,7 @@  static unsigned int s_cbFillTxBufHead(struct vnt_private *p_device,
 				pvRrvTime = (void *)(pbyTxBufferAddr + wTxBufSize);
 				pMICHDR = (struct vnt_mic_hdr *)(pbyTxBufferAddr + wTxBufSize +
 								 sizeof(struct vnt_rrv_time_ab));
-				pvRTS = NULL;
+				pv_rts = NULL;
 				pvCTS = NULL;
 				pvTxDataHd = (void *)(pbyTxBufferAddr + wTxBufSize +
 						      sizeof(struct vnt_rrv_time_ab) + cbMICHDR);
@@ -1190,7 +1190,7 @@  static unsigned int s_cbFillTxBufHead(struct vnt_private *p_device,
 				pvRrvTime = (void *)(pbyTxBufferAddr + wTxBufSize);
 				pMICHDR = (struct vnt_mic_hdr *)(pbyTxBufferAddr + wTxBufSize +
 								 sizeof(struct vnt_rrv_time_ab));
-				pvRTS = (void *)(pbyTxBufferAddr + wTxBufSize +
+				pv_rts = (void *)(pbyTxBufferAddr + wTxBufSize +
 						 sizeof(struct vnt_rrv_time_ab) + cbMICHDR);
 				pvCTS = NULL;
 				pvTxDataHd = (void *)(pbyTxBufferAddr + wTxBufSize +
@@ -1203,7 +1203,7 @@  static unsigned int s_cbFillTxBufHead(struct vnt_private *p_device,
 				pvRrvTime = (void *)(pbyTxBufferAddr + wTxBufSize);
 				pMICHDR = (struct vnt_mic_hdr *)(pbyTxBufferAddr + wTxBufSize +
 								 sizeof(struct vnt_rrv_time_ab));
-				pvRTS = NULL;
+				pv_rts = NULL;
 				pvCTS = NULL;
 				pvTxDataHd = (void *)(pbyTxBufferAddr + wTxBufSize +
 						      sizeof(struct vnt_rrv_time_ab) + cbMICHDR);
@@ -1218,7 +1218,7 @@  static unsigned int s_cbFillTxBufHead(struct vnt_private *p_device,
 	memset((void *)(pbyTxBufferAddr + wTxBufSize), 0, (cbHeaderLength - wTxBufSize));
 
 	/* Fill FIFO,RrvTime,RTS,and CTS */
-	s_vGenerateTxParameter(p_device, by_pkt_type, tx_buffer_head, pvRrvTime, pvRTS, pvCTS,
+	s_vGenerateTxParameter(p_device, by_pkt_type, tx_buffer_head, pvRrvTime, pv_rts, pvCTS,
 			       cbFrameSize, bNeedACK, uDMAIdx, hdr, p_device->wCurrentRate);
 	/* Fill DataHead */
 	uDuration = s_uFillDataHead(p_device, by_pkt_type, pvTxDataHd, cbFrameSize, uDMAIdx,