diff mbox

[OPW,kernel,09/13] staging: rtl8187se: fixup multi-line comment

Message ID 1382316061-31542-10-git-send-email-teobaluta@gmail.com
State New, archived
Headers show

Commit Message

Teodora Baluta Oct. 21, 2013, 12:40 a.m. UTC
This patch ensures that all multi-line comments are consistent with the
Linux kernel coding style for long (multi-line) comments.

Signed-off-by: Teodora Baluta <teobaluta@gmail.com>
---
 drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c |   40 ++++++++++++--------
 1 file changed, 24 insertions(+), 16 deletions(-)

Comments

Josh Triplett Oct. 21, 2013, 7:04 p.m. UTC | #1
On Mon, Oct 21, 2013 at 03:40:57AM +0300, Teodora Baluta wrote:
> This patch ensures that all multi-line comments are consistent with the
> Linux kernel coding style for long (multi-line) comments.
> 
> Signed-off-by: Teodora Baluta <teobaluta@gmail.com>

In a few cases, these are comments you introduced.  You should not
introduce a bug in an earlier commit of your series and then fix it in
a later commit of the same series.

> diff --git a/drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c b/drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c
> index 8c89209..367eb9b 100644
> --- a/drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c
> +++ b/drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c
> @@ -187,7 +187,7 @@ int ieee80211_encrypt_fragment(
>  	int res;
>  
>  	/*
> -	 * added to care about null crypt condition, to solve that system hangs
> +	 * Added to care about null crypt condition, to solve that system hangs
>  	 * when shared keys error
>  	 */
>  	if (!crypt || !crypt->ops)
> @@ -207,13 +207,15 @@ int ieee80211_encrypt_fragment(
>  		return -1;
>  	}
>  #endif
> -	/* To encrypt, frame format is:
> -	 * IV (4 bytes), clear payload (including SNAP), ICV (4 bytes) */
> -
> -	/* PR: FIXME: Copied from hostap. Check fragmentation/MSDU/MPDU
> -	 * encryption. */
> -	/* Host-based IEEE 802.11 fragmentation for TX is not yet supported, so
> -	 * call both MSDU and MPDU encryption functions from here. */
> +	/*
> +	 * To encrypt, frame format is:
> +	 * IV (4 bytes), clear payload (including SNAP), ICV (4 bytes)
> +	 *
> +	 * PR: FIXME: Copied from hostap. Check fragmentation/MSDU/MPDU
> +	 * encryption.
> +	 * Host-based IEEE 802.11 fragmentation for TX is not yet supported, so
> +	 * call both MSDU and MPDU encryption functions from here.
> +	 */
>  	atomic_inc(&crypt->refcnt);
>  	res = 0;
>  	if (crypt->ops->encrypt_msdu)
> @@ -336,7 +338,7 @@ int ieee80211_rtl_xmit(struct sk_buff *skb,
>  	/*
>  	 * If there is no driver handler to take the TXB, don't bother
>  	 * creating it...
> -	 * */
> +	 */
>  	if ((!ieee->hard_start_xmit &&
>  	     !(ieee->softmac_features & IEEE_SOFTMAC_TX_QUEUE)) ||
>  	    ((!ieee->softmac_data_hard_start_xmit &&
> @@ -456,8 +458,10 @@ int ieee80211_rtl_xmit(struct sk_buff *skb,
>  			bytes_per_frag -= crypt->ops->extra_prefix_len +
>  				crypt->ops->extra_postfix_len;
>  
> -		/* Number of fragments is the total bytes_per_frag /
> -		* payload_per_fragment */
> +		/*
> +		 * Number of fragments is the total bytes_per_frag /
> +		 * payload_per_fragment
> +		 */
>  		nr_frags = bytes / bytes_per_frag;
>  		bytes_last_frag = bytes % bytes_per_frag;
>  		if (bytes_last_frag)
> @@ -465,9 +469,11 @@ int ieee80211_rtl_xmit(struct sk_buff *skb,
>  		else
>  			bytes_last_frag = bytes_per_frag;
>  
> -		/* When we allocate the TXB we allocate enough space for the reserve
> -		* and full fragment bytes (bytes_per_frag doesn't include prefix,
> -		* postfix, header, FCS, etc.) */
> +		/*
> +		 * When we allocate the TXB we allocate enough space for the
> +		 * reserve and full fragment bytes (bytes_per_frag doesn't
> +		 * include prefix, postfix, header, FCS, etc.)
> +		 */
>  		txb = ieee80211_alloc_txb(nr_frags, frag_size, GFP_ATOMIC);
>  		if (unlikely(!txb)) {
>  			printk(KERN_WARNING "%s: Could not allocate TXB\n",
> @@ -486,8 +492,10 @@ int ieee80211_rtl_xmit(struct sk_buff *skb,
>  			frag_hdr = (struct ieee80211_hdr_3addrqos *)skb_put(skb_frag, hdr_len);
>  			memcpy(frag_hdr, &header, hdr_len);
>  
> -			/* If this is not the last fragment, then add the MOREFRAGS
> -			* bit to the frame control */
> +			/*
> +			 * If this is not the last fragment, then add the MOREFRAGS
> +			 * bit to the frame control
> +			 */
>  			if (i != nr_frags - 1) {
>  				frag_hdr->frame_ctl = cpu_to_le16(
>  					fc | IEEE80211_FCTL_MOREFRAGS);
> -- 
> 1.7.10.4
> 
> -- 
> You received this message because you are subscribed to the Google Groups "opw-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to opw-kernel+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
Teodora Baluta Oct. 21, 2013, 7:09 p.m. UTC | #2
On Monday, October 21, 2013 10:04:49 PM UTC+3, Josh Triplett wrote:
>
> On Mon, Oct 21, 2013 at 03:40:57AM +0300, Teodora Baluta wrote: 
> > This patch ensures that all multi-line comments are consistent with the 
> > Linux kernel coding style for long (multi-line) comments. 
> > 
> > Signed-off-by: Teodora Baluta <teob...@gmail.com <javascript:>> 
>
> In a few cases, these are comments you introduced.  You should not 
> introduce a bug in an earlier commit of your series and then fix it in 
> a later commit of the same series. 
>
>
Thank you very much for your review, Josh! What do you mean by comments 
introduced by me? By bug do you mean the /*.. **/ comment? Should I redo 
the patches on comments and resend them? Or maybe resend the whole series?

Thanks,
Teodora
 

> > diff --git a/drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c 
> b/drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c 
> > index 8c89209..367eb9b 100644 
> > --- a/drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c 
> > +++ b/drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c 
> > @@ -187,7 +187,7 @@ int ieee80211_encrypt_fragment( 
> >          int res; 
> >   
> >          /* 
> > -         * added to care about null crypt condition, to solve that 
> system hangs 
> > +         * Added to care about null crypt condition, to solve that 
> system hangs 
> >           * when shared keys error 
> >           */ 
> >          if (!crypt || !crypt->ops) 
> > @@ -207,13 +207,15 @@ int ieee80211_encrypt_fragment( 
> >                  return -1; 
> >          } 
> >  #endif 
> > -        /* To encrypt, frame format is: 
> > -         * IV (4 bytes), clear payload (including SNAP), ICV (4 bytes) 
> */ 
> > - 
> > -        /* PR: FIXME: Copied from hostap. Check fragmentation/MSDU/MPDU 
> > -         * encryption. */ 
> > -        /* Host-based IEEE 802.11 fragmentation for TX is not yet 
> supported, so 
> > -         * call both MSDU and MPDU encryption functions from here. */ 
> > +        /* 
> > +         * To encrypt, frame format is: 
> > +         * IV (4 bytes), clear payload (including SNAP), ICV (4 bytes) 
> > +         * 
> > +         * PR: FIXME: Copied from hostap. Check fragmentation/MSDU/MPDU 
> > +         * encryption. 
> > +         * Host-based IEEE 802.11 fragmentation for TX is not yet 
> supported, so 
> > +         * call both MSDU and MPDU encryption functions from here. 
> > +         */ 
> >          atomic_inc(&crypt->refcnt); 
> >          res = 0; 
> >          if (crypt->ops->encrypt_msdu) 
> > @@ -336,7 +338,7 @@ int ieee80211_rtl_xmit(struct sk_buff *skb, 
> >          /* 
> >           * If there is no driver handler to take the TXB, don't bother 
> >           * creating it... 
> > -         * */ 
> > +         */ 
> >          if ((!ieee->hard_start_xmit && 
> >               !(ieee->softmac_features & IEEE_SOFTMAC_TX_QUEUE)) || 
> >              ((!ieee->softmac_data_hard_start_xmit && 
> > @@ -456,8 +458,10 @@ int ieee80211_rtl_xmit(struct sk_buff *skb, 
> >                          bytes_per_frag -= crypt->ops->extra_prefix_len 
> + 
> >                                  crypt->ops->extra_postfix_len; 
> >   
> > -                /* Number of fragments is the total bytes_per_frag / 
> > -                * payload_per_fragment */ 
> > +                /* 
> > +                 * Number of fragments is the total bytes_per_frag / 
> > +                 * payload_per_fragment 
> > +                 */ 
> >                  nr_frags = bytes / bytes_per_frag; 
> >                  bytes_last_frag = bytes % bytes_per_frag; 
> >                  if (bytes_last_frag) 
> > @@ -465,9 +469,11 @@ int ieee80211_rtl_xmit(struct sk_buff *skb, 
> >                  else 
> >                          bytes_last_frag = bytes_per_frag; 
> >   
> > -                /* When we allocate the TXB we allocate enough space 
> for the reserve 
> > -                * and full fragment bytes (bytes_per_frag doesn't 
> include prefix, 
> > -                * postfix, header, FCS, etc.) */ 
> > +                /* 
> > +                 * When we allocate the TXB we allocate enough space 
> for the 
> > +                 * reserve and full fragment bytes (bytes_per_frag 
> doesn't 
> > +                 * include prefix, postfix, header, FCS, etc.) 
> > +                 */ 
> >                  txb = ieee80211_alloc_txb(nr_frags, frag_size, 
> GFP_ATOMIC); 
> >                  if (unlikely(!txb)) { 
> >                          printk(KERN_WARNING "%s: Could not allocate 
> TXB\n", 
> > @@ -486,8 +492,10 @@ int ieee80211_rtl_xmit(struct sk_buff *skb, 
> >                          frag_hdr = (struct ieee80211_hdr_3addrqos 
> *)skb_put(skb_frag, hdr_len); 
> >                          memcpy(frag_hdr, &header, hdr_len); 
> >   
> > -                        /* If this is not the last fragment, then add 
> the MOREFRAGS 
> > -                        * bit to the frame control */ 
> > +                        /* 
> > +                         * If this is not the last fragment, then add 
> the MOREFRAGS 
> > +                         * bit to the frame control 
> > +                         */ 
> >                          if (i != nr_frags - 1) { 
> >                                  frag_hdr->frame_ctl = cpu_to_le16( 
> >                                          fc | IEEE80211_FCTL_MOREFRAGS); 
> > -- 
> > 1.7.10.4 
> > 
> > -- 
> > You received this message because you are subscribed to the Google 
> Groups "opw-kernel" group. 
> > To unsubscribe from this group and stop receiving emails from it, send 
> an email to opw-kernel+...@googlegroups.com <javascript:>. 
> > For more options, visit https://groups.google.com/groups/opt_out. 
>
Josh Triplett Oct. 21, 2013, 8:01 p.m. UTC | #3
On Mon, Oct 21, 2013 at 12:09:33PM -0700, Teodora B?lu?? wrote:
> 
> 
> On Monday, October 21, 2013 10:04:49 PM UTC+3, Josh Triplett wrote:
> >
> > On Mon, Oct 21, 2013 at 03:40:57AM +0300, Teodora Baluta wrote: 
> > > This patch ensures that all multi-line comments are consistent with the 
> > > Linux kernel coding style for long (multi-line) comments. 
> > > 
> > > Signed-off-by: Teodora Baluta <teob...@gmail.com <javascript:>> 
> >
> > In a few cases, these are comments you introduced.  You should not 
> > introduce a bug in an earlier commit of your series and then fix it in 
> > a later commit of the same series. 
> >
> >
> Thank you very much for your review, Josh! What do you mean by comments 
> introduced by me? By bug do you mean the /*.. **/ comment? Should I redo 
> the patches on comments and resend them? Or maybe resend the whole series?

I'm saying that in an earlier commit you converted existing comments to
multi-line comments using the wrong style, and then in this commit you
fixed the style.  You should format the comment correctly to begin with
if you're already reformatting it.

You just need to redo the couple of patches I replied to with comments,
not the ones I already posted a Reviewed-by for.  Greg will probably
find it easier if you resend the whole series with the updated patches,
though.

- Josh Triplett
diff mbox

Patch

diff --git a/drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c b/drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c
index 8c89209..367eb9b 100644
--- a/drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c
+++ b/drivers/staging/rtl8187se/ieee80211/ieee80211_tx.c
@@ -187,7 +187,7 @@  int ieee80211_encrypt_fragment(
 	int res;
 
 	/*
-	 * added to care about null crypt condition, to solve that system hangs
+	 * Added to care about null crypt condition, to solve that system hangs
 	 * when shared keys error
 	 */
 	if (!crypt || !crypt->ops)
@@ -207,13 +207,15 @@  int ieee80211_encrypt_fragment(
 		return -1;
 	}
 #endif
-	/* To encrypt, frame format is:
-	 * IV (4 bytes), clear payload (including SNAP), ICV (4 bytes) */
-
-	/* PR: FIXME: Copied from hostap. Check fragmentation/MSDU/MPDU
-	 * encryption. */
-	/* Host-based IEEE 802.11 fragmentation for TX is not yet supported, so
-	 * call both MSDU and MPDU encryption functions from here. */
+	/*
+	 * To encrypt, frame format is:
+	 * IV (4 bytes), clear payload (including SNAP), ICV (4 bytes)
+	 *
+	 * PR: FIXME: Copied from hostap. Check fragmentation/MSDU/MPDU
+	 * encryption.
+	 * Host-based IEEE 802.11 fragmentation for TX is not yet supported, so
+	 * call both MSDU and MPDU encryption functions from here.
+	 */
 	atomic_inc(&crypt->refcnt);
 	res = 0;
 	if (crypt->ops->encrypt_msdu)
@@ -336,7 +338,7 @@  int ieee80211_rtl_xmit(struct sk_buff *skb,
 	/*
 	 * If there is no driver handler to take the TXB, don't bother
 	 * creating it...
-	 * */
+	 */
 	if ((!ieee->hard_start_xmit &&
 	     !(ieee->softmac_features & IEEE_SOFTMAC_TX_QUEUE)) ||
 	    ((!ieee->softmac_data_hard_start_xmit &&
@@ -456,8 +458,10 @@  int ieee80211_rtl_xmit(struct sk_buff *skb,
 			bytes_per_frag -= crypt->ops->extra_prefix_len +
 				crypt->ops->extra_postfix_len;
 
-		/* Number of fragments is the total bytes_per_frag /
-		* payload_per_fragment */
+		/*
+		 * Number of fragments is the total bytes_per_frag /
+		 * payload_per_fragment
+		 */
 		nr_frags = bytes / bytes_per_frag;
 		bytes_last_frag = bytes % bytes_per_frag;
 		if (bytes_last_frag)
@@ -465,9 +469,11 @@  int ieee80211_rtl_xmit(struct sk_buff *skb,
 		else
 			bytes_last_frag = bytes_per_frag;
 
-		/* When we allocate the TXB we allocate enough space for the reserve
-		* and full fragment bytes (bytes_per_frag doesn't include prefix,
-		* postfix, header, FCS, etc.) */
+		/*
+		 * When we allocate the TXB we allocate enough space for the
+		 * reserve and full fragment bytes (bytes_per_frag doesn't
+		 * include prefix, postfix, header, FCS, etc.)
+		 */
 		txb = ieee80211_alloc_txb(nr_frags, frag_size, GFP_ATOMIC);
 		if (unlikely(!txb)) {
 			printk(KERN_WARNING "%s: Could not allocate TXB\n",
@@ -486,8 +492,10 @@  int ieee80211_rtl_xmit(struct sk_buff *skb,
 			frag_hdr = (struct ieee80211_hdr_3addrqos *)skb_put(skb_frag, hdr_len);
 			memcpy(frag_hdr, &header, hdr_len);
 
-			/* If this is not the last fragment, then add the MOREFRAGS
-			* bit to the frame control */
+			/*
+			 * If this is not the last fragment, then add the MOREFRAGS
+			 * bit to the frame control
+			 */
 			if (i != nr_frags - 1) {
 				frag_hdr->frame_ctl = cpu_to_le16(
 					fc | IEEE80211_FCTL_MOREFRAGS);