Messages in this thread | | | From | Dwaipayan Ray <> | Date | Tue, 8 Jun 2021 03:02:16 +0530 | Subject | Re: [PATCH v2] docs: checkpatch: Document and segregate more checkpatch message types |
| |
On Mon, Jun 7, 2021 at 7:17 PM Dwaipayan Ray <dwaipayanray1@gmail.com> wrote: > > > > + **CONSTANT_CONVERSION** > > > + Use of __constant_<foo> form is discouraged for the following functions:: > > > + > > > + __constant_cpu_to_be[x] > > > + __constant_cpu_to_le[x] > > > + __constant_be[x]_to_cpu > > > + __constant_le[x]_to_cpu > > > + __constant_htons > > > + __constant_ntohs > > > + > > > + Using any of these outside of include/uapi/ isn't preferred as using the > > > > write out: s/isn't/is not/ > > > > ...or even stylistically much better is to just write the > > recommendation positively and clear: > > > > Use the corresponding function without __constant_ prefix, e.g., htons > > instead of __constant_htons, for any use in files, except > > include/uapi/. > > > > Are there other __constant_ functions in the code base beyond all the > > ones you listed? Then, we should explain why only those above and why > > not the others. Otherwise, we can keep the list above quite brief, and > > just say all __constant_ functions can be replaced by their > > counterparts without __constant_ prefix. > >
So, as Lukas said, I came up with this updated explanation for constant conversion:
**CONSTANT_CONVERSION** Use of __constant_<foo> form is discouraged for the following functions::
__constant_cpu_to_be[x] __constant_cpu_to_le[x] __constant_be[x]_to_cpu __constant_le[x]_to_cpu __constant_htons __constant_ntohs
Using any of these outside of include/uapi/ is not preferred as using the function without __constant_ is identical when the argument is a constant.
In big endian systems, the macros like __constant_cpu_to_be32(x) and cpu_to_be32(x) expand to the same expression::
#define __constant_cpu_to_be32(x) ((__force __be32)(__u32)(x)) #define __cpu_to_be32(x) ((__force __be32)(__u32)(x))
In little endian systems, the macros __constant_cpu_to_be32(x) and cpu_to_be32(x) expand to __constant_swab32 and __swab32. __swab32 has a __builtin_constant_p check::
#define __swab32(x) \ (__builtin_constant_p((__u32)(x)) ? \ ___constant_swab32(x) : \ __fswab32(x))
So ultimately they have a special case for constants. Similar is the case with all of the macros in the list. Thus using the __constant_... forms are unnecessarily verbose and not preferred outside of include/uapi.
See: https://lore.kernel.org/lkml/1400106425.12666.6.camel@joe-AO725/
Can Lukas or Joe confirm this or have any comments on it. I can send an updated patch then.
Thanks, Dwaipayan.
| |