linux-kernel - Re: [PATCH 02/25] tracing: Improve "if" macro code generation

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]

Date:   Wed, 20 Mar 2019 10:26:17 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     David Laight <David.Laight@...lab.com>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "hpa@...or.com" <hpa@...or.com>,
        "julien.thierry@....com" <julien.thierry@....com>,
        "will.deacon@....com" <will.deacon@....com>,
        "luto@...capital.net" <luto@...capital.net>,
        "mingo@...nel.org" <mingo@...nel.org>,
        "catalin.marinas@....com" <catalin.marinas@....com>,
        "james.morse@....com" <james.morse@....com>,
        "valentin.schneider@....com" <valentin.schneider@....com>,
        "brgerst@...il.com" <brgerst@...il.com>,
        "jpoimboe@...hat.com" <jpoimboe@...hat.com>,
        "luto@...nel.org" <luto@...nel.org>, "bp@...en8.de" <bp@...en8.de>,
        "dvlasenk@...hat.com" <dvlasenk@...hat.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "dvyukov@...gle.com" <dvyukov@...gle.com>,
        "rostedt@...dmis.org" <rostedt@...dmis.org>
Subject: Re: [PATCH 02/25] tracing: Improve "if" macro code generation

On Wed, Mar 20, 2019 at 4:17 AM David Laight <David.Laight@...lab.com> wrote:
>
> >               ______r = !!(cond);                                     \
>
>         Is that (or maybe just the !!) needed any more??

It is, because the 'cond' expression might not be an int, it could be
a test for a pointer being non-NULL, or an u64 being non-zero, and not
having the "!!" would mean that you'd get a warning or drop bits when
assigning to 'int'.

And you do need the new temporary variable to avoid double evaluation
the way that code is written.

That said, I do think the code is really ugly. We could:

 - avoid the temporary by just simplifying things.

 - do the '!!' just once in the parent macro.

 - Steven has this crazy model of "more underscores are better". They
aren't. They don't help if things nest anyway, but what does help is
meaningful names. Both when things don't nest, and when looking at
generated asm files.

 - ,, and finally, what _is_ better is to chop things up so that they
are smaller and make each macro do only one thing

So maybe do the patch something like the attached instead? Completely
untested, but it looks sane to me.

                  Linus

View attachment "patch.diff" of type "text/x-patch" (1490 bytes)