On Wed, 2022-06-08 at 13:15 +0530, Veerendranath Jakkam wrote:
> On 6/8/2022 1:01 PM, Johannes Berg wrote:
> > On Mon, 2022-06-06 at 23:01 +0530, Veerendranath Jakkam wrote:
> >
> > > + if (cr->status == WLAN_STATUS_SUCCESS) {
> > > + for_each_valid_link(cr, link) {
> > > + if (!cr->links[link].bss)
> > > + break;
> > > + }
> > > +
> > > + WARN_ON_ONCE((!cr->valid_links && link != 1) ||
> > > + (cr->valid_links &&
> > > + link != ARRAY_SIZE(wdev->links)));
> > > +
> > I will say I'm not super happy with using the link variable after the
> > loop, that always feels a bit magic to me, especially if the loop is
> > hidden like that...
> >
> > But I guess I don't see a lot of alternatives here, other than open-
> > coding it, or keeping track of "how many BSSes do I have".
>
>
> Since we need to WARN even if single BSS is not present I think we can
> use "bss_not_found" flag?
>
> >
> > Actually, for the MLO case, is this even valid? link[14] could be set,
> > so you wouldn't break, ending up with link==15? Or am I confused?
> In MLO case the link value will be always 15 after loop completes if
> bsses are available for all valid links since the check is only for the
> valid links
>
> so, In above case also when "link==15" the condition fails and WARN will
> be skipped right.
>
Ah, indeed. I was thinking of the 'break', but you only get there for
valid links and the valid links should indeed have a .bss pointer.
OK, so I guess let's leave it as is, that way we check both cases
accurately.
johannes
