lkml.org 
[lkml]   [2022]   [May]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [v5 08/14] iio: imu: add Bosch Sensortec BNO055 core driver
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?

\
 
 \ /
  Last update: 2022-05-02 15:13    [W:0.043 / U:0.200 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site