[CCing build-system folks and others likely to know about potential
issues.]
Does anyone have any objection to the use of "#pragma once" instead of
the usual #ifndef-#define-...-#endif include guard? GCC, LLVM/clang,
and the latest Sparse all support either method just fine. (I added
support to Sparse myself.) Both have equivalent performance. "#pragma
once" is simpler, and avoids the possibility of a typo in the defined
guard symbol.
That's not a theoretical concern: I've found quite a few headers in the
kernel with typoed (and thus non-functional) include guards. Rashika
Kheria (CCed) ran into one such header when attempting to add some
includes to fix some warnings (resulting in the broken header being
included twice), and that prompted me to look for other headers with
broken guards. I've included a Python script at the end of this mail
that finds many such issues, though it does have a few false positives
on files that don't have include guards at all.
I'm not suggesting a mass conversion from include guards to "#pragma
once", though that *would* be fairly easy to script and review if
desired. I'd just like to verify that "#pragma once" seems acceptable
before, for instance, converting the headers that already have broken
include guards, adding recommendations in Documentation to standardize
on the pragma for new headers, and possibly adding checks in checkpatch
or similar if I can come up with one that has minimal false positives.
Python script to check header guards (run with $(find -name '*.h') as
arguments, and grep for "mismatch" in the output if you don't care about
files without guards):
#!/usr/bin/python
import re
import sys
start = re.compile("[ \t]*#[ \t]*(ifndef|define)[ \t]+([a-zA-Z0-9_]*)")
def main(args):
for header in args[1:]:
symbol = None
for line in open(header):
m = start.match(line)
if not m:
if symbol:
print "{}: does not appear to use ifndef-based include guard".format(header)
break
continue
if m.group(1) == "ifndef":
if symbol is not None:
print "{}: does not appear to use ifndef-based include guard".format(header)
break
symbol = m.group(2)
elif m.group(1) == "define":
if symbol is None:
print "{}: does not appear to use ifndef-based include guard".format(header)
break
if symbol != m.group(2):
print "{}: include guard symbol mismatch: #ifndef {} with #define {}".format(header, symbol, m.group(2))
break
else:
if symbol:
print "{}: ifndef {} with no subsequent define".format(header, symbol)
if __name__ == "__main__":
sys.exit(main(sys.argv))
On Mon, 6 Jan 2014 12:47:07 -0800 Josh Triplett <[email protected]> wrote:
> Does anyone have any objection to the use of "#pragma once" instead of
> the usual #ifndef-#define-...-#endif include guard?
Sounds OK to me. gcc has supported this for quite a long time, yes? I
wonder if ICC supports it.
(I haven't heard anything of ICC-for-kernel in a long time - is it still
a living thing?)
On Mon, Jan 06, 2014 at 01:00:03PM -0800, Andrew Morton wrote:
> On Mon, 6 Jan 2014 12:47:07 -0800 Josh Triplett <[email protected]> wrote:
>
> > Does anyone have any objection to the use of "#pragma once" instead of
> > the usual #ifndef-#define-...-#endif include guard?
>
> Sounds OK to me. gcc has supported this for quite a long time, yes? I
> wonder if ICC supports it.
Yes, ICC supports it as well, if anyone cares.
- Josh Triplett
On Mon, Jan 06, 2014 at 12:47:07PM -0800, Josh Triplett wrote:
> Does anyone have any objection to the use of "#pragma once" instead of
> the usual #ifndef-#define-...-#endif include guard? GCC, LLVM/clang,
> and the latest Sparse all support either method just fine. (I added
> support to Sparse myself.) Both have equivalent performance. "#pragma
> once" is simpler, and avoids the possibility of a typo in the defined
> guard symbol.
Does anybody know whether other static code analysis tools such as
Coverity can handle #pragma once?
- Ted
On Mon, Jan 06, 2014 at 04:33:49PM -0500, Theodore Ts'o wrote:
> On Mon, Jan 06, 2014 at 12:47:07PM -0800, Josh Triplett wrote:
> > Does anyone have any objection to the use of "#pragma once" instead of
> > the usual #ifndef-#define-...-#endif include guard? GCC, LLVM/clang,
> > and the latest Sparse all support either method just fine. (I added
> > support to Sparse myself.) Both have equivalent performance. "#pragma
> > once" is simpler, and avoids the possibility of a typo in the defined
> > guard symbol.
>
> Does anybody know whether other static code analysis tools such as
> Coverity can handle #pragma once?
Coverity should be fine. If it does break, I'm sure they'd fix it.
Dave
On Mon, Jan 06, 2014 at 12:47:07PM -0800, Josh Triplett wrote:
> [CCing build-system folks and others likely to know about potential
> issues.]
>
> Does anyone have any objection to the use of "#pragma once" instead of
> the usual #ifndef-#define-...-#endif include guard? GCC, LLVM/clang,
> and the latest Sparse all support either method just fine. (I added
> support to Sparse myself.) Both have equivalent performance. "#pragma
> once" is simpler, and avoids the possibility of a typo in the defined
> guard symbol.
For kernel headers no concern.
For UAPI headers we should be more carefull - as we do not know which
compiler it ends up seeing - and what version.
Sam
On Tue, Jan 7, 2014 at 6:55 AM, Sam Ravnborg <[email protected]> wrote:
> On Mon, Jan 06, 2014 at 12:47:07PM -0800, Josh Triplett wrote:
>> [CCing build-system folks and others likely to know about potential
>> issues.]
>>
>> Does anyone have any objection to the use of "#pragma once" instead of
>> the usual #ifndef-#define-...-#endif include guard? GCC, LLVM/clang,
>> and the latest Sparse all support either method just fine. (I added
>> support to Sparse myself.) Both have equivalent performance. "#pragma
>> once" is simpler, and avoids the possibility of a typo in the defined
>> guard symbol.
> For kernel headers no concern.
Just being cautious:
Do we know the minimum gcc version that supports #pragma once?
Furthermore I found this:
| #pragma once does have one drawback (other than being non-standard) and
| that is if you have the same file in different locations then the
compiler will
| think these are different files.
http://stackoverflow.com/questions/787533/is-pragma-once-a-safe-include-guard
With asm-generic and uapi, do we have multiple header files that
deliberately use the same include guards?
I know we have header files that deliberately don't have include guards
(e.g. asm/unistd.h on some architectures).
> For UAPI headers we should be more carefull - as we do not know which
> compiler it ends up seeing - and what version.
Furthermore some userspace may rely on doing #define XXX to avoid
including a specific kernel header (yes, it's ugly).
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On 2014-01-07 10:48, Geert Uytterhoeven wrote:
> Furthermore some userspace may rely on doing #define XXX to avoid
> including a specific kernel header (yes, it's ugly).
This pattern is also sometimes used:
$ head -6 include/linux/spinlock_up.h
#ifndef __LINUX_SPINLOCK_UP_H
#define __LINUX_SPINLOCK_UP_H
#ifndef __LINUX_SPINLOCK_H
# error "please don't include this file directly"
#endif
And there is nothing ugly about it. So #pragma once is probably a good
idea for most headers that are not exposed to userspace. But making it a
requirement in scripts/checkpatch.pl or Documentation/CodingStyle means
that it will become hard to defend the few legitimate uses of ifndef
guards against people who have a printed copy of checkpatch.pl under
their pillow.
Michal
On Tue, Jan 07, 2014 at 11:09:11AM +0100, Michal Marek wrote:
> On 2014-01-07 10:48, Geert Uytterhoeven wrote:
> > Furthermore some userspace may rely on doing #define XXX to avoid
> > including a specific kernel header (yes, it's ugly).
>
> This pattern is also sometimes used:
> $ head -6 include/linux/spinlock_up.h
> #ifndef __LINUX_SPINLOCK_UP_H
> #define __LINUX_SPINLOCK_UP_H
>
> #ifndef __LINUX_SPINLOCK_H
> # error "please don't include this file directly"
> #endif
>
> And there is nothing ugly about it.
That's debatable, but it's certainly reasonable to try to enforce
non-inclusion of "internal" headers directly. However, for headers not
exposed to userspace, it'd be easy to write that as:
include/linux/spinlock.h:
#pragma once
#define LINUX_SPINLOCK_H_INCLUDED
include/linux/spinlock_up.h:
#pragma once
#ifndef LINUX_SPINLOCK_H_INCLUDED
#error "Only include this file via spinlock.h, never directly"
#endif
> So #pragma once is probably a good
> idea for most headers that are not exposed to userspace. But making it a
> requirement in scripts/checkpatch.pl or Documentation/CodingStyle means
> that it will become hard to defend the few legitimate uses of ifndef
> guards against people who have a printed copy of checkpatch.pl under
> their pillow.
Any mention in CodingStyle or check in checkpatch would need to cover
the two exceptions: uapi, and headers that are intentionally parsed
multiple times for preprocessor magic (TRACE_HEADER_MULTI_READ).
- Josh Triplett
On Tue, Jan 07, 2014 at 10:48:53AM +0100, Geert Uytterhoeven wrote:
> On Tue, Jan 7, 2014 at 6:55 AM, Sam Ravnborg <[email protected]> wrote:
> > On Mon, Jan 06, 2014 at 12:47:07PM -0800, Josh Triplett wrote:
> >> [CCing build-system folks and others likely to know about potential
> >> issues.]
> >>
> >> Does anyone have any objection to the use of "#pragma once" instead of
> >> the usual #ifndef-#define-...-#endif include guard? GCC, LLVM/clang,
> >> and the latest Sparse all support either method just fine. (I added
> >> support to Sparse myself.) Both have equivalent performance. "#pragma
> >> once" is simpler, and avoids the possibility of a typo in the defined
> >> guard symbol.
> > For kernel headers no concern.
>
> Just being cautious:
>
> Do we know the minimum gcc version that supports #pragma once?
>From checking the manuals, it goes back to at least 2.95. Searching
suggests that versions before 3.4 have a few bugs in "#pragma once"
support, but that those bugs only apply to using #pragma once in
combination with precompiled headers, which doesn't apply to the kernel.
- Josh Triplett
On Mon, Jan 6, 2014 at 3:47 PM, Josh Triplett <[email protected]> wrote:
> Does anyone have any objection to the use of "#pragma once" instead of
> the usual #ifndef-#define-...-#endif include guard? GCC, LLVM/clang,
> and the latest Sparse all support either method just fine. (I added
> support to Sparse myself.) Both have equivalent performance. "#pragma
> once" is simpler, and avoids the possibility of a typo in the defined
> guard symbol.
Unfortunately in GCC #pragma once is slower and slightly buggier than
regular include guards:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52566
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58770
On Sun, Jan 12, 2014 at 11:14:56AM -0500, Patrick Palka wrote:
> On Mon, Jan 6, 2014 at 3:47 PM, Josh Triplett <[email protected]> wrote:
> > Does anyone have any objection to the use of "#pragma once" instead of
> > the usual #ifndef-#define-...-#endif include guard? GCC, LLVM/clang,
> > and the latest Sparse all support either method just fine. (I added
> > support to Sparse myself.) Both have equivalent performance. "#pragma
> > once" is simpler, and avoids the possibility of a typo in the defined
> > guard symbol.
>
> Unfortunately in GCC #pragma once is slower and slightly buggier than
> regular include guards:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52566
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58770
The bug in question doesn't seem to apply to any likely use in the Linux
kernel.
As for performance, that benchmark seems somewhat odd to me, and I'm not
entirely convinced that it's representative of real-world projects,
which don't typically include ten thousand include files in one
compilation. I've benchmarked the case of a single main.c including a
single guarded.h with both types of guards, and found the two guard
types indistinguishable in performance. Beyond that, I'd say real-world
benchmarks would be preferable to artificial ones before declaring a
performance difference between the two types of guards.
- Josh Trpilett