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

[Dwaipayan Ray]

On Mon, Jun 7, 2021 at 7:17 PM Dwaipayan Ray <dwaipayanray1@xxxxxxxxx> 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.