Re: [patch 6/9] nl80211/cfg80211: station handling

[Johannes Berg <johannes@xxxxxxxxxxxxxxxx>]

> > +static int get_vlan(struct nlattr *vlanattr,
> > +                   struct cfg80211_registered_device *rdev,
> > +                   struct net_device **vlan)
> > +{
> > +       if (vlanattr) {
> > +               *vlan = dev_get_by_index(&init_net, nla_get_u32(vlanattr));
> > +               if (!*vlan)
> > +                       return -ENODEV;
> 
> 
> > +               if (!(*vlan)->ieee80211_ptr)
> > +                       return -EINVAL;
> 
> Check #1 for (!(*vlan)->ieee80211_ptr)
> 
> > +               if (!(*vlan)->ieee80211_ptr)
> > +                       return -EINVAL;
> 
> Check #2 for (!(*vlan)->ieee80211_ptr). Double check.

Hah. I think the latter was meant to be ->wiphy but that shouldn't be
necessary.

> > +       err = get_vlan(info->attrs[NL80211_ATTR_STA_VLAN], drv, &params.vlan);
> > +       if (err)
> > +               goto out;
> 
> This is minor, but how about doing the check of
> info->attrs[NL80211_ATTR_STA_VLAN] outside of get_vlan to make it
> clear here we're setting it to 0 if not found?

Dunno. That makes the error path more complicated, no? I guess that then
I'd rather do an *vlan = NULL assignment in get_vlan() to make it
crystal clear.

johannes

Attachment: signature.asc
Description: This is a digitally signed message part