Re: [v5 08/14] iio: imu: add Bosch Sensortec BNO055 core driver

[Andrea Merello <andrea.merello@gmail.com>]

Il giorno lun 2 mag 2022 alle ore 12:12 Andy Shevchenko
<andy.shevchenko@gmail.com> ha scritto:

One inline comment. OK for the rest

> > > > +#define BNO055_ATTR_VALS(...)          \
> > > > +       .vals = (int[]){ __VA_ARGS__},  \
> > > > +       .len = ARRAY_SIZE(((int[]){__VA_ARGS__}))

[...]

> And my point about readability. The reader, and even the author after
> some time, may have no clue in this forest of the macros and castings
> what's going on.

While I'm OK wrt your point in general, consider that it's just a
three LOC macro, used only in a few structs just below. I wouldn't say
it's so inricated; I've seen by far worse in the kernel :)

> > but about avoiding as much as
> > possible bugs caused by mismatched attr_vals, attr_aux and
> > ARRAY_SIZE() arg. e.g:
> > bno055_sysfs_attr_avail(priv, bno_foo_vals, ARRAY_SIZE(bno_bar_vals),
> > bno_foobar_aux, vals, len)
> >
> > I used to make quite a lot of mess until I grouped all the stuff in
> > one struct :/
>
> If something you want to prevent at compile time, consider to utilize
> static_assert() and / or BUILD_BUG_ON() depending on the place in the
> code (the former is preferred).

I would be happy to get rid of my macro and use those assertion
things, but I can't see how exactly. Do you have any advice about how
to take advantage of them for catching bugs like the one above in this
specific case?