Re: [PATCH 3/3] media: uapi: Use unsigned int values for assigning bits in u32 fields

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, May 08, 2023 at 10:52:09AM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Mon, May 08, 2023 at 09:58:48AM +0300, Sakari Ailus wrote:
> > On Mon, May 08, 2023 at 09:30:23AM +0300, Laurent Pinchart wrote:
> > > On Mon, May 08, 2023 at 09:19:24AM +0300, Sakari Ailus wrote:
> > > > On Sat, May 06, 2023 at 02:32:23PM +0300, Laurent Pinchart wrote:
> > > > > On Fri, May 05, 2023 at 11:51:01PM +0300, Sakari Ailus wrote:
> > > > > > Use unsigned int values annoted by "U" for u32 fields. While this is a
> > > > > > good practice, there doesn't appear to be a bug that this patch would fix.
> > > > > > 
> > > > > > The patch has been generated using the following command:
> > > > > > 
> > > > > > 	perl -i -pe 's/\([0-9]+\K <</U <</g' -- include/uapi/linux/media.h
> > > > > 
> > > > > How about using the _BITUL() macro from include/uapi/linux/const.h ?
> > > > 
> > > > These are u32 whereas _BITUL makes an unsigned long. Int (as in U) is 32
> > > > bits on all platforms where Linux is used AFAIK.
> > > 
> > > I know, but is it a problem ?
> > 
> > If we have a u32 field, unsigned int is the right type for that (from
> > non-fixed length C types), not unsigned long. In practice it would work, I
> > have no doubts about that. The compiler could still do different decisions
> > due to this, promoting values to a 64-bits for instance.
> > 
> > If we had _BITU(), I'd be happy to use that. :-)
> 
> Note how BIT() is defined in include/vdso/bits.h:
> 
> #include <vdso/const.h>
> 
> #define BIT(nr)                 (UL(1) << (nr))
> 
> And in include/vdso/const.h:
> 
> #include <uapi/linux/const.h>
> 
> #define UL(x)           (_UL(x))
> 
> BIT() is thus essentially identical to _BITUL(). As we use the former
> everywhere without any trouble, I wouldn't expect issue with the latter.

Fair enough. I'll switch to _BITUL() in v2.

-- 
Sakari Ailus



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux