So I'm not sure what your desired path for getting this upstream is. I can take it, but I'm generally quite leery of taking coding-style changes without some serious acks on them - nobody elected me as the arbiter of proper coding style.
A nit below Barry Song <[email protected]> writes: > From: Barry Song <[email protected]> > > 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' >>> [-Wunused-variable] > 174 | struct page *dst_page = 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 = sg_page(req->dst); > > for (i = 0; i < nr_pages; i++) > flush_dcache_page(dst_page + i); > > And it should be independent of architectural implementation > differences. > > Let's provide guidance on coding style for requesting parameter > evaluation or proposing the migration to a static inline > function. > > Signed-off-by: Barry Song <[email protected]> > Suggested-by: Max Filippov <[email protected]> > Reviewed-by: Mark Brown <[email protected]> > Cc: Chris Zankel <[email protected]> > Cc: Huacai Chen <[email protected]> > Cc: Herbert Xu <[email protected]> > Cc: Guenter Roeck <[email protected]> > Cc: Stephen Rothwell <[email protected]> > Cc: Andy Whitcroft <[email protected]> > Cc: Dwaipayan Ray <[email protected]> > Cc: Joe Perches <[email protected]> > Cc: Jonathan Corbet <[email protected]> > Cc: Lukas Bulwahn <[email protected]> > Cc: Xining Xu <[email protected]> > --- > Documentation/process/coding-style.rst | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/Documentation/process/coding-style.rst > b/Documentation/process/coding-style.rst > index 9c7cf7347394..791d333a57fd 100644 > --- a/Documentation/process/coding-style.rst > +++ b/Documentation/process/coding-style.rst > @@ -827,6 +827,22 @@ Macros with multiple statements should be enclosed in a > do - while block: > do_this(b, c); \ > } while (0) > > +Function-like macros with unused parameters should be replaced by static > +inline functions to avoid the issue of unused variables: > + > +.. code-block:: c I would just use the "::" notation here; the ..code-block:: just adds noise IMO. > + static inline void fun(struct foo *foo) > + { > + } > + > +For historical reasons, many files still use the cast to (void) to evaluate > +parameters, but this method is not recommended: > + > +.. code-block:: c > + > + #define macrofun(foo) do { (void) (foo); } while (0) > + 1) If you're putting in examples of something *not* to do, it's probably better to also put in something like: /* don't do this */ people don't always read closely. 2) Can we say *why* it's not recommended? Thanks, jon
