On Wed, 14 Oct 2026 13:39:03 -0700 Bill Wendling <[email protected]> wrote:
> On Wed, Oct 14, 2026 at 5:38 AM David Laight > <[email protected]> wrote: > > On Tue, 13 Oct 2026 20:31:46 +0000 > > Bill Wendling <[email protected]> wrote: > > > > > The two parts of '__builtin_choose_expr()' had different arithmetic > > > expressions. This patch corrects it so that both parts calculate: > > > > > > COUNT * sizeof(*MEMBER) + __must_be_array(MEMBER) > > > > > > In practice, '__must_be_array' is meant to assert if 'MEMBER' isn't an > > > array and return '0' otherwise. However, symmetry is less prone to > > > subtle errors (and far more satisfying). > > > > > > Signed-off-by: Bill Wendling <[email protected]> > > > --- > > > v2: Don't use 'size_add', which just adds more complicated macro cruft to > > > the > > > expression. Instead, just adjust the parentheses. > > > --- > > > include/linux/overflow.h | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/include/linux/overflow.h b/include/linux/overflow.h > > > index a8cb6319b4fb..d0f04436343e 100644 > > > --- a/include/linux/overflow.h > > > +++ b/include/linux/overflow.h > > > @@ -418,7 +418,7 @@ static __always_inline size_t __must_check > > > size_sub(size_t minuend, size_t subtr > > > #define flex_array_size(p, member, count) \ > > > __builtin_choose_expr(__is_constexpr(count), \ > > > (count) * sizeof(*(p)->member) + > > > __must_be_array((p)->member), \ > > > - size_mul(count, sizeof(*(p)->member) + > > > __must_be_array((p)->member))) > > > + size_mul(count, sizeof(*(p)->member)) + > > > __must_be_array((p)->member)) > > > > Why does this use __builtin_choose_expr(). > > The types of the two expressions are the same so it might as well use ?: > > > It's to emit only one of the expressions rather than both. If the > 'count' is a constant expression, then it's possible for the compiler > to perform the calculation without having to hold onto the "false" > path of the ternary operator. So much more efficient in the long-run. I doubt it. Even with __builtin_choose_expr() the compile has to do most of the work for compiling the 'false' path. If you use ?: then you can also use the much simpler __builtin_constant_p() and possibly pick up cases where it is only known to be constant later. > > > Also doesn't this optimisation apply to all uses of size_mul() (aka > > saturating_mul()). > > Presumably relying on the compiler to report the compilation time numeric > > overflow. > > I'm not familiar with saturating_mul(). I'm sure others on here can talk to > it. That is me giving is a better name. The same is true for size_add(). IMHO saturation size_t calculations is really broken because it allows through 'bigger than sensible' sizes that still might cause problems (eg kmalloc() of sizes just below the limit.) David > > -bw > > > David > > > > > > > > /** > > > * struct_size() - Calculate size of structure with trailing flexible > > > array. > >
