diff mbox series

net: replace ternary operator with min()

Message ID 4e5c1182.347.18404f42721.Coremail.wangkailong@jari.cn
State New
Headers
Series net: replace ternary operator with min() |

Commit Message

KaiLong Wang Oct. 23, 2022, 1:07 p.m. UTC 
  Fix the following coccicheck warning:

net/ipv4/igmp.c:2621: WARNING opportunity for min()
net/ipv4/igmp.c:2574: WARNING opportunity for min()
net/ipv4/ip_sockglue.c:285: WARNING opportunity for min()

Signed-off-by: KaiLong Wang <wangkailong@jari.cn>
---
 net/ipv4/igmp.c        | 4 ++--
 net/ipv4/ip_sockglue.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

Comments

kernel test robot Oct. 23, 2022, 3:47 p.m. UTC | #1 
Hi KaiLong,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]
[also build test WARNING on net/master linus/master v6.1-rc1 next-20221021]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/KaiLong-Wang/net-replace-ternary-operator-with-min/20221023-211341
patch link:    https://lore.kernel.org/r/4e5c1182.347.18404f42721.Coremail.wangkailong%40jari.cn
patch subject: [PATCH] net: replace ternary operator with min()
config: i386-randconfig-a013
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
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
        # https://github.com/intel-lab-lkp/linux/commit/f816f1331754a52709034b8a73ce44ffe4722b90
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review KaiLong-Wang/net-replace-ternary-operator-with-min/20221023-211341
        git checkout f816f1331754a52709034b8a73ce44ffe4722b90
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash net/ipv4/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> net/ipv4/igmp.c:2574:14: warning: comparison of distinct pointer types ('typeof (count) *' (aka 'int *') and 'typeof (msf->imsf_numsrc) *' (aka 'unsigned int *')) [-Wcompare-distinct-pointer-types]
           copycount = min(count, msf->imsf_numsrc);
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:45:19: note: expanded from macro 'min'
   #define min(x, y)       __careful_cmp(x, y, <)
                           ^~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:36:24: note: expanded from macro '__careful_cmp'
           __builtin_choose_expr(__safe_cmp(x, y), \
                                 ^~~~~~~~~~~~~~~~
   include/linux/minmax.h:26:4: note: expanded from macro '__safe_cmp'
                   (__typecheck(x, y) && __no_side_effects(x, y))
                    ^~~~~~~~~~~~~~~~~
   include/linux/minmax.h:20:28: note: expanded from macro '__typecheck'
           (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
                      ~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~
>> net/ipv4/igmp.c:2621:14: warning: comparison of distinct pointer types ('typeof (count) *' (aka 'int *') and 'typeof (gsf->gf_numsrc) *' (aka 'unsigned int *')) [-Wcompare-distinct-pointer-types]
           copycount = min(count, gsf->gf_numsrc);
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:45:19: note: expanded from macro 'min'
   #define min(x, y)       __careful_cmp(x, y, <)
                           ^~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:36:24: note: expanded from macro '__careful_cmp'
           __builtin_choose_expr(__safe_cmp(x, y), \
                                 ^~~~~~~~~~~~~~~~
   include/linux/minmax.h:26:4: note: expanded from macro '__safe_cmp'
                   (__typecheck(x, y) && __no_side_effects(x, y))
                    ^~~~~~~~~~~~~~~~~
   include/linux/minmax.h:20:28: note: expanded from macro '__typecheck'
           (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
                      ~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~
   2 warnings generated.


vim +2574 net/ipv4/igmp.c

  2441	
  2442	int ip_mc_msfilter(struct sock *sk, struct ip_msfilter *msf, int ifindex)
  2443	{
  2444		int err = 0;
  2445		struct ip_mreqn	imr;
  2446		__be32 addr = msf->imsf_multiaddr;
  2447		struct ip_mc_socklist *pmc;
  2448		struct in_device *in_dev;
  2449		struct inet_sock *inet = inet_sk(sk);
  2450		struct ip_sf_socklist *newpsl, *psl;
  2451		struct net *net = sock_net(sk);
  2452		int leavegroup = 0;
  2453	
  2454		if (!ipv4_is_multicast(addr))
  2455			return -EINVAL;
  2456		if (msf->imsf_fmode != MCAST_INCLUDE &&
  2457		    msf->imsf_fmode != MCAST_EXCLUDE)
  2458			return -EINVAL;
  2459	
  2460		ASSERT_RTNL();
  2461	
  2462		imr.imr_multiaddr.s_addr = msf->imsf_multiaddr;
  2463		imr.imr_address.s_addr = msf->imsf_interface;
  2464		imr.imr_ifindex = ifindex;
  2465		in_dev = ip_mc_find_dev(net, &imr);
  2466	
  2467		if (!in_dev) {
  2468			err = -ENODEV;
  2469			goto done;
  2470		}
  2471	
  2472		/* special case - (INCLUDE, empty) == LEAVE_GROUP */
  2473		if (msf->imsf_fmode == MCAST_INCLUDE && msf->imsf_numsrc == 0) {
  2474			leavegroup = 1;
  2475			goto done;
  2476		}
  2477	
  2478		for_each_pmc_rtnl(inet, pmc) {
  2479			if (pmc->multi.imr_multiaddr.s_addr == msf->imsf_multiaddr &&
  2480			    pmc->multi.imr_ifindex == imr.imr_ifindex)
  2481				break;
  2482		}
  2483		if (!pmc) {		/* must have a prior join */
  2484			err = -EINVAL;
  2485			goto done;
  2486		}
  2487		if (msf->imsf_numsrc) {
  2488			newpsl = sock_kmalloc(sk, struct_size(newpsl, sl_addr,
  2489							      msf->imsf_numsrc),
  2490					      GFP_KERNEL);
  2491			if (!newpsl) {
  2492				err = -ENOBUFS;
  2493				goto done;
  2494			}
  2495			newpsl->sl_max = newpsl->sl_count = msf->imsf_numsrc;
  2496			memcpy(newpsl->sl_addr, msf->imsf_slist_flex,
  2497			       flex_array_size(msf, imsf_slist_flex, msf->imsf_numsrc));
  2498			err = ip_mc_add_src(in_dev, &msf->imsf_multiaddr,
  2499				msf->imsf_fmode, newpsl->sl_count, newpsl->sl_addr, 0);
  2500			if (err) {
  2501				sock_kfree_s(sk, newpsl,
  2502					     struct_size(newpsl, sl_addr,
  2503							 newpsl->sl_max));
  2504				goto done;
  2505			}
  2506		} else {
  2507			newpsl = NULL;
  2508			(void) ip_mc_add_src(in_dev, &msf->imsf_multiaddr,
  2509					     msf->imsf_fmode, 0, NULL, 0);
  2510		}
  2511		psl = rtnl_dereference(pmc->sflist);
  2512		if (psl) {
  2513			(void) ip_mc_del_src(in_dev, &msf->imsf_multiaddr, pmc->sfmode,
  2514				psl->sl_count, psl->sl_addr, 0);
  2515			/* decrease mem now to avoid the memleak warning */
  2516			atomic_sub(struct_size(psl, sl_addr, psl->sl_max),
  2517				   &sk->sk_omem_alloc);
  2518		} else {
  2519			(void) ip_mc_del_src(in_dev, &msf->imsf_multiaddr, pmc->sfmode,
  2520				0, NULL, 0);
  2521		}
  2522		rcu_assign_pointer(pmc->sflist, newpsl);
  2523		if (psl)
  2524			kfree_rcu(psl, rcu);
  2525		pmc->sfmode = msf->imsf_fmode;
  2526		err = 0;
  2527	done:
  2528		if (leavegroup)
  2529			err = ip_mc_leave_group(sk, &imr);
  2530		return err;
  2531	}
  2532	int ip_mc_msfget(struct sock *sk, struct ip_msfilter *msf,
  2533			 sockptr_t optval, sockptr_t optlen)
  2534	{
  2535		int err, len, count, copycount, msf_size;
  2536		struct ip_mreqn	imr;
  2537		__be32 addr = msf->imsf_multiaddr;
  2538		struct ip_mc_socklist *pmc;
  2539		struct in_device *in_dev;
  2540		struct inet_sock *inet = inet_sk(sk);
  2541		struct ip_sf_socklist *psl;
  2542		struct net *net = sock_net(sk);
  2543	
  2544		ASSERT_RTNL();
  2545	
  2546		if (!ipv4_is_multicast(addr))
  2547			return -EINVAL;
  2548	
  2549		imr.imr_multiaddr.s_addr = msf->imsf_multiaddr;
  2550		imr.imr_address.s_addr = msf->imsf_interface;
  2551		imr.imr_ifindex = 0;
  2552		in_dev = ip_mc_find_dev(net, &imr);
  2553	
  2554		if (!in_dev) {
  2555			err = -ENODEV;
  2556			goto done;
  2557		}
  2558		err = -EADDRNOTAVAIL;
  2559	
  2560		for_each_pmc_rtnl(inet, pmc) {
  2561			if (pmc->multi.imr_multiaddr.s_addr == msf->imsf_multiaddr &&
  2562			    pmc->multi.imr_ifindex == imr.imr_ifindex)
  2563				break;
  2564		}
  2565		if (!pmc)		/* must have a prior join */
  2566			goto done;
  2567		msf->imsf_fmode = pmc->sfmode;
  2568		psl = rtnl_dereference(pmc->sflist);
  2569		if (!psl) {
  2570			count = 0;
  2571		} else {
  2572			count = psl->sl_count;
  2573		}
> 2574		copycount = min(count, msf->imsf_numsrc);
  2575		len = flex_array_size(psl, sl_addr, copycount);
  2576		msf->imsf_numsrc = count;
  2577		msf_size = IP_MSFILTER_SIZE(copycount);
  2578		if (copy_to_sockptr(optlen, &msf_size, sizeof(int)) ||
  2579		    copy_to_sockptr(optval, msf, IP_MSFILTER_SIZE(0))) {
  2580			return -EFAULT;
  2581		}
  2582		if (len &&
  2583		    copy_to_sockptr_offset(optval,
  2584					   offsetof(struct ip_msfilter, imsf_slist_flex),
  2585					   psl->sl_addr, len))
  2586			return -EFAULT;
  2587		return 0;
  2588	done:
  2589		return err;
  2590	}
  2591	
  2592	int ip_mc_gsfget(struct sock *sk, struct group_filter *gsf,
  2593			 sockptr_t optval, size_t ss_offset)
  2594	{
  2595		int i, count, copycount;
  2596		struct sockaddr_in *psin;
  2597		__be32 addr;
  2598		struct ip_mc_socklist *pmc;
  2599		struct inet_sock *inet = inet_sk(sk);
  2600		struct ip_sf_socklist *psl;
  2601	
  2602		ASSERT_RTNL();
  2603	
  2604		psin = (struct sockaddr_in *)&gsf->gf_group;
  2605		if (psin->sin_family != AF_INET)
  2606			return -EINVAL;
  2607		addr = psin->sin_addr.s_addr;
  2608		if (!ipv4_is_multicast(addr))
  2609			return -EINVAL;
  2610	
  2611		for_each_pmc_rtnl(inet, pmc) {
  2612			if (pmc->multi.imr_multiaddr.s_addr == addr &&
  2613			    pmc->multi.imr_ifindex == gsf->gf_interface)
  2614				break;
  2615		}
  2616		if (!pmc)		/* must have a prior join */
  2617			return -EADDRNOTAVAIL;
  2618		gsf->gf_fmode = pmc->sfmode;
  2619		psl = rtnl_dereference(pmc->sflist);
  2620		count = psl ? psl->sl_count : 0;
> 2621		copycount = min(count, gsf->gf_numsrc);
  2622		gsf->gf_numsrc = count;
  2623		for (i = 0; i < copycount; i++) {
  2624			struct sockaddr_storage ss;
  2625	
  2626			psin = (struct sockaddr_in *)&ss;
  2627			memset(&ss, 0, sizeof(ss));
  2628			psin->sin_family = AF_INET;
  2629			psin->sin_addr.s_addr = psl->sl_addr[i];
  2630			if (copy_to_sockptr_offset(optval, ss_offset,
  2631						   &ss, sizeof(ss)))
  2632				return -EFAULT;
  2633			ss_offset += sizeof(ss);
  2634		}
  2635		return 0;
  2636	}
  2637
kernel test robot Oct. 23, 2022, 4:07 p.m. UTC | #2 
Hi KaiLong,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]
[also build test WARNING on net/master linus/master v6.1-rc1 next-20221021]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/KaiLong-Wang/net-replace-ternary-operator-with-min/20221023-211341
patch link:    https://lore.kernel.org/r/4e5c1182.347.18404f42721.Coremail.wangkailong%40jari.cn
patch subject: [PATCH] net: replace ternary operator with min()
config: hexagon-randconfig-r041-20221023
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 791a7ae1ba3efd6bca96338e10ffde557ba83920)
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
        # https://github.com/intel-lab-lkp/linux/commit/f816f1331754a52709034b8a73ce44ffe4722b90
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review KaiLong-Wang/net-replace-ternary-operator-with-min/20221023-211341
        git checkout f816f1331754a52709034b8a73ce44ffe4722b90
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash net/ipv4/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from net/ipv4/igmp.c:79:
   In file included from include/linux/inet.h:42:
   In file included from include/net/net_namespace.h:43:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/hexagon/include/asm/io.h:334:
   include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __raw_readb(PCI_IOBASE + addr);
                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
   #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
                                                     ^
   In file included from net/ipv4/igmp.c:79:
   In file included from include/linux/inet.h:42:
   In file included from include/net/net_namespace.h:43:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/hexagon/include/asm/io.h:334:
   include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
                                                     ^
   In file included from net/ipv4/igmp.c:79:
   In file included from include/linux/inet.h:42:
   In file included from include/net/net_namespace.h:43:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/hexagon/include/asm/io.h:334:
   include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writeb(value, PCI_IOBASE + addr);
                               ~~~~~~~~~~ ^
   include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   net/ipv4/igmp.c:1912:6: warning: variable 'changerec' set but not used [-Wunused-but-set-variable]
           int     changerec = 0;
                   ^
>> net/ipv4/igmp.c:2574:14: warning: comparison of distinct pointer types ('typeof (count) *' (aka 'int *') and 'typeof (msf->imsf_numsrc) *' (aka 'unsigned int *')) [-Wcompare-distinct-pointer-types]
           copycount = min(count, msf->imsf_numsrc);
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:45:19: note: expanded from macro 'min'
   #define min(x, y)       __careful_cmp(x, y, <)
                           ^~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:36:24: note: expanded from macro '__careful_cmp'
           __builtin_choose_expr(__safe_cmp(x, y), \
                                 ^~~~~~~~~~~~~~~~
   include/linux/minmax.h:26:4: note: expanded from macro '__safe_cmp'
                   (__typecheck(x, y) && __no_side_effects(x, y))
                    ^~~~~~~~~~~~~~~~~
   include/linux/minmax.h:20:28: note: expanded from macro '__typecheck'
           (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
                      ~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~
>> net/ipv4/igmp.c:2621:14: warning: comparison of distinct pointer types ('typeof (count) *' (aka 'int *') and 'typeof (gsf->gf_numsrc) *' (aka 'unsigned int *')) [-Wcompare-distinct-pointer-types]
           copycount = min(count, gsf->gf_numsrc);
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:45:19: note: expanded from macro 'min'
   #define min(x, y)       __careful_cmp(x, y, <)
                           ^~~~~~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:36:24: note: expanded from macro '__careful_cmp'
           __builtin_choose_expr(__safe_cmp(x, y), \
                                 ^~~~~~~~~~~~~~~~
   include/linux/minmax.h:26:4: note: expanded from macro '__safe_cmp'
                   (__typecheck(x, y) && __no_side_effects(x, y))
                    ^~~~~~~~~~~~~~~~~
   include/linux/minmax.h:20:28: note: expanded from macro '__typecheck'
           (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
                      ~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~
   9 warnings generated.


vim +2574 net/ipv4/igmp.c

  2441	
  2442	int ip_mc_msfilter(struct sock *sk, struct ip_msfilter *msf, int ifindex)
  2443	{
  2444		int err = 0;
  2445		struct ip_mreqn	imr;
  2446		__be32 addr = msf->imsf_multiaddr;
  2447		struct ip_mc_socklist *pmc;
  2448		struct in_device *in_dev;
  2449		struct inet_sock *inet = inet_sk(sk);
  2450		struct ip_sf_socklist *newpsl, *psl;
  2451		struct net *net = sock_net(sk);
  2452		int leavegroup = 0;
  2453	
  2454		if (!ipv4_is_multicast(addr))
  2455			return -EINVAL;
  2456		if (msf->imsf_fmode != MCAST_INCLUDE &&
  2457		    msf->imsf_fmode != MCAST_EXCLUDE)
  2458			return -EINVAL;
  2459	
  2460		ASSERT_RTNL();
  2461	
  2462		imr.imr_multiaddr.s_addr = msf->imsf_multiaddr;
  2463		imr.imr_address.s_addr = msf->imsf_interface;
  2464		imr.imr_ifindex = ifindex;
  2465		in_dev = ip_mc_find_dev(net, &imr);
  2466	
  2467		if (!in_dev) {
  2468			err = -ENODEV;
  2469			goto done;
  2470		}
  2471	
  2472		/* special case - (INCLUDE, empty) == LEAVE_GROUP */
  2473		if (msf->imsf_fmode == MCAST_INCLUDE && msf->imsf_numsrc == 0) {
  2474			leavegroup = 1;
  2475			goto done;
  2476		}
  2477	
  2478		for_each_pmc_rtnl(inet, pmc) {
  2479			if (pmc->multi.imr_multiaddr.s_addr == msf->imsf_multiaddr &&
  2480			    pmc->multi.imr_ifindex == imr.imr_ifindex)
  2481				break;
  2482		}
  2483		if (!pmc) {		/* must have a prior join */
  2484			err = -EINVAL;
  2485			goto done;
  2486		}
  2487		if (msf->imsf_numsrc) {
  2488			newpsl = sock_kmalloc(sk, struct_size(newpsl, sl_addr,
  2489							      msf->imsf_numsrc),
  2490					      GFP_KERNEL);
  2491			if (!newpsl) {
  2492				err = -ENOBUFS;
  2493				goto done;
  2494			}
  2495			newpsl->sl_max = newpsl->sl_count = msf->imsf_numsrc;
  2496			memcpy(newpsl->sl_addr, msf->imsf_slist_flex,
  2497			       flex_array_size(msf, imsf_slist_flex, msf->imsf_numsrc));
  2498			err = ip_mc_add_src(in_dev, &msf->imsf_multiaddr,
  2499				msf->imsf_fmode, newpsl->sl_count, newpsl->sl_addr, 0);
  2500			if (err) {
  2501				sock_kfree_s(sk, newpsl,
  2502					     struct_size(newpsl, sl_addr,
  2503							 newpsl->sl_max));
  2504				goto done;
  2505			}
  2506		} else {
  2507			newpsl = NULL;
  2508			(void) ip_mc_add_src(in_dev, &msf->imsf_multiaddr,
  2509					     msf->imsf_fmode, 0, NULL, 0);
  2510		}
  2511		psl = rtnl_dereference(pmc->sflist);
  2512		if (psl) {
  2513			(void) ip_mc_del_src(in_dev, &msf->imsf_multiaddr, pmc->sfmode,
  2514				psl->sl_count, psl->sl_addr, 0);
  2515			/* decrease mem now to avoid the memleak warning */
  2516			atomic_sub(struct_size(psl, sl_addr, psl->sl_max),
  2517				   &sk->sk_omem_alloc);
  2518		} else {
  2519			(void) ip_mc_del_src(in_dev, &msf->imsf_multiaddr, pmc->sfmode,
  2520				0, NULL, 0);
  2521		}
  2522		rcu_assign_pointer(pmc->sflist, newpsl);
  2523		if (psl)
  2524			kfree_rcu(psl, rcu);
  2525		pmc->sfmode = msf->imsf_fmode;
  2526		err = 0;
  2527	done:
  2528		if (leavegroup)
  2529			err = ip_mc_leave_group(sk, &imr);
  2530		return err;
  2531	}
  2532	int ip_mc_msfget(struct sock *sk, struct ip_msfilter *msf,
  2533			 sockptr_t optval, sockptr_t optlen)
  2534	{
  2535		int err, len, count, copycount, msf_size;
  2536		struct ip_mreqn	imr;
  2537		__be32 addr = msf->imsf_multiaddr;
  2538		struct ip_mc_socklist *pmc;
  2539		struct in_device *in_dev;
  2540		struct inet_sock *inet = inet_sk(sk);
  2541		struct ip_sf_socklist *psl;
  2542		struct net *net = sock_net(sk);
  2543	
  2544		ASSERT_RTNL();
  2545	
  2546		if (!ipv4_is_multicast(addr))
  2547			return -EINVAL;
  2548	
  2549		imr.imr_multiaddr.s_addr = msf->imsf_multiaddr;
  2550		imr.imr_address.s_addr = msf->imsf_interface;
  2551		imr.imr_ifindex = 0;
  2552		in_dev = ip_mc_find_dev(net, &imr);
  2553	
  2554		if (!in_dev) {
  2555			err = -ENODEV;
  2556			goto done;
  2557		}
  2558		err = -EADDRNOTAVAIL;
  2559	
  2560		for_each_pmc_rtnl(inet, pmc) {
  2561			if (pmc->multi.imr_multiaddr.s_addr == msf->imsf_multiaddr &&
  2562			    pmc->multi.imr_ifindex == imr.imr_ifindex)
  2563				break;
  2564		}
  2565		if (!pmc)		/* must have a prior join */
  2566			goto done;
  2567		msf->imsf_fmode = pmc->sfmode;
  2568		psl = rtnl_dereference(pmc->sflist);
  2569		if (!psl) {
  2570			count = 0;
  2571		} else {
  2572			count = psl->sl_count;
  2573		}
> 2574		copycount = min(count, msf->imsf_numsrc);
  2575		len = flex_array_size(psl, sl_addr, copycount);
  2576		msf->imsf_numsrc = count;
  2577		msf_size = IP_MSFILTER_SIZE(copycount);
  2578		if (copy_to_sockptr(optlen, &msf_size, sizeof(int)) ||
  2579		    copy_to_sockptr(optval, msf, IP_MSFILTER_SIZE(0))) {
  2580			return -EFAULT;
  2581		}
  2582		if (len &&
  2583		    copy_to_sockptr_offset(optval,
  2584					   offsetof(struct ip_msfilter, imsf_slist_flex),
  2585					   psl->sl_addr, len))
  2586			return -EFAULT;
  2587		return 0;
  2588	done:
  2589		return err;
  2590	}
  2591	
  2592	int ip_mc_gsfget(struct sock *sk, struct group_filter *gsf,
  2593			 sockptr_t optval, size_t ss_offset)
  2594	{
  2595		int i, count, copycount;
  2596		struct sockaddr_in *psin;
  2597		__be32 addr;
  2598		struct ip_mc_socklist *pmc;
  2599		struct inet_sock *inet = inet_sk(sk);
  2600		struct ip_sf_socklist *psl;
  2601	
  2602		ASSERT_RTNL();
  2603	
  2604		psin = (struct sockaddr_in *)&gsf->gf_group;
  2605		if (psin->sin_family != AF_INET)
  2606			return -EINVAL;
  2607		addr = psin->sin_addr.s_addr;
  2608		if (!ipv4_is_multicast(addr))
  2609			return -EINVAL;
  2610	
  2611		for_each_pmc_rtnl(inet, pmc) {
  2612			if (pmc->multi.imr_multiaddr.s_addr == addr &&
  2613			    pmc->multi.imr_ifindex == gsf->gf_interface)
  2614				break;
  2615		}
  2616		if (!pmc)		/* must have a prior join */
  2617			return -EADDRNOTAVAIL;
  2618		gsf->gf_fmode = pmc->sfmode;
  2619		psl = rtnl_dereference(pmc->sflist);
  2620		count = psl ? psl->sl_count : 0;
> 2621		copycount = min(count, gsf->gf_numsrc);
  2622		gsf->gf_numsrc = count;
  2623		for (i = 0; i < copycount; i++) {
  2624			struct sockaddr_storage ss;
  2625	
  2626			psin = (struct sockaddr_in *)&ss;
  2627			memset(&ss, 0, sizeof(ss));
  2628			psin->sin_family = AF_INET;
  2629			psin->sin_addr.s_addr = psl->sl_addr[i];
  2630			if (copy_to_sockptr_offset(optval, ss_offset,
  2631						   &ss, sizeof(ss)))
  2632				return -EFAULT;
  2633			ss_offset += sizeof(ss);
  2634		}
  2635		return 0;
  2636	}
  2637
kernel test robot Oct. 23, 2022, 11:11 p.m. UTC | #3 
Hi KaiLong,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]
[also build test WARNING on net/master linus/master v6.1-rc1 next-20221021]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/KaiLong-Wang/net-replace-ternary-operator-with-min/20221023-211341
patch link:    https://lore.kernel.org/r/4e5c1182.347.18404f42721.Coremail.wangkailong%40jari.cn
patch subject: [PATCH] net: replace ternary operator with min()
config: loongarch-randconfig-s031-20221023
compiler: loongarch64-linux-gcc (GCC) 12.1.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.4-39-gce1a6720-dirty
        # https://github.com/intel-lab-lkp/linux/commit/f816f1331754a52709034b8a73ce44ffe4722b90
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review KaiLong-Wang/net-replace-ternary-operator-with-min/20221023-211341
        git checkout f816f1331754a52709034b8a73ce44ffe4722b90
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=loongarch SHELL=/bin/bash net/ipv4/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

sparse warnings: (new ones prefixed by >>)
   net/ipv4/igmp.c: note: in included file (through include/linux/igmp.h):
   include/uapi/linux/igmp.h:60:32: sparse: sparse: array of flexible structures
>> net/ipv4/igmp.c:2574:21: sparse: sparse: incompatible types in comparison expression (different signedness):
>> net/ipv4/igmp.c:2574:21: sparse:    int *
>> net/ipv4/igmp.c:2574:21: sparse:    unsigned int *
   net/ipv4/igmp.c:2621:21: sparse: sparse: incompatible types in comparison expression (different signedness):
   net/ipv4/igmp.c:2621:21: sparse:    int *
   net/ipv4/igmp.c:2621:21: sparse:    unsigned int *
   net/ipv4/igmp.c:2933:31: sparse: sparse: context imbalance in 'igmp_mcf_get_next' - unexpected unlock
   net/ipv4/igmp.c:2961:9: sparse: sparse: context imbalance in 'igmp_mcf_get_idx' - wrong count at exit
   net/ipv4/igmp.c:2978:9: sparse: sparse: context imbalance in 'igmp_mcf_seq_next' - wrong count at exit
   net/ipv4/igmp.c: note: in included file (through include/linux/rbtree.h, include/linux/mm_types.h, include/linux/buildid.h, ...):
   include/linux/rcupdate.h:781:9: sparse: sparse: context imbalance in 'igmp_mcf_seq_stop' - unexpected unlock

vim +2574 net/ipv4/igmp.c

  2441	
  2442	int ip_mc_msfilter(struct sock *sk, struct ip_msfilter *msf, int ifindex)
  2443	{
  2444		int err = 0;
  2445		struct ip_mreqn	imr;
  2446		__be32 addr = msf->imsf_multiaddr;
  2447		struct ip_mc_socklist *pmc;
  2448		struct in_device *in_dev;
  2449		struct inet_sock *inet = inet_sk(sk);
  2450		struct ip_sf_socklist *newpsl, *psl;
  2451		struct net *net = sock_net(sk);
  2452		int leavegroup = 0;
  2453	
  2454		if (!ipv4_is_multicast(addr))
  2455			return -EINVAL;
  2456		if (msf->imsf_fmode != MCAST_INCLUDE &&
  2457		    msf->imsf_fmode != MCAST_EXCLUDE)
  2458			return -EINVAL;
  2459	
  2460		ASSERT_RTNL();
  2461	
  2462		imr.imr_multiaddr.s_addr = msf->imsf_multiaddr;
  2463		imr.imr_address.s_addr = msf->imsf_interface;
  2464		imr.imr_ifindex = ifindex;
  2465		in_dev = ip_mc_find_dev(net, &imr);
  2466	
  2467		if (!in_dev) {
  2468			err = -ENODEV;
  2469			goto done;
  2470		}
  2471	
  2472		/* special case - (INCLUDE, empty) == LEAVE_GROUP */
  2473		if (msf->imsf_fmode == MCAST_INCLUDE && msf->imsf_numsrc == 0) {
  2474			leavegroup = 1;
  2475			goto done;
  2476		}
  2477	
  2478		for_each_pmc_rtnl(inet, pmc) {
  2479			if (pmc->multi.imr_multiaddr.s_addr == msf->imsf_multiaddr &&
  2480			    pmc->multi.imr_ifindex == imr.imr_ifindex)
  2481				break;
  2482		}
  2483		if (!pmc) {		/* must have a prior join */
  2484			err = -EINVAL;
  2485			goto done;
  2486		}
  2487		if (msf->imsf_numsrc) {
  2488			newpsl = sock_kmalloc(sk, struct_size(newpsl, sl_addr,
  2489							      msf->imsf_numsrc),
  2490					      GFP_KERNEL);
  2491			if (!newpsl) {
  2492				err = -ENOBUFS;
  2493				goto done;
  2494			}
  2495			newpsl->sl_max = newpsl->sl_count = msf->imsf_numsrc;
  2496			memcpy(newpsl->sl_addr, msf->imsf_slist_flex,
  2497			       flex_array_size(msf, imsf_slist_flex, msf->imsf_numsrc));
  2498			err = ip_mc_add_src(in_dev, &msf->imsf_multiaddr,
  2499				msf->imsf_fmode, newpsl->sl_count, newpsl->sl_addr, 0);
  2500			if (err) {
  2501				sock_kfree_s(sk, newpsl,
  2502					     struct_size(newpsl, sl_addr,
  2503							 newpsl->sl_max));
  2504				goto done;
  2505			}
  2506		} else {
  2507			newpsl = NULL;
  2508			(void) ip_mc_add_src(in_dev, &msf->imsf_multiaddr,
  2509					     msf->imsf_fmode, 0, NULL, 0);
  2510		}
  2511		psl = rtnl_dereference(pmc->sflist);
  2512		if (psl) {
  2513			(void) ip_mc_del_src(in_dev, &msf->imsf_multiaddr, pmc->sfmode,
  2514				psl->sl_count, psl->sl_addr, 0);
  2515			/* decrease mem now to avoid the memleak warning */
  2516			atomic_sub(struct_size(psl, sl_addr, psl->sl_max),
  2517				   &sk->sk_omem_alloc);
  2518		} else {
  2519			(void) ip_mc_del_src(in_dev, &msf->imsf_multiaddr, pmc->sfmode,
  2520				0, NULL, 0);
  2521		}
  2522		rcu_assign_pointer(pmc->sflist, newpsl);
  2523		if (psl)
  2524			kfree_rcu(psl, rcu);
  2525		pmc->sfmode = msf->imsf_fmode;
  2526		err = 0;
  2527	done:
  2528		if (leavegroup)
  2529			err = ip_mc_leave_group(sk, &imr);
  2530		return err;
  2531	}
  2532	int ip_mc_msfget(struct sock *sk, struct ip_msfilter *msf,
  2533			 sockptr_t optval, sockptr_t optlen)
  2534	{
  2535		int err, len, count, copycount, msf_size;
  2536		struct ip_mreqn	imr;
  2537		__be32 addr = msf->imsf_multiaddr;
  2538		struct ip_mc_socklist *pmc;
  2539		struct in_device *in_dev;
  2540		struct inet_sock *inet = inet_sk(sk);
  2541		struct ip_sf_socklist *psl;
  2542		struct net *net = sock_net(sk);
  2543	
  2544		ASSERT_RTNL();
  2545	
  2546		if (!ipv4_is_multicast(addr))
  2547			return -EINVAL;
  2548	
  2549		imr.imr_multiaddr.s_addr = msf->imsf_multiaddr;
  2550		imr.imr_address.s_addr = msf->imsf_interface;
  2551		imr.imr_ifindex = 0;
  2552		in_dev = ip_mc_find_dev(net, &imr);
  2553	
  2554		if (!in_dev) {
  2555			err = -ENODEV;
  2556			goto done;
  2557		}
  2558		err = -EADDRNOTAVAIL;
  2559	
  2560		for_each_pmc_rtnl(inet, pmc) {
  2561			if (pmc->multi.imr_multiaddr.s_addr == msf->imsf_multiaddr &&
  2562			    pmc->multi.imr_ifindex == imr.imr_ifindex)
  2563				break;
  2564		}
  2565		if (!pmc)		/* must have a prior join */
  2566			goto done;
  2567		msf->imsf_fmode = pmc->sfmode;
  2568		psl = rtnl_dereference(pmc->sflist);
  2569		if (!psl) {
  2570			count = 0;
  2571		} else {
  2572			count = psl->sl_count;
  2573		}
> 2574		copycount = min(count, msf->imsf_numsrc);
  2575		len = flex_array_size(psl, sl_addr, copycount);
  2576		msf->imsf_numsrc = count;
  2577		msf_size = IP_MSFILTER_SIZE(copycount);
  2578		if (copy_to_sockptr(optlen, &msf_size, sizeof(int)) ||
  2579		    copy_to_sockptr(optval, msf, IP_MSFILTER_SIZE(0))) {
  2580			return -EFAULT;
  2581		}
  2582		if (len &&
  2583		    copy_to_sockptr_offset(optval,
  2584					   offsetof(struct ip_msfilter, imsf_slist_flex),
  2585					   psl->sl_addr, len))
  2586			return -EFAULT;
  2587		return 0;
  2588	done:
  2589		return err;
  2590	}
  2591
Jakub Kicinski Oct. 24, 2022, 5:29 p.m. UTC | #4 
On Sun, 23 Oct 2022 21:07:00 +0800 (GMT+08:00) KaiLong Wang wrote:
> Fix the following coccicheck warning:
> 
> net/ipv4/igmp.c:2621: WARNING opportunity for min()
> net/ipv4/igmp.c:2574: WARNING opportunity for min()
> net/ipv4/ip_sockglue.c:285: WARNING opportunity for min()
> 
> Signed-off-by: KaiLong Wang <wangkailong@jari.cn>

Please don't send coccicheck fixes for net and drivers/net.
(And I mean *you* specifically shouldn't send them since you 
don't even build test the fix.)
diff mbox series

Patch

diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index 81be3e0f0e70..7939d8febb2b 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -2571,7 +2571,7 @@  int ip_mc_msfget(struct sock *sk, struct ip_msfilter *msf,
 	} else {
 		count = psl->sl_count;
 	}
-	copycount = count < msf->imsf_numsrc ? count : msf->imsf_numsrc;
+	copycount = min(count, msf->imsf_numsrc);
 	len = flex_array_size(psl, sl_addr, copycount);
 	msf->imsf_numsrc = count;
 	msf_size = IP_MSFILTER_SIZE(copycount);
@@ -2618,7 +2618,7 @@  int ip_mc_gsfget(struct sock *sk, struct group_filter *gsf,
 	gsf->gf_fmode = pmc->sfmode;
 	psl = rtnl_dereference(pmc->sflist);
 	count = psl ? psl->sl_count : 0;
-	copycount = count < gsf->gf_numsrc ? count : gsf->gf_numsrc;
+	copycount = min(count, gsf->gf_numsrc);
 	gsf->gf_numsrc = count;
 	for (i = 0; i < copycount; i++) {
 		struct sockaddr_storage ss;
diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index 6e19cad154f5..19ad37897227 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -282,7 +282,7 @@  int ip_cmsg_send(struct sock *sk, struct msghdr *msg, struct ipcm_cookie *ipc,
 			/* Our caller is responsible for freeing ipc->opt */
 			err = ip_options_get(net, &ipc->opt,
 					     KERNEL_SOCKPTR(CMSG_DATA(cmsg)),
-					     err < 40 ? err : 40);
+					     min(err, 40));
 			if (err)
 				return err;
 			break;