On Sun Apr 24, 2022 at 3:42 PM CEST, Jouni Malinen wrote:
> On Thu, Mar 24, 2022 at 03:19:17PM +0100, Nicolas Escande wrote:
> > diff --git a/src/ap/acs.c b/src/ap/acs.c
> > +const struct {
> > + int first;
> > + int last;
> > + int center_chan;
> > +} bw40_seg[] = { { 5180, 5200, 38 }, { 5220, 5240, 46 }, { 5260, 5280, 54 },
>
> This would make the bw40_seg visible to out her files.. Shouldn't this
> have "static" somewhere if the list is moved to the file global level
> from the function?
Yes
>
> > static int acs_usable_bw40_chan(const struct hostapd_channel_data *chan)
> > - for (i = 0; i < ARRAY_SIZE(allowed); i++)
> > - if (chan->freq == allowed[i])
> > + for (i = 0; i < ARRAY_SIZE(bw40_seg); i++)
> > + if (chan->freq == bw40_seg[i].first)
> > return 1;
> >
> > return 0;
>
> And with this type of design, it would seem cleaner to replace these
> separate acs_usable_bw*_chan() functions with a single one that takes
> the appropriate table (e.g., bw40_seg) as an argument to avoid
> duplicated implementation. This would also apply for the patch 2/4
> acs_get_bw*_center_chan() functions.
>
> It might be easier to have an explicit termination entry in the array
> (e.g., first = last = center_chan = -1) instead of depending on
> ARRAY_SIZE() for that. And the struct could be named as well for this to
> work cleanly.
Sure, fine by me, I'll do it like that.
>
> --
> Jouni Malinen PGP id EFC895FA
_______________________________________________
Hostap mailing list
Hostap@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/hostap
