> > > +static inline int ieee80211_fctl_has_a4(struct ieee80211_hdr *hdr)
> >
> > That seems fine too.
> >
> > > +static inline int ieee80211_ftype(struct ieee80211_hdr *hdr, u16 ftype)
> >
> > That, I think, is misnamed, it should be ieee80211_is_ftype()
>
> Well, I didn't expect this to be used directly that much, hence the
> following three helpers...ftype_mgmt/ctl/data
Right. Still, I think it should be named with _is_ in there.
> > > +static inline int ieee80211_stype(struct ieee80211_hdr *hdr, u16 stype)
> > > +{
> > > + return (hdr->frame_control & cpu_to_le16(stype)) != 0;
> > > +}
> >
> > And that even seems implemented wrongly? stype is a 4-bit field, this
> > doesn't make much sense to me.
>
> It was meant to be used to replace code like:
>
> fc & IEEE80211_STYPE_QOS_DATA
>
> ieee80211_stype(hdr, IEEE80211_STYPE_QOS_DATA)
>
> As most users of the stype bits test against a specific constant,
> otherwise you could add a bunch of helpers, one for each stype
> constant (similar to the ftype helpers I added. There were only
> 3 in the ftype case, so I just added them all and dropped the second
> parameter, in the stype case, there are more options, so I just
> left one two-parameter helper.
Well, yes, but that's semantically unclear because of the way these bits
are used vs. how they are called. The subtype (stype) is a four-bit
field in the frame control field which isn't actually defined to have
"flags" or bits like that in it. Look at the table in IEEE 802.11-2007,
it's at the beginning of clause 7 somewhere.
Hence, hiding this simple AND behind a function that claims to check the
subtype is confusing.
johannes
Attachment:
signature.asc
Description: This is a digitally signed message part
