Message ID | 20130531184438.21966f63@nehalam.linuxnetplumber.net |
---|---|
State | Changes Requested, archived |
Delegated to: | David Miller |
Headers | show |
On Sat, 01 Jun 2013 at 01:44 GMT, Stephen Hemminger <stephen@networkplumber.org> wrote: > Back in the old Unix lint days, it was common to put (void) in front > of functions when return value was being ignored. Now this is considered > unnecessary, catch up with the fashion. > None of the callers of ip_mc_leave_src() checks its return value, so I think we can just make it void. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Cong Wang <xiyou.wangcong@gmail.com> Date: Sat, 1 Jun 2013 02:51:05 +0000 (UTC) > On Sat, 01 Jun 2013 at 01:44 GMT, Stephen Hemminger <stephen@networkplumber.org> wrote: >> Back in the old Unix lint days, it was common to put (void) in front >> of functions when return value was being ignored. Now this is considered >> unnecessary, catch up with the fashion. >> > > None of the callers of ip_mc_leave_src() checks its return value, so I > think we can just make it void. So ip_mc_leave_src() calls ip_mc_del_src and all the call chains essentially ignore the one error case that ip_mc_del_src() has, namely "!in_dev". Most call chains validate that in_dev is not NULL before it ends up calling ip_mc_del_src(). However, ip_mc_leave_group() is peculiar, it seems to expect that sometimes "in_dev" can be NULL, and still signals -ENODEV when this happens even though it still did all of the adjustments to the socket's multicast group blobs. Someone really needs to audit this code before we start just pretending the error returns do not matter. I'm not applying this patch for now, there are too many loose ends. Thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--- a/net/ipv4/igmp.c 2013-05-31 08:35:07.838917459 -0700 +++ b/net/ipv4/igmp.c 2013-05-31 08:37:21.345081065 -0700 @@ -633,7 +633,7 @@ static void igmpv3_send_cr(struct in_dev if (!skb) return; - (void) igmpv3_sendpack(skb); + igmpv3_sendpack(skb); } static int igmp_send_report(struct in_device *in_dev, struct ip_mc_list *pmc, @@ -1733,7 +1733,7 @@ static int ip_mc_add_src(struct in_devic if (!delta) pmc->sfcount[sfmode]--; for (j=0; j<i; j++) - (void) ip_mc_del1_src(pmc, sfmode, &psfsrc[j]); + ip_mc_del1_src(pmc, sfmode, &psfsrc[j]); } else if (isexclude != (pmc->sfcount[MCAST_EXCLUDE] != 0)) { #ifdef CONFIG_IP_MULTICAST struct ip_sf_list *psf; @@ -1887,7 +1887,7 @@ int ip_mc_leave_group(struct sock *sk, s iml->multi.imr_address.s_addr) continue; - (void) ip_mc_leave_src(sk, iml, in_dev); + ip_mc_leave_src(sk, iml, in_dev); *imlp = iml->next_rcu; @@ -2106,18 +2106,18 @@ int ip_mc_msfilter(struct sock *sk, stru } } else { newpsl = NULL; - (void) ip_mc_add_src(in_dev, &msf->imsf_multiaddr, + ip_mc_add_src(in_dev, &msf->imsf_multiaddr, msf->imsf_fmode, 0, NULL, 0); } psl = rtnl_dereference(pmc->sflist); if (psl) { - (void) ip_mc_del_src(in_dev, &msf->imsf_multiaddr, pmc->sfmode, + ip_mc_del_src(in_dev, &msf->imsf_multiaddr, pmc->sfmode, psl->sl_count, psl->sl_addr, 0); /* decrease mem now to avoid the memleak warning */ atomic_sub(IP_SFLSIZE(psl->sl_max), &sk->sk_omem_alloc); kfree_rcu(psl, rcu); } else - (void) ip_mc_del_src(in_dev, &msf->imsf_multiaddr, pmc->sfmode, + ip_mc_del_src(in_dev, &msf->imsf_multiaddr, pmc->sfmode, 0, NULL, 0); rcu_assign_pointer(pmc->sflist, newpsl); pmc->sfmode = msf->imsf_fmode; @@ -2307,7 +2307,7 @@ void ip_mc_drop_socket(struct sock *sk) inet->mc_list = iml->next_rcu; in_dev = inetdev_by_index(net, iml->multi.imr_ifindex); - (void) ip_mc_leave_src(sk, iml, in_dev); + ip_mc_leave_src(sk, iml, in_dev); if (in_dev != NULL) ip_mc_dec_group(in_dev, iml->multi.imr_multiaddr.s_addr); /* decrease mem now to avoid the memleak warning */
Back in the old Unix lint days, it was common to put (void) in front of functions when return value was being ignored. Now this is considered unnecessary, catch up with the fashion. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html