[PATCH v3] hostap: Support ht-cap over-rides.

Jouni Malinen j at w1.fi
Mon Dec 19 04:49:10 EST 2011


On Sun, Dec 18, 2011 at 06:43:00PM -0800, Ben Greear wrote:
> On 12/18/2011 10:49 AM, Jouni Malinen wrote:
> > On Tue, Nov 22, 2011 at 12:37:40PM -0800, greearb at candelatech.com wrote:
> >> @@ -516,6 +517,21 @@ struct wpa_driver_associate_params {
> >> +#ifdef CONFIG_HT_OVERRIDES
> >> +	struct ieee80211_ht_capabilities htcaps;
> >> +	struct ieee80211_ht_capabilities htcaps_mask;
> >> +#endif
> >
> > i.e., these could be just const u8 *htcaps, *htcaps_mask and I would
> > drop the #ifdef from this case to keep driver.h API more independent of
> > the build configuration.
> 
> That just adds more void* pointers effectively, and that just
> makes it easier to introduce bugs that the compiler could otherwise
> easily catch.

Not sure I would really agree in this case. These are just payload of an
information element that gets copied in driver_nl80211.c to the driver
as-is.

> I added all the ifdef stuff to make the code smaller per your previous
> comments.  Making them pointers instead of members adds some extra coding
> overhead to deal with properly freeing memory, but if you really want
> them to be pointers, I will make the changes.

I understand the ifdefs that actually change code size - this particular
one does not (it changes the size of a short lived memory area). It is
not like these const u8 * values would use separately allocated memory
that would need explicit freeing (it would be a stack variable in
sme_associate() and wpa_supplicant_associate()).

> >> diff --git a/wpa_supplicant/config.c b/wpa_supplicant/config.c
> >> @@ -1864,6 +1875,14 @@ void wpa_config_set_network_defaults(struct wpa_ssid *ssid)
> >> +#ifdef CONFIG_HT_OVERRIDES
> >> +	ssid->ht_mcs = strdup("");
> >
> > Why? Wouldn't NULL work here?
> 
> Probably..but makes the parsing slightly harder if you have to check
> for null as well as empty string.  I can change it to null if you want.

Why would you need to check for an empty string? It just sounds odd to
allocate an empty buffer here (and by the way, that strdup() can return
NULL anyway)..

> I won't be able to work on this until first of January, but will
> post a new patch then that attempts to address your comments.

OK.

-- 
Jouni Malinen                                            PGP id EFC895FA


More information about the HostAP mailing list