linux-kernel - Re: [PATCH] staging: octeon: fix line over 80 characters

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 25 Jul 2017 06:30:25 -0700
From:   Joe Perches <joe@...ches.com>
To:     catalinnow@...il.com, gregkh@...uxfoundation.org,
        aaro.koskinen@....fi, davem@...emloft.net, johannes.berg@...el.com,
        stephen@...workplumber.org, tklauser@...tanz.ch,
        chuckleberryfinn@...il.com, arnd@...db.de
Cc:     devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] staging: octeon: fix line over 80 characters

On Tue, 2017-07-25 at 16:01 +0300, catalinnow@...il.com wrote:
> From: John Smith <catalinnow@...il.com>
> 
> ethernet-rx.c:
> 
> fix WARNING: line over 80 characters

Instead of just shutting up checkpatch, how about
restructuring the code a bit by adding a helper function
to this very long cvm_oct_poll?

> diff --git a/drivers/staging/octeon/ethernet-rx.c b/drivers/staging/octeon/ethernet-rx.c
[]
> @@ -323,7 +323,8 @@ static int cvm_oct_poll(struct oct_rx_group *rx_group, int budget)
>  						segment_size = len;
>  					/* Copy the data into the packet */
>  					skb_put_data(skb,
> -						     cvmx_phys_to_ptr(segment_ptr.s.addr),
> +						     cvmx_phys_to_ptr(
> +						     segment_ptr.s.addr),
>  						     segment_size);
>  					len -= segment_size;
>  					segment_ptr = next_ptr;


Something like (uncompiled, untested)
---
 drivers/staging/octeon/ethernet-rx.c | 78 ++++++++++++++++++------------------
 1 file changed, 40 insertions(+), 38 deletions(-)

diff --git a/drivers/staging/octeon/ethernet-rx.c b/drivers/staging/octeon/ethernet-rx.c
index 72baedefa0f1..7315c8150dcb 100644
--- a/drivers/staging/octeon/ethernet-rx.c
+++ b/drivers/staging/octeon/ethernet-rx.c
@@ -149,6 +149,45 @@ static inline int cvm_oct_check_rcv_error(cvmx_wqe_t *work)
 	return 0;
 }
 
+static void copy_segments_to_skb(cvmx_wqe_t *work, struct sk_buff *skb)
+{
+	int segments = work->word2.s.bufs;
+	union cvmx_buf_ptr segment_ptr = work->packet_ptr;
+	int len = work->word1.len;
+
+	while (segments--) {
+		union cvmx_buf_ptr next_ptr;
+
+		next_ptr = *(union cvmx_buf_ptr *)
+			cvmx_phys_to_ptr(segment_ptr.s.addr - 8);
+
+		/*
+		 * Octeon Errata PKI-100: The segment size is wrong.
+		 *
+		 * Until it is fixed, calculate the segment size based on
+		 * the packet pool buffer size.
+		 * When it is fixed, the following line should be replaced
+		 * with this one:
+		 * int segment_size = segment_ptr.s.size;
+		 */
+		int segment_size =
+			CVMX_FPA_PACKET_POOL_SIZE -
+			(segment_ptr.s.addr -
+			 (((segment_ptr.s.addr >> 7) -
+			   segment_ptr.s.back) << 7));
+
+		/* Don't copy more than what is left in the packet */
+		if (segment_size > len)
+			segment_size = len;
+
+		/* Copy the data into the packet */
+		skb_put_data(skb, cvmx_phys_to_ptr(segment_ptr.s.addr),
+			     segment_size);
+		len -= segment_size;
+		segment_ptr = next_ptr;
+	}
+}
+
 static int cvm_oct_poll(struct oct_rx_group *rx_group, int budget)
 {
 	const int	coreid = cvmx_get_core_num();
@@ -290,44 +329,7 @@ static int cvm_oct_poll(struct oct_rx_group *rx_group, int budget)
 				skb_put_data(skb, ptr, work->word1.len);
 				/* No packet buffers to free */
 			} else {
-				int segments = work->word2.s.bufs;
-				union cvmx_buf_ptr segment_ptr =
-				    work->packet_ptr;
-				int len = work->word1.len;
-
-				while (segments--) {
-					union cvmx_buf_ptr next_ptr =
-					    *(union cvmx_buf_ptr *)
-					      cvmx_phys_to_ptr(
-					      segment_ptr.s.addr - 8);
-
-			/*
-			 * Octeon Errata PKI-100: The segment size is
-			 * wrong. Until it is fixed, calculate the
-			 * segment size based on the packet pool
-			 * buffer size. When it is fixed, the
-			 * following line should be replaced with this
-			 * one: int segment_size =
-			 * segment_ptr.s.size;
-			 */
-					int segment_size =
-					    CVMX_FPA_PACKET_POOL_SIZE -
-					    (segment_ptr.s.addr -
-					     (((segment_ptr.s.addr >> 7) -
-					       segment_ptr.s.back) << 7));
-					/*
-					 * Don't copy more than what
-					 * is left in the packet.
-					 */
-					if (segment_size > len)
-						segment_size = len;
-					/* Copy the data into the packet */
-					skb_put_data(skb,
-						     cvmx_phys_to_ptr(segment_ptr.s.addr),
-						     segment_size);
-					len -= segment_size;
-					segment_ptr = next_ptr;
-				}
+				copy_segments_to_skb(work, skb);
 			}
 			packet_not_copied = 0;
 		}