On Fri, 2008-05-16 at 22:31 +0200, Johannes Berg wrote:
> > +static inline int ieee80211_fctl_tods(struct ieee80211_hdr *hdr)
>
> > +static inline int ieee80211_fctl_fromds(struct ieee80211_hdr *hdr)
>
> These seem fine though I don't see why you implement them with a > 0
> rather than != 0 comparison?
No reason, I'll move to all != 0
>
> > +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
>
> > +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.
>
> > +static inline int ieee80211_ftype_mgmt(struct ieee80211_hdr *hdr)
>
> > +static inline int ieee80211_ftype_ctl(struct ieee80211_hdr *hdr)
>
> > +static inline int ieee80211_ftype_data(struct ieee80211_hdr *hdr)
>
> similarly, ieee80211_is_data() (remove the ftype, everybody hacking
> wireless should know that data/mgmt/ctl are frame types)
OK, I'll drop ftype from these three, but leave it in the two-param
version as it is really just a helper for these three.
What do you think if I just left the stype version as-is, subject
to the usage I showed above, which is the common case.
Harvey
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
