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