linux-kernel - Re: [PATCH 00/11] pragma once: treewide conversion

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: <CAHk-=wjFWZMVWTbvUMVxQqGKvGMC_BNrahCtTkpEjxoC0k-T=A@mail.gmail.com>
Date:   Sun, 28 Feb 2021 09:46:17 -0800
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Alexey Dobriyan <adobriyan@...il.com>,
        Luc Van Oostenryck <luc.vanoostenryck@...il.com>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Sparse Mailing-list <linux-sparse@...r.kernel.org>
Subject: Re: [PATCH 00/11] pragma once: treewide conversion

On Sun, Feb 28, 2021 at 8:57 AM Alexey Dobriyan <adobriyan@...il.com> wrote:
>
> This is bulk deletion of preprocessor include guards and conversion
> to #pragma once directive.

So as mentioned earlier, I'm not 100% convinced about the advantage of
#pragma once.

But I decided to actually test it, and it turns out that it causes
problems for at least sparse.

Sparse *does* support pragma once, but it does it purely based on
pathname equality. So a simple test-program like this:

 File 'pragma.h':

    #pragma once
    #include "header.h"

works fine. But this doesn't work at all:

    #pragma once
    #include "./header.h"

because it causes the filename to be different every time, and you
eventually end up with trying to open   "././....../pragma.h" and it
causes ENAMETOOLONG.

So at least sparse isn't ready for this.

I guess sparse could always simplify the name, but that's non-trivial.

And honestly, using st_dev/st_ino is problematic too, since

 (a) they can easily be re-used for generated files

 (b) you'd have to actually open/fstat the filename to use it, which
obviates one of the optimizations

Trying the same on gcc, you don't get that endless "add "./" behavior"
that sparse did, but a quick test shows that it actually opens the
file and reads it three times: once for "pramga.h", once for
"./pragma.h" and a third time for "pragma.h". It only seems to
_expand_ it once, though.

I have no idea what gcc does. Maybe it does some "different name, so
let's open and read it, and then does st_dev/st_ino again". But if so,
why the _third_ time? Is it some guard against "st_ino might have been
re-used, so I'll open the original name and re-verify"?

End result: #pragma is fundamentally less reliable than the
traditional #ifdef guard. The #ifdef guard works fine even if you
re-read the file for whatever reason, while #pragma relies on some
kind of magical behavior.

I'm adding Luc in case he has any ideas of what the magical behavior might be.

Honestly, I think #pragma once is complete garbage. It's really is
fundamenetally more complicated than the #ifdef guard, and it has
absolutely zero upsides.

I'm not taking this pramga series unless somebody can explain why it's
a good thing. Because all I see are downsides.

               Linus