[PATCH 1/2] net: core: datagram.c: Fix use of assignment in if condition

The assignment inside the if condition has been changed to
initialising outside the if condition.

Signed-off-by: Shubhankar Kuranagatti <redacted>
---
 net/core/datagram.c | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/net/core/datagram.c b/net/core/datagram.c
index 15ab9ffb27fe..7b2204f102b7 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c

@@ -427,7 +427,8 @@ static int __skb_datagram_iter(const struct sk_buff *skb, int offset,
 		offset += n;
 		if (n != copy)
 			goto short_copy;
-		if ((len -= copy) == 0)
+		len -= copy
+		if ((len) == 0)
 			return 0;
 	}
 

@@ -439,7 +440,8 @@ static int __skb_datagram_iter(const struct sk_buff *skb, int offset,
 		WARN_ON(start > offset + len);
 
 		end = start + skb_frag_size(frag);
-		if ((copy = end - offset) > 0) {
+		copy = end - offset
+		if ((copy) > 0) {
 			struct page *page = skb_frag_page(frag);
 			u8 *vaddr = kmap(page);
 

@@ -452,7 +454,8 @@ static int __skb_datagram_iter(const struct sk_buff *skb, int offset,
 			offset += n;
 			if (n != copy)
 				goto short_copy;
-			if (!(len -= copy))
+			len -= copy
+			if (!(len))
 				return 0;
 		}
 		start = end;

@@ -464,13 +467,15 @@ static int __skb_datagram_iter(const struct sk_buff *skb, int offset,
 		WARN_ON(start > offset + len);
 
 		end = start + frag_iter->len;
-		if ((copy = end - offset) > 0) {
+		copy = end - offset;
+		if ((copy) > 0) {
 			if (copy > len)
 				copy = len;
 			if (__skb_datagram_iter(frag_iter, offset - start,
 						to, copy, fault_short, cb, data))
 				goto fault;
-			if ((len -= copy) == 0)
+			len -= copy
+			if ((len) == 0)
 				return 0;
 			offset += copy;
 		}

@@ -558,7 +563,8 @@ int skb_copy_datagram_from_iter(struct sk_buff *skb, int offset,
 			copy = len;
 		if (copy_from_iter(skb->data + offset, copy, from) != copy)
 			goto fault;
-		if ((len -= copy) == 0)
+		len -= copy;
+		if ((len) == 0)
 			return 0;
 		offset += copy;
 	}

@@ -571,7 +577,8 @@ int skb_copy_datagram_from_iter(struct sk_buff *skb, int offset,
 		WARN_ON(start > offset + len);
 
 		end = start + skb_frag_size(frag);
-		if ((copy = end - offset) > 0) {
+		copy = end - offset;
+		if ((copy) > 0) {
 			size_t copied;
 
 			if (copy > len)

@@ -581,8 +588,8 @@ int skb_copy_datagram_from_iter(struct sk_buff *skb, int offset,
 					  copy, from);
 			if (copied != copy)
 				goto fault;
-
-			if (!(len -= copy))
+			len -= copy
+			if (!(len))
 				return 0;
 			offset += copy;
 		}

@@ -595,14 +602,16 @@ int skb_copy_datagram_from_iter(struct sk_buff *skb, int offset,
 		WARN_ON(start > offset + len);
 
 		end = start + frag_iter->len;
-		if ((copy = end - offset) > 0) {
+		copy = end - offset;
+		if ((copy) > 0) {
 			if (copy > len)
 				copy = len;
 			if (skb_copy_datagram_from_iter(frag_iter,
 							offset - start,
 							from, copy))
 				goto fault;
-			if ((len -= copy) == 0)
+			len -= copy;
+			if ((len) == 0)
 				return 0;
 			offset += copy;
 		}

-- 
2.17.1

On Thu, Mar 11, 2021 at 11:34 AM Shubhankar Kuranagatti
[off-list ref] wrote:

Quite frankly I prefer the current style.

It also seems you have not even compiled your change, this is not a good start.

Lets keep reviewer time to review patches that really bring an improvement,
since stylistic changes like that make our backports more likely to
have conflicts.

Thanks.

Hi Shubhankar,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net-next/master]
[also build test ERROR on linus/master v5.12-rc2 next-20210311]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Shubhankar-Kuranagatti/net-core-datagram-c-Fix-use-of-assignment-in-if-condition/20210311-184120
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 34bb975126419e86bc3b95e200dc41de6c6ca69c
config: x86_64-randconfig-r025-20210311 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 574a9dabc63ba1e7a04c08d4bde2eacd61b44ce1)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/89811231e3ec535f3e5188fb8578535e13c1f1ba
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Shubhankar-Kuranagatti/net-core-datagram-c-Fix-use-of-assignment-in-if-condition/20210311-184120
        git checkout 89811231e3ec535f3e5188fb8578535e13c1f1ba
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <redacted>

All errors (new ones prefixed by >>):

quoted

net/core/datagram.c:430:14: error: expected ';' after expression

                   len -= copy
                              ^
                              ;
   net/core/datagram.c:443:22: error: expected ';' after expression
                   copy = end - offset
                                      ^
                                      ;
   net/core/datagram.c:457:15: error: expected ';' after expression
                           len -= copy
                                      ^
                                      ;
   net/core/datagram.c:477:15: error: expected ';' after expression
                           len -= copy
                                      ^
                                      ;
   net/core/datagram.c:591:15: error: expected ';' after expression
                           len -= copy
                                      ^
                                      ;
   5 errors generated.


vim +430 net/core/datagram.c

   406	
   407	INDIRECT_CALLABLE_DECLARE(static size_t simple_copy_to_iter(const void *addr,
   408							size_t bytes,
   409							void *data __always_unused,
   410							struct iov_iter *i));
   411	
   412	static int __skb_datagram_iter(const struct sk_buff *skb, int offset,
   413				       struct iov_iter *to, int len, bool fault_short,
   414				       size_t (*cb)(const void *, size_t, void *,
   415						    struct iov_iter *), void *data)
   416	{
   417		int start = skb_headlen(skb);
   418		int i, copy = start - offset, start_off = offset, n;
   419		struct sk_buff *frag_iter;
   420	
   421		/* Copy header. */
   422		if (copy > 0) {
   423			if (copy > len)
   424				copy = len;
   425			n = INDIRECT_CALL_1(cb, simple_copy_to_iter,
   426					    skb->data + offset, copy, data, to);
   427			offset += n;
   428			if (n != copy)
   429				goto short_copy;
 > 430			len -= copy
   431			if ((len) == 0)
   432				return 0;
   433		}
   434	
   435		/* Copy paged appendix. Hmm... why does this look so complicated? */
   436		for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
   437			int end;
   438			const skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
   439	
   440			WARN_ON(start > offset + len);
   441	
   442			end = start + skb_frag_size(frag);
   443			copy = end - offset
   444			if ((copy) > 0) {
   445				struct page *page = skb_frag_page(frag);
   446				u8 *vaddr = kmap(page);
   447	
   448				if (copy > len)
   449					copy = len;
   450				n = INDIRECT_CALL_1(cb, simple_copy_to_iter,
   451						vaddr + skb_frag_off(frag) + offset - start,
   452						copy, data, to);
   453				kunmap(page);
   454				offset += n;
   455				if (n != copy)
   456					goto short_copy;
   457				len -= copy
   458				if (!(len))
   459					return 0;
   460			}
   461			start = end;
   462		}
   463	
   464		skb_walk_frags(skb, frag_iter) {
   465			int end;
   466	
   467			WARN_ON(start > offset + len);
   468	
   469			end = start + frag_iter->len;
   470			copy = end - offset;
   471			if ((copy) > 0) {
   472				if (copy > len)
   473					copy = len;
   474				if (__skb_datagram_iter(frag_iter, offset - start,
   475							to, copy, fault_short, cb, data))
   476					goto fault;
   477				len -= copy
   478				if ((len) == 0)
   479					return 0;
   480				offset += copy;
   481			}
   482			start = end;
   483		}
   484		if (!len)
   485			return 0;
   486	
   487		/* This is not really a user copy fault, but rather someone
   488		 * gave us a bogus length on the skb.  We should probably
   489		 * print a warning here as it may indicate a kernel bug.
   490		 */
   491	
   492	fault:
   493		iov_iter_revert(to, offset - start_off);
   494		return -EFAULT;
   495	
   496	short_copy:
   497		if (fault_short || iov_iter_count(to))
   498			goto fault;
   499	
   500		return 0;
   501	}
   502	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org