linux-kernel - Re: [PATCH] crypto: aes_generic: fixed styling warnings

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-next>] [day] [month] [year] [list]

Date:   Mon, 28 Mar 2022 09:39:14 +0200
From:   Ard Biesheuvel <ardb@...nel.org>
To:     Paul Lemmermann <thepaulodoom@...paulodoom.com>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Linux Crypto Mailing List <linux-crypto@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] crypto: aes_generic: fixed styling warnings

(please keep the cc's)

On Mon, 28 Mar 2022 at 00:46, Paul Lemmermann
<thepaulodoom@...paulodoom.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@...paulodoom.com> wrote:
> > >
> > > Fixed all styling warnings from the checkpatch.pl script.
> > >
> > > Signed-off-by: Paul Lemmermann <thepaulodoom@...paulodoom.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.