Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220326172051.14722-1-thepaulodoom@thepaulodoom.com>
 <CAMj1kXEQtTAMPLTtgc=9sDYvgxs+oihfnY7Q6bggC0p5u-V1Hw@mail.gmail.com> <20220327224009.2jotnczk67j4cfh2@hp-amd-paul>
In-Reply-To: <20220327224009.2jotnczk67j4cfh2@hp-amd-paul>
From:   Ard Biesheuvel <ardb@kernel.org>
Date:   Mon, 28 Mar 2022 09:39:14 +0200
Message-ID: <CAMj1kXHCR1nD24WDnYpD4Nu23x9+hw+=7EXOpq7y7m9LDk2J0w@mail.gmail.com>
Subject: Re: [PATCH] crypto: aes_generic: fixed styling warnings
To:     Paul Lemmermann <thepaulodoom@thepaulodoom.com>
Cc:     "David S. Miller" <davem@davemloft.net>,
        Linux Crypto Mailing List <linux-crypto@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

(please keep the cc's)

On Mon, 28 Mar 2022 at 00:46, Paul Lemmermann
<thepaulodoom@thepaulodoom.com> wrote:
>
> On Sun, Mar 27, 2022 at 01:41:19PM +0200, Ard Biesheuvel wrote:
> > On Sat, 26 Mar 2022 at 18:48, Paul Lemmermann
> > <thepaulodoom@thepaulodoom.com> wrote:
> > >
> > > Fixed all styling warnings from the checkpatch.pl script.
> > >
> > > Signed-off-by: Paul Lemmermann <thepaulodoom@thepaulodoom.com>
> >
> > Did you test this code after 'fixing' it?
> >
> No, I did not. Now that I scrutinized it a bit more, I realized the
> kernel coding conventions. Sorry about that, this is my first patch.

In that case, welcome!

This is not about coding conventions. This is about correctness.

For instance,

> > >
> > > -#define f_nround(bo, bi, k)    do {\
> > > +#define f_nround(bo, bi, k)    while (0) {\
> > >         f_rn(bo, bi, 0, k);     \
> > >         f_rn(bo, bi, 1, k);     \
> > >         f_rn(bo, bi, 2, k);     \
> > >         f_rn(bo, bi, 3, k);     \
> > >         k += 4;                 \
> > > -} while (0)
> > > +}
> > >

Why are you making this change, and why do you think it produces the
same result?

> Can you remove everything in the patch past the section with line
> 1144, or do I have to resubit the patch?
>

checkpatch.pl is a useful tool for finding style issues, but please
use it with care. And changing decades old code just to fix issues
reported by checkpatch.pl is really just pointless churn.

So let's just drop this patch altogether, shall we? If you're
interested in helping out, please have a look at the staging/ tree -
there is a lot of code there that needs cleaning up.

Thanks,
Ard.