Re: [PATCH v2] docs: checkpatch: Document and segregate more checkpatch message types

On Mon, Jun 7, 2021 at 7:17 PM Dwaipayan Ray [off-list ref] wrote:

quoted

quoted

+  **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/ (local)

Can Lukas or Joe confirm this or have any comments on it. I can send
an updated patch then.

Thanks,
Dwaipayan.