>
> Fixes: 9eaffe5078ca ("cfg80211: convert S1G beacon to scan results")
Should it really be wireless-next rather than wireless with this? We're
still even in the merge window, so seems entirely appropriate to put it
into wireless instead, even if the bug is old, rather than wait months
for it to release?
It also seems you should split this to cfg80211 and mac80211, if
possible, and have two different Fixes: lines? The cfg80211 part this
has might fix the above, but for the mac80211 part that doesn't seem
plausible?
> +/**
> + * ieee80211_s1g_has_next_tbtt - check if IEEE80211_S1G_BCN_NEXT_TBTT
> + * @fc: frame control bytes in little-endian byteorder
> + * Return: whether or not the frame contains the variable-length
> + * next TBTT field
nit: the indentation should be a tab there (and in the later instances
in the other functions)
> +/**
> + * ieee80211_s1g_optional_len - determine length of optional S1G beacon fields
> + * @fc: frame control bytes in little-endian byteorder
> + * Return: total length in bytes of the optional fixed-length fields
> + *
> + * S1G beacons may contain up to three optional fixed-length fields that
> + * precede the variable-length information elements (IEs). Whether these
Also kind of nit, but the current spec versions (for a long time) have
stopped calling it "information elements" and say just "elements"
instead, I've been trying to at least do that in new code in Linux.
> if (ieee80211_is_s1g_beacon(mgmt->frame_control)) {
> struct ieee80211_ext *ext = (void *) mgmt;
> -
> - if (ieee80211_is_s1g_short_beacon(ext->frame_control))
> - variable = ext->u.s1g_short_beacon.variable;
> - else
> - variable = ext->u.s1g_beacon.variable;
> + variable = ext->u.s1g_beacon.variable
> + + ieee80211_s1g_optional_len(ext->frame_control);
Please do this like in other code:
variable = ... +
...;
(also more instances below)
> if (ieee80211_is_s1g_beacon(mgmt->frame_control)) {
> - ext = (void *) mgmt;
> - if (ieee80211_is_s1g_short_beacon(mgmt->frame_control))
> - min_hdr_len = offsetof(struct ieee80211_ext,
> - u.s1g_short_beacon.variable);
> - else
> - min_hdr_len = offsetof(struct ieee80211_ext,
> - u.s1g_beacon.variable);
> + ext = (struct ieee80211_ext *)mgmt;
Keeping the (void *) cast seems fine? I guess you can remove the space
if you like :)
Overall though I like the fact that we don't try to distinguish the
whole short/normal beacon thing, that was always a bit of a mess, and if
it fixes more bugs while at it ... nice :)
johannes
