Messages in this thread | | | Date | Wed, 30 Mar 2022 18:16:58 +0300 | From | Dan Carpenter <> | Subject | Re: [PATCH v1 19/24] media: rkvdec-h264: Add field decoding support |
| |
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.
regards, dan carpenter
| |