Le mercredi 30 mars 2022 à 18:16 +0300, Dan Carpenter a écrit :
> Yeah. I'm aboslutely fine with whatever you do. Some of the questions
> you're asking occurred to me too but I don't have the answers.
>
> > > > > > + for (i = 0; i < builder->num_valid; i++) {
> > > > > > + struct v4l2_h264_reference *ref;
> > > > > > + u8 dpb_valid;
> > > > > > + u8 bottom;
> > > > >
> > > > > These would be better as type bool.
> > > >
> > > > I never used a bool for bit operations before, but I guess that can work, thanks
> > > > for the suggestion. As this deviates from the original code, I suppose I should
> > > > make this a separate patch ?
> > >
> > > I just saw the name and wondered why it was a u8. bool does make more
> > > sense and works fine for the bitwise stuff. But I don't really care at
> > > all.
> >
> > I'll do that in v2, in same patch, looks minor enough. I think if using bool
> > could guaranty that only 1 or 0 is possible, it would be even better, but don't
> > think C works like this.
>
> I'm not sure I understand. If you assign "bool x = <any non-zero>;"
> then x is set to true. Do you want a static checker warning for if
> <any non-zero> can be something other than one or zero? The problem is
> that people sometimes deliberately do stuff like "bool x = var & 0xf0;".
> Smatch will complain if you assign a negative value to x.
>
> test.c:8 test() warn: assigning (-3) to unsigned variable 'x'
>
> It's supposed to print a warning if you used it to save error codes like:
>
> x = some_kernel_function();
>
> But it does not. :/ Something to investigate.
That would be an amazing catch, you might have seen a lot of:
x = !!(var & 0xf0)
For branches, it does no matter, but if you use x it like this dpb_valid
variable is used, not having 0 or 1 can lead to very surprising results. In the
end its used like this
set_reg(reg0, val | (x << N))
So using bool type can hint the analyzer that 0 or 1 was likely expected, while
currently an u8 would be ambiguous and lead to false positive if we were to
warn.
>
> regards,
> dan carpenter
>
