On Tue, May 14, 2024 at 6:05 PM Satya Priya Kakitapalli (Temp) <quic_skakitap@xxxxxxxxxxx> wrote: > On 5/14/2024 7:48 PM, Andy Shevchenko wrote: > > On Tue, May 14, 2024 at 5:05 PM Satya Priya Kakitapalli > > <quic_skakitap@xxxxxxxxxxx> wrote: > >>> On Thu, May 09, 2024 at 03:07:02PM +0300, Andy Shevchenko wrote: > >>>> Wed, May 08, 2024 at 10:37:50PM +0000, Stephen Boyd kirjoitti: > >>>>> Quoting Johan Hovold (2024-05-06 08:08:29) ... > >>>>>> + BUILD_BUG_ON((ARRAY_SIZE(pldo_ranges) != 1) || > >>>>> This should be an && not || right? > >>>>>> + (ARRAY_SIZE(nldo_ranges) != 1)); > >>>> In any case BUILD_BUG_ON() is not encouraged for such cases, it would be much > >>>> better to have a static_assert() near to one of those arrays. > >>> I think the reason it is placed here is that the above line reads: > >>> > >>> rdesc->n_linear_ranges = 1; > >>> > >>> and that would need to change if anyone expands the arrays. > >> Correct. static_assert() cannot be used in the middle of code here, it can only be used at the declarations part which doesn't serve the purpose. > > I didn't get this. The ARRAY_SIZE():s are defined at compile time > > globally. How does this prevent from using static_assert()? > The reason we added it here is to make sure the nlod_ranges and > pldo_ranges doesn't become larger, and we forget updating the > n_linear_ranges. > Adding static_assert here is not feasible so adding a > BUILD_BUG_ON at this point makes sure the n_linear_ranges is proper. No, static_assert() will do _exactly_ the same with better error reporting and location, but what you are trying to say is that the location is chosen to be near to the n_liner_ranges assignment which happens at runtime, that's why it can't be used as an argument to BUILD_BUG_ON(). Based on this discussion I think the comment is missing before BUILD_BUG_ON() to explain the semantics of 1 and all that was just said. > >> So, BUILD_BUG_ON is the only way to go here. > > I don't think so. As i said. -- With Best Regards, Andy Shevchenko
