Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210605070502.22288-1-dwaipayanray1@gmail.com>
 <CAKXUXMydnnun3iu3B2_acQLd6EgDHOGUhKvbuUCh6WaS-ohiDw@mail.gmail.com> <CABJPP5CfoPrybLgmZ2AP15x_esCPC6J780QYzEahMB9qzkRD=Q@mail.gmail.com>
In-Reply-To: <CABJPP5CfoPrybLgmZ2AP15x_esCPC6J780QYzEahMB9qzkRD=Q@mail.gmail.com>
From:   Dwaipayan Ray <dwaipayanray1@gmail.com>
Date:   Tue, 8 Jun 2021 03:02:16 +0530
Message-ID: <CABJPP5BufzwYjHd=U0Xhz+3+miMOv-DP5DZiUo+jvNFraw2X_A@mail.gmail.com>
Subject: Re: [PATCH v2] docs: checkpatch: Document and segregate more
 checkpatch message types
To:     Lukas Bulwahn <lukas.bulwahn@gmail.com>
Cc:     Joe Perches <joe@perches.com>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        "open list:DOCUMENTATION" <linux-doc@vger.kernel.org>,
        Jonathan Corbet <corbet@lwn.net>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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.