Message ID | 20131101101844.GA29795@longonot.mountain |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
On Fri, Nov 01, 2013 at 01:18:44PM +0300, Dan Carpenter wrote: >There is an extra semi-colon so bond_get_size() doesn't return the >correct value. > >Fixes: ec76aa49855f ('bonding: add Netlink support active_slave option') >Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> Hrm, how does it even build. Good catch, thanks. Acked-by: Veaceslav Falico <vfalico@redhat.com> > >diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c >index 7661261..40e7b1c 100644 >--- a/drivers/net/bonding/bond_netlink.c >+++ b/drivers/net/bonding/bond_netlink.c >@@ -82,8 +82,8 @@ static int bond_newlink(struct net *src_net, struct net_device *bond_dev, > > static size_t bond_get_size(const struct net_device *bond_dev) > { >- return nla_total_size(sizeof(u8)); /* IFLA_BOND_MODE */ >- + nla_total_size(sizeof(u32)); /* IFLA_BOND_ACTIVE_SLAVE */ >+ return nla_total_size(sizeof(u8)) + /* IFLA_BOND_MODE */ >+ nla_total_size(sizeof(u32)); /* IFLA_BOND_ACTIVE_SLAVE */ > } > > static int bond_fill_info(struct sk_buff *skb, -- 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
On Fri, 2013-11-01 at 11:41 +0100, Veaceslav Falico wrote: > On Fri, Nov 01, 2013 at 01:18:44PM +0300, Dan Carpenter wrote: > >There is an extra semi-colon so bond_get_size() doesn't return the > >correct value. > > > >Fixes: ec76aa49855f ('bonding: add Netlink support active_slave option') > >Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > Hrm, how does it even build. Good catch, thanks. > > Acked-by: Veaceslav Falico <vfalico@redhat.com> Yep, an alternative is to use following format : static inline size_t br_port_info_size(void) { return nla_total_size(1) /* IFLA_BRPORT_STATE */ + nla_total_size(2) /* IFLA_BRPORT_PRIORITY */ + nla_total_size(4) /* IFLA_BRPORT_COST */ + nla_total_size(1) /* IFLA_BRPORT_MODE */ + nla_total_size(1) /* IFLA_BRPORT_GUARD */ + nla_total_size(1) /* IFLA_BRPORT_PROTECT */ + nla_total_size(1) /* IFLA_BRPORT_FAST_LEAVE */ + nla_total_size(1) /* IFLA_BRPORT_LEARNING */ + nla_total_size(1) /* IFLA_BRPORT_UNICAST_FLOOD */ + 0; } So that a patch adding a new attribute doesn't have to change the 'last line' -- 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
Fri, Nov 01, 2013 at 11:41:00AM CET, vfalico@redhat.com wrote: >On Fri, Nov 01, 2013 at 01:18:44PM +0300, Dan Carpenter wrote: >>There is an extra semi-colon so bond_get_size() doesn't return the >>correct value. >> >>Fixes: ec76aa49855f ('bonding: add Netlink support active_slave option') >>Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > >Hrm, how does it even build. Good catch, thanks. I wonder about the same thing. Reviewed-by: Jiri Pirko <jiri@resnulli.us> > >Acked-by: Veaceslav Falico <vfalico@redhat.com> > >> >>diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c >>index 7661261..40e7b1c 100644 >>--- a/drivers/net/bonding/bond_netlink.c >>+++ b/drivers/net/bonding/bond_netlink.c >>@@ -82,8 +82,8 @@ static int bond_newlink(struct net *src_net, struct net_device *bond_dev, >> >>static size_t bond_get_size(const struct net_device *bond_dev) >>{ >>- return nla_total_size(sizeof(u8)); /* IFLA_BOND_MODE */ >>- + nla_total_size(sizeof(u32)); /* IFLA_BOND_ACTIVE_SLAVE */ >>+ return nla_total_size(sizeof(u8)) + /* IFLA_BOND_MODE */ >>+ nla_total_size(sizeof(u32)); /* IFLA_BOND_ACTIVE_SLAVE */ >>} >> >>static int bond_fill_info(struct sk_buff *skb, -- 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
On 11/01/2013 12:35 PM, Jiri Pirko wrote: > Fri, Nov 01, 2013 at 11:41:00AM CET, vfalico@redhat.com wrote: >> On Fri, Nov 01, 2013 at 01:18:44PM +0300, Dan Carpenter wrote: >>> There is an extra semi-colon so bond_get_size() doesn't return the >>> correct value. >>> >>> Fixes: ec76aa49855f ('bonding: add Netlink support active_slave option') >>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> >> >> Hrm, how does it even build. Good catch, thanks. > > I wonder about the same thing. > + and - can be unary operators so it's a valid expression :-) Nice catch Nik -- 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
Am 01.11.2013 13:25, schrieb Nikolay Aleksandrov: > On 11/01/2013 12:35 PM, Jiri Pirko wrote: >> Fri, Nov 01, 2013 at 11:41:00AM CET, vfalico@redhat.com wrote: >>> On Fri, Nov 01, 2013 at 01:18:44PM +0300, Dan Carpenter wrote: >>>> There is an extra semi-colon so bond_get_size() doesn't return the >>>> correct value. >>>> >>>> Fixes: ec76aa49855f ('bonding: add Netlink support active_slave option') >>>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> >>> >>> Hrm, how does it even build. Good catch, thanks. >> >> I wonder about the same thing. >> > + and - can be unary operators so it's a valid expression :-) > Nice catch > NTL my gcc warns (-Wall) warning: statement with no effect this should come up. re, wh > Nik > > -- > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- 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: Dan Carpenter <dan.carpenter@oracle.com> Date: Fri, 1 Nov 2013 13:18:44 +0300 > There is an extra semi-colon so bond_get_size() doesn't return the > correct value. > > Fixes: ec76aa49855f ('bonding: add Netlink support active_slave option') > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> Applied, thanks a lot Dan. -- 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
diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c index 7661261..40e7b1c 100644 --- a/drivers/net/bonding/bond_netlink.c +++ b/drivers/net/bonding/bond_netlink.c @@ -82,8 +82,8 @@ static int bond_newlink(struct net *src_net, struct net_device *bond_dev, static size_t bond_get_size(const struct net_device *bond_dev) { - return nla_total_size(sizeof(u8)); /* IFLA_BOND_MODE */ - + nla_total_size(sizeof(u32)); /* IFLA_BOND_ACTIVE_SLAVE */ + return nla_total_size(sizeof(u8)) + /* IFLA_BOND_MODE */ + nla_total_size(sizeof(u32)); /* IFLA_BOND_ACTIVE_SLAVE */ } static int bond_fill_info(struct sk_buff *skb,
There is an extra semi-colon so bond_get_size() doesn't return the correct value. Fixes: ec76aa49855f ('bonding: add Netlink support active_slave option') Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> -- 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