> > > +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