* [PATCH 1/2] net: core: datagram.c: Fix use of assignment in if condition
@ 2021-03-11 10:34 Shubhankar Kuranagatti
2021-03-11 10:59 ` Eric Dumazet
2021-03-11 15:39 ` kernel test robot
0 siblings, 2 replies; 4+ messages in thread
From: Shubhankar Kuranagatti @ 2021-03-11 10:34 UTC (permalink / raw)
To: davem; +Cc: kuba, edumazet, willemb, netdev, linux-kernel, mchehab+huawei
The assignment inside the if condition has been changed to
initialising outside the if condition.
Signed-off-by: Shubhankar Kuranagatti <shubhankarvk@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] net: core: datagram.c: Fix use of assignment in if condition
2021-03-11 10:34 [PATCH 1/2] net: core: datagram.c: Fix use of assignment in if condition Shubhankar Kuranagatti
@ 2021-03-11 10:59 ` Eric Dumazet
2021-03-11 15:39 ` kernel test robot
1 sibling, 0 replies; 4+ messages in thread
From: Eric Dumazet @ 2021-03-11 10:59 UTC (permalink / raw)
To: Shubhankar Kuranagatti
Cc: David Miller, Jakub Kicinski, Willem de Bruijn, netdev, LKML,
mchehab+huawei
On Thu, Mar 11, 2021 at 11:34 AM Shubhankar Kuranagatti
<shubhankarvk@gmail.com> wrote:
>
> The assignment inside the if condition has been changed to
> initialising outside the if condition.
>
> Signed-off-by: Shubhankar Kuranagatti <shubhankarvk@gmail.com>
> ---
> 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;
>
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.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] net: core: datagram.c: Fix use of assignment in if condition
2021-03-11 10:34 [PATCH 1/2] net: core: datagram.c: Fix use of assignment in if condition Shubhankar Kuranagatti
@ 2021-03-11 15:39 ` kernel test robot
2021-03-11 15:39 ` kernel test robot
1 sibling, 0 replies; 4+ messages in thread
From: kernel test robot @ 2021-03-11 15:39 UTC (permalink / raw)
To: Shubhankar Kuranagatti, davem
Cc: kbuild-all, clang-built-linux, kuba, edumazet, willemb, netdev,
linux-kernel, mchehab+huawei
[-- Attachment #1: Type: text/plain, Size: 5664 bytes --]
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 <lkp@intel.com>
All errors (new ones prefixed by >>):
>> 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
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 37800 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] net: core: datagram.c: Fix use of assignment in if condition
@ 2021-03-11 15:39 ` kernel test robot
0 siblings, 0 replies; 4+ messages in thread
From: kernel test robot @ 2021-03-11 15:39 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 5823 bytes --]
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 <lkp@intel.com>
All errors (new ones prefixed by >>):
>> 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(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 37800 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-03-11 15:42 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-11 10:34 [PATCH 1/2] net: core: datagram.c: Fix use of assignment in if condition Shubhankar Kuranagatti
2021-03-11 10:59 ` Eric Dumazet
2021-03-11 15:39 ` kernel test robot
2021-03-11 15:39 ` kernel test robot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.