[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1293984061.29378.101.camel@lb-tlvb-shmulik.il.broadcom.com>
Date: Sun, 2 Jan 2011 18:01:01 +0200
From: "Shmulik Ravid" <shmulikr@...adcom.com>
To: "John Fastabend" <john.r.fastabend@...el.com>
cc: "davem@...emloft.net" <davem@...emloft.net>,
"Eilon Greenstein" <eilong@...adcom.com>,
"Liu, Lucy" <lucy.liu@...el.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [net-next-2.6 PATCH v2 2/4] dcbnl: adding DCBX feature
flags get-set
> One more nit ;)
>
> > +
> > + ret = nla_parse_nested(data, DCB_FEATCFG_ATTR_MAX, tb[DCB_ATTR_FEATCFG],
> > + dcbnl_featcfg_nest);
> > + if (ret) {
> > + ret = -EINVAL;
> > + goto err_out;
> > + }
>
> Why do you set EINVAL here if you use the returned error code from nla_parse_nested you get a more descriptive error. See ./lib/nlattr.c:nla_parse()/validate_nla().
>
> [...]
>
> > +static int dcbnl_setfeatcfg(struct net_device *netdev, struct nlattr **tb,
> > + u32 pid, u32 seq, u16 flags)
> > +{
> > + struct nlattr *data[DCB_FEATCFG_ATTR_MAX + 1];
> > + int ret = -EINVAL;
> > + u8 value;
> > + int i;
> > +
> > + if (!tb[DCB_ATTR_FEATCFG] || !netdev->dcbnl_ops->setfeatcfg)
> > + return ret;
> > +
> > + ret = nla_parse_nested(data, DCB_FEATCFG_ATTR_MAX, tb[DCB_ATTR_FEATCFG],
> > + dcbnl_featcfg_nest);
> > +
> > + if (ret) {
> > + ret = -EINVAL;
> > + goto err;
> > + }
>
> Same here.
>
I'll send a patch with the improved return values for the the new dcbnl
routines. While I'm at it, is it safe to fix on the same lines the
older already established dcbnl routines?
Thanks,
Shmulik
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists