linux-kernel - Re: [PATCH 4/4] selftests/mm: add self tests for guard page feature

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a64650ea-fef3-4d6c-8a36-3fab73f7a0bf@lucifer.local>
Date: Fri, 18 Oct 2024 17:41:34 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Shuah Khan <skhan@...uxfoundation.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
        Suren Baghdasaryan <surenb@...gle.com>,
        "Liam R . Howlett" <Liam.Howlett@...cle.com>,
        Matthew Wilcox <willy@...radead.org>, Vlastimil Babka <vbabka@...e.cz>,
        "Paul E . McKenney" <paulmck@...nel.org>, Jann Horn <jannh@...gle.com>,
        David Hildenbrand <david@...hat.com>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, Muchun Song <muchun.song@...ux.dev>,
        Richard Henderson <richard.henderson@...aro.org>,
        Ivan Kokshaysky <ink@...assic.park.msu.ru>,
        Matt Turner <mattst88@...il.com>,
        Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        "James E . J . Bottomley" <James.Bottomley@...senpartnership.com>,
        Helge Deller <deller@....de>, Chris Zankel <chris@...kel.net>,
        Max Filippov <jcmvbkbc@...il.com>, Arnd Bergmann <arnd@...db.de>,
        linux-alpha@...r.kernel.org, linux-mips@...r.kernel.org,
        linux-parisc@...r.kernel.org, linux-arch@...r.kernel.org,
        Shuah Khan <shuah@...nel.org>, Christian Brauner <brauner@...nel.org>,
        linux-kselftest@...r.kernel.org,
        Sidhartha Kumar <sidhartha.kumar@...cle.com>,
        Jeff Xu <jeffxu@...omium.org>, Christoph Hellwig <hch@...radead.org>
Subject: Re: [PATCH 4/4] selftests/mm: add self tests for guard page feature

On Fri, Oct 18, 2024 at 10:25:55AM -0600, Shuah Khan wrote:
[snip]
> > > Sorry - this violates the coding styles and makes it hard to read.
> > >
> > > See process/coding-style.rst:
> > >
> > > Do not unnecessarily use braces where a single statement will do.
> > >
> > > .. code-block:: c
> > >
> > >          if (condition)
> > >                  action();
> > >
> > > and
> > >
> > > .. code-block:: c
> > >
> > >          if (condition)
> > >                  do_this();
> > >          else
> > >                  do_that();
> > >
> > > This does not apply if only one branch of a conditional statement is a single
> > > statement; in the latter case use braces in both branches:
> > >
> > > .. code-block:: c
> > >
> > >          if (condition) {
> > >                  do_this();
> > >                  do_that();
> > >          } else {
> > >                  otherwise();
> > >          }
> > >
> > > Also, use braces when a loop contains more than a single simple statement:
> > >
> > > .. code-block:: c
> > >
> > >          while (condition) {
> > >                  if (test)
> > >                          do_something();
> > >          }
> > >
> > > thanks,
> > > -- Shuah
> >
> > Shuah, quoting coding standards to an experienced kernel developer
> > (maintainer now) is maybe not the best way to engage here + it may have
> > been more productive for you to first engage on why it is I'm deviating
> > here.
> >
>
> This is not the only comment I gave you in this patch and your
> other patches.
>

And to be clear - I absolutely appreciate your feedback and in all cases
except ones which would result in things not compiling have acted (rather
promptly) to address them.

I am simply pointing out that it's not my lack of knowledge of these rules
that's the issue it's 3 things:

1. Some code doesn't compile following these rules
2. I therefore don't trust the macros in single-line statements
3. I am not a fan of for/while loops with heavily compound statements

1 and is unavoidable, 2 maybe is avoidable with auditing and 3 is
subjective, and is something I have now undertaken to change.

I've heard a number of kernel developers' opinions on checkpatch and the
overall consensus has been that, while the core style is sacrosanct, strict
adherence to checkpatch warnings is not.

This may be worth a broader discussion (outside of this series). One must
definitely account for cases where the syntactic analysis fails for
instance so 'always strictly adhere' is out unfortunately (why I say it is
fallible) however perhaps 'strict adherence except where it is obviously
wrong' is a position one could take.

Your have a significantly different view on this and that's fine, as I said
I always try to be accommodating.

Key thing is we've found a way forward which I will act on.

Thanks, Lorenzo