Il giorno lun 2 mag 2022 alle ore 12:12 Andy Shevchenko
<andy.shevchenko@xxxxxxxxx> 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?
