| Message ID | 1510669859-12055-1-git-send-email-cugyly@163.com |
|---|---|
| State | Rejected, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net-next] net: assign err to 0 at begin in do_setlink() function | expand |
From: yuan linyu <cugyly@163.com> Date: Tue, 14 Nov 2017 22:30:59 +0800 > From: yuan linyu <Linyu.Yuan@alcatel-sbell.com.cn> > > each netlink attribute have proper process when error happen, > when exit on attribute process, it implies that no error, > so err = 0; is useless. > > assign err = 0; at beginning if all attributes not set. > > Signed-off-by: yuan linyu <Linyu.Yuan@alcatel-sbell.com.cn> This change is not correct. The IFLA_VF_PORTS code block can finish with err set non-zero. The assignment to zero there is really necessary.
> -----Original Message----- > From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] > On Behalf Of David Miller > Sent: Wednesday, November 15, 2017 1:08 PM > To: cugyly@163.com > Cc: netdev@vger.kernel.org; Yuan, Linyu (NSB - CN/Shanghai) > Subject: Re: [PATCH net-next] net: assign err to 0 at begin in do_setlink() > function > > From: yuan linyu <cugyly@163.com> > Date: Tue, 14 Nov 2017 22:30:59 +0800 > > > From: yuan linyu <Linyu.Yuan@alcatel-sbell.com.cn> > > > > each netlink attribute have proper process when error happen, > > when exit on attribute process, it implies that no error, > > so err = 0; is useless. > > > > assign err = 0; at beginning if all attributes not set. > > > > Signed-off-by: yuan linyu <Linyu.Yuan@alcatel-sbell.com.cn> > > This change is not correct. > > The IFLA_VF_PORTS code block can finish with err set non-zero. Do you mean this block can return err = -EOPNOTSUPP; ? It means nla_for_each_nested() in this block will never enter, right ? > The assignment to zero there is really necessary.
From: "Yuan, Linyu (NSB - CN/Shanghai)" <linyu.yuan@nokia-sbell.com> Date: Wed, 15 Nov 2017 05:15:35 +0000 > > >> -----Original Message----- >> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] >> On Behalf Of David Miller >> Sent: Wednesday, November 15, 2017 1:08 PM >> To: cugyly@163.com >> Cc: netdev@vger.kernel.org; Yuan, Linyu (NSB - CN/Shanghai) >> Subject: Re: [PATCH net-next] net: assign err to 0 at begin in do_setlink() >> function >> >> From: yuan linyu <cugyly@163.com> >> Date: Tue, 14 Nov 2017 22:30:59 +0800 >> >> > From: yuan linyu <Linyu.Yuan@alcatel-sbell.com.cn> >> > >> > each netlink attribute have proper process when error happen, >> > when exit on attribute process, it implies that no error, >> > so err = 0; is useless. >> > >> > assign err = 0; at beginning if all attributes not set. >> > >> > Signed-off-by: yuan linyu <Linyu.Yuan@alcatel-sbell.com.cn> >> >> This change is not correct. >> >> The IFLA_VF_PORTS code block can finish with err set non-zero. > > Do you mean this block can return err = -EOPNOTSUPP; ? > It means nla_for_each_nested() in this block will never enter, right ? Yes, that is what I mean.
Thanks, I will provide a new patch > -----Original Message----- > From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] > On Behalf Of David Miller > Sent: Wednesday, November 15, 2017 1:20 PM > To: Yuan, Linyu (NSB - CN/Shanghai) > Cc: cugyly@163.com; netdev@vger.kernel.org > Subject: Re: [PATCH net-next] net: assign err to 0 at begin in do_setlink() > function > > From: "Yuan, Linyu (NSB - CN/Shanghai)" <linyu.yuan@nokia-sbell.com> > Date: Wed, 15 Nov 2017 05:15:35 +0000 > > > > > > >> -----Original Message----- > >> From: netdev-owner@vger.kernel.org > [mailto:netdev-owner@vger.kernel.org] > >> On Behalf Of David Miller > >> Sent: Wednesday, November 15, 2017 1:08 PM > >> To: cugyly@163.com > >> Cc: netdev@vger.kernel.org; Yuan, Linyu (NSB - CN/Shanghai) > >> Subject: Re: [PATCH net-next] net: assign err to 0 at begin in do_setlink() > >> function > >> > >> From: yuan linyu <cugyly@163.com> > >> Date: Tue, 14 Nov 2017 22:30:59 +0800 > >> > >> > From: yuan linyu <Linyu.Yuan@alcatel-sbell.com.cn> > >> > > >> > each netlink attribute have proper process when error happen, > >> > when exit on attribute process, it implies that no error, > >> > so err = 0; is useless. > >> > > >> > assign err = 0; at beginning if all attributes not set. > >> > > >> > Signed-off-by: yuan linyu <Linyu.Yuan@alcatel-sbell.com.cn> > >> > >> This change is not correct. > >> > >> The IFLA_VF_PORTS code block can finish with err set non-zero. > > > > Do you mean this block can return err = -EOPNOTSUPP; ? > > It means nla_for_each_nested() in this block will never enter, right ? > > Yes, that is what I mean.
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index dabba2a..4760249 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -2075,7 +2075,7 @@ static int do_setlink(const struct sk_buff *skb, struct nlattr **tb, char *ifname, int status) { const struct net_device_ops *ops = dev->netdev_ops; - int err; + int err = 0; if (tb[IFLA_NET_NS_PID] || tb[IFLA_NET_NS_FD]) { struct net *net = rtnl_link_get_net(dev_net(dev), tb); @@ -2253,7 +2253,6 @@ static int do_setlink(const struct sk_buff *skb, status |= DO_SETLINK_NOTIFY; } } - err = 0; if (tb[IFLA_VF_PORTS]) { struct nlattr *port[IFLA_PORT_MAX+1]; @@ -2286,7 +2285,6 @@ static int do_setlink(const struct sk_buff *skb, status |= DO_SETLINK_NOTIFY; } } - err = 0; if (tb[IFLA_PORT_SELF]) { struct nlattr *port[IFLA_PORT_MAX+1]; @@ -2326,7 +2324,6 @@ static int do_setlink(const struct sk_buff *skb, status |= DO_SETLINK_NOTIFY; } } - err = 0; if (tb[IFLA_PROTO_DOWN]) { err = dev_change_proto_down(dev,