On Mon, 2008-06-09 at 20:02 +0200, Johannes Berg wrote:
> > Well, the way the other functions of the same type:
> >
> > ieee80211_ctl_is_ack
> > ieee80211_mgmt_is_beacon
> >
> > check not only that the ftype is exactly ctl/mgmt and that it is of
> > stype ack/beacon.
>
> Hah. Well, I think they should probably be named
> ieee80211_is_ctl_ack/is_mgmt_beacon then, or just shorter
> _is_ack/_is_beacon (since everybody knows ack are ctl and beacon are
> mgmt.)
OK, just for clarity, could you spell out the helper names you'd like to
see, and I'll adjust accordingly, at this point I think you would agree
with:
ieee80211_is_data
ieee80211_is_ctl
ieee80211_is_mgmt
ieee80211_has_protected
ieee80211_has_morefrags
ieee80211_has_tods
ieee80211_has_fromds
ieee80211_has_a4
And you'd like to see the following changes:
ieee80211_data_has_qos -> ieee80211_is_data_qos (maybe dataqos)
ieee80211_ctl_is_ack -> ieee80211_is_ack
ieee80211_ctl_is_pspoll -> ieee80211_is_pspoll
ieee80211_ctl_is_back_req -> ieee80211_is_back_req
ieee80211_mgmt_is_beacon -> ieee80211_is_beacon
I agree that does look nicer, and it just has to be clear that these
explicity check the ftype as well.
>
> > I chose ieee80211_data_has_qos because it checks
> > the ftype _is_ data _and_ that it _has_ qos included. I think my
> > naming is more consistent with this.
> >
> > Do you still think it should be changed?
>
> I think you're looking at the bits too much. If you look at 802.11-2007,
> Table 7-1, you'll notice that all frames that have data ftype and the
> "QoS bit" set in the subtype are actually data+qos frames, just with
> extra functionality added onto them by the other stype bits.
>
> Hence, I do think it should be changed, we're more interested in the
> semantic meaning ("this is a data+QoS [possibly +stuff] frame") rather
> than the bitwise meaning ("this is a data frame which has some QoS bit
> set"). I guess the correct way would be to say "this is data frame which
> has QoS information" which we'd have to express as "_is_data_has_qos" or
> something, no?
Well, I agree with your semantics argument above, so this won't be necessary.
I guess I got a little caught up in the checks themselves vs the semantics
it was expressing.
So choose a colour for the bikeshed and I'll get painting ;-)
Cheers,
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
