Re: [RFC-PATCH] mac80211: add helpers for frame control tests

[Harvey Harrison <harvey.harrison@gmail.com>]

On Fri, 2008-05-16 at 22:31 +0200, Johannes Berg wrote:
> > +static inline int ieee80211_fctl_tods(struct ieee80211_hdr *hdr)
>=20
> > +static inline int ieee80211_fctl_fromds(struct ieee80211_hdr *hdr)
>=20
> These seem fine though I don't see why you implement them with a > 0
> rather than !=3D 0 comparison?

No reason, I'll move to all !=3D 0

>=20
> > +static inline int ieee80211_fctl_has_a4(struct ieee80211_hdr *hdr)
>=20
> That seems fine too.
>=20
> > +static inline int ieee80211_ftype(struct ieee80211_hdr *hdr, u16 f=
type)
>=20
> 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

>=20
> > +static inline int ieee80211_stype(struct ieee80211_hdr *hdr, u16 s=
type)
> > +{
> > +	return (hdr->frame_control & cpu_to_le16(stype)) !=3D 0;
> > +}
>=20
> 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:

=EF=BB=BFfc & IEEE80211_STYPE_QOS_DATA

=EF=BB=BFieee80211_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.

>=20
> > +static inline int ieee80211_ftype_mgmt(struct ieee80211_hdr *hdr)
>=20
> > +static inline int ieee80211_ftype_ctl(struct ieee80211_hdr *hdr)
>=20
> > +static inline int ieee80211_ftype_data(struct ieee80211_hdr *hdr)
>=20
> 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-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html