On Wed, 2025-02-12 at 10:01 +0100, Uwe Kleine-König wrote:
> On Thu, Jan 30, 2025 at 08:14:39PM +0200, Andy Shevchenko wrote:
> > On Thu, Jan 30, 2025 at 06:45:01PM +0100, Uwe Kleine-König wrote:
> >
> > ...
> >
> > > + BUILD_BUG_ON(sizeof(*a) !=
> > > + sizeof(struct {
> > > + unsigned int iout0_val;
> > > + unsigned int iout1_val;
> > > + unsigned int burnout;
> > > + unsigned int pga;
> > > + unsigned int fs;
> > > + u32 ref_sel;
> > > + enum ad4130_filter_mode filter_mode;
> > > + bool ref_bufp;
> > > + bool ref_bufm;
> > > + }));
> >
> > Is I shuffle the fields (for whatever reason) this may give false positive
> > warnings.
>
> That's fine in by book. Whenever the struct is changed it's a good
> opportunity to check if the cmp function needs adaption, too.
>
In practise I think shuffling the fields is unlikely...
> > I think this BUILD_BUG_ON() is unreliable and ugly looking
> > (static_assert() won't help much here either), so on the second though I
> > think
> > it's better to simply add a comments in both places (here and near to the
> > structure definition) to explain that these needs to be in sync.
>
> The nice thing about BUILD_BUG_ON (or static assert) is that the
> compiler quite reliably enforces the two being in sync. A comment
> doesn't.
>
Agreed... I guess we could also add a comment on top of the affected struct so
that if someone messes with it, is also aware of this comparison.
- Nuno Sá
