Received-SPF: pass (google.com: domain of linux-kernel+bounces-109511-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240320001656.10075-1-21cnbao@gmail.com> <EFB48F08-F0B5-47C0-8C47-00A542344AC9@126.com>
In-Reply-To: <EFB48F08-F0B5-47C0-8C47-00A542344AC9@126.com>
From: Barry Song <21cnbao@gmail.com>
Date: Thu, 21 Mar 2024 13:11:11 +1300
Message-ID: <CAGsJ_4xJsqOO-NXs3OWVA47vcK-zUpcrMxCbnY7x5khRH0dnxA@mail.gmail.com>
Subject: Re: [PATCH] Documentation: coding-style: ask function-like macros to
 evaluate parameters
To: Meiyong Yu <meiyong.yu@126.com>
Cc: corbet@lwn.net, workflows@vger.kernel.org, linux-doc@vger.kernel.org, 
	linux-kernel@vger.kernel.org, Barry Song <v-songbaohua@oppo.com>, 
	Andrew Morton <akpm@linux-foundation.org>, Chris Zankel <chris@zankel.net>, 
	Huacai Chen <chenhuacai@loongson.cn>, Herbert Xu <herbert@gondor.apana.org.au>, 
	Guenter Roeck <linux@roeck-us.net>, Max Filippov <jcmvbkbc@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, Mar 21, 2024 at 12:39=E2=80=AFPM Meiyong Yu <meiyong.yu@126.com> wr=
ote:
>
>
> > On Mar 20, 2024, at 08:17, Barry Song <21cnbao@gmail.com> wrote:
> >
> > =EF=BB=BFFrom: Barry Song <v-songbaohua@oppo.com>
> >
> > Recent commit 77292bb8ca69c80 ("crypto: scomp - remove memcpy if
> > sg_nents is 1 and pages are lowmem") leads to warnings on xtensa
> > and loongarch,
> >   In file included from crypto/scompress.c:12:
> >   include/crypto/scatterwalk.h: In function 'scatterwalk_pagedone':
> >   include/crypto/scatterwalk.h:76:30: warning: variable 'page' set but =
not used [-Wunused-but-set-variable]
> >      76 |                 struct page *page;
> >         |                              ^~~~
> >   crypto/scompress.c: In function 'scomp_acomp_comp_decomp':
> >>> crypto/scompress.c:174:38: warning: unused variable 'dst_page' [-Wunu=
sed-variable]
> >     174 |                         struct page *dst_page =3D sg_page(req=
->dst);
> >         |
> >
> > The reason is that flush_dcache_page() is implemented as a noop
> > macro on these platforms as below,
> >
> > #define flush_dcache_page(page) do { } while (0)
> >
> > The driver code, for itself, seems be quite innocent and placing
> > maybe_unused seems pointless,
> >
> > struct page *dst_page =3D sg_page(req->dst);
> >
> > for (i =3D 0; i < nr_pages; i++)
> >    flush_dcache_page(dst_page + i);
> >
> > And it should be independent of architectural implementation
> > differences.
> >
> > Let's have a guidance in codingstyle to ask for the evaluation
> > of parameters.
> >
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Chris Zankel <chris@zankel.net>
> > Cc: Huacai Chen <chenhuacai@loongson.cn>
> > Cc: Herbert Xu <herbert@gondor.apana.org.au>
> > Cc: Guenter Roeck <linux@roeck-us.net>
> > Suggested-by: Max Filippov <jcmvbkbc@gmail.com>
> > Signed-off-by: Barry Song <v-songbaohua@oppo.com>
> > ---
> > Documentation/process/coding-style.rst | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/Documentation/process/coding-style.rst b/Documentation/pro=
cess/coding-style.rst
> > index 9c7cf7347394..8065747fddff 100644
> > --- a/Documentation/process/coding-style.rst
> > +++ b/Documentation/process/coding-style.rst
> > @@ -827,6 +827,13 @@ Macros with multiple statements should be enclosed=
 in a do - while block:
> >                do_this(b, c);        \
> >        } while (0)
> >
>
>
> > +Function-like macros should evaluate their parameters, for unused para=
meters,
> I do not support this point, if the parameter is unused, why not to remov=
e it.
>

Linux boasts support for numerous architectures, striving for
independence in its
drivers and core code implementation across these architectures. Consequent=
ly,
certain architectures may utilize parameters for the same APIs, while other=
s may
not.

> about the warning,  is  tool misreport,  the tool must make better
>

no. This is not the case.

> > +cast them to void:
> > +
> > +.. code-block:: c
> > +
> > +    #define macrofun(a) do { (void) (a); } while (0)
> > +
> > Things to avoid when using macros:
> >
> > 1) macros that affect control flow:
> > --
> > 2.34.1
> >
>
>