| Message ID | 30a12210-dbcc-f92f-ad59-2b1c5ad10136@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Failing misc/check-installed-headers-c with new kernel headers. | expand |
* Carlos O'Donell: > Should we expand the regexp in some way to ignore text within single-line > comments that we can easily detect? I think we should use #pragma GCC poison in $cih_test_c, like this: diff --git a/scripts/check-installed-headers.sh b/scripts/check-installed-headers.sh index 8e7beffd82..f9e39eb995 100644 --- a/scripts/check-installed-headers.sh +++ b/scripts/check-installed-headers.sh @@ -144,6 +144,12 @@ EOF inappropriate for this test. */ #undef _LIBC #undef _GNU_SOURCE + +/* Obsolete type names. */ +#pragma GCC poison ushort +#pragma GCC poison ulong +#pragma GCC poison uint + /* The library mode is selected here rather than on the command line to ensure that this selection wins. */ $expanded_lib_mode And then get rid of the grep. Thanks, Florian
On 1/16/19 4:21 PM, Florian Weimer wrote: > * Carlos O'Donell: > >> Should we expand the regexp in some way to ignore text within single-line >> comments that we can easily detect? > > I think we should use #pragma GCC poison in $cih_test_c, like this: > > diff --git a/scripts/check-installed-headers.sh b/scripts/check-installed-headers.sh > index 8e7beffd82..f9e39eb995 100644 > --- a/scripts/check-installed-headers.sh > +++ b/scripts/check-installed-headers.sh > @@ -144,6 +144,12 @@ EOF > inappropriate for this test. */ > #undef _LIBC > #undef _GNU_SOURCE > + > +/* Obsolete type names. */ > +#pragma GCC poison ushort > +#pragma GCC poison ulong > +#pragma GCC poison uint > + > /* The library mode is selected here rather than on the command line to > ensure that this selection wins. */ > $expanded_lib_mode > > And then get rid of the grep. I like this solution, but it drastically complicates the test. It's certainly more robust than the regexp. The problem is that you have to split the tail part of the test into 2 compilations. One to test for obsolete types, and another to test for the correct compilation under the std/macro test matrix. You can no longer keep the two together because the poison always triggers on the old types in the base headers. And when you're looking for poisoning you now have to go through all the raised compiler errors just like you would for headers, and exclude any from headers that are allowed to have the obsolete types. I guess all-in-all it's more robust, but more work than I was expecting :/
* Carlos O'Donell: > On 1/16/19 4:21 PM, Florian Weimer wrote: >> * Carlos O'Donell: >> >>> Should we expand the regexp in some way to ignore text within single-line >>> comments that we can easily detect? >> >> I think we should use #pragma GCC poison in $cih_test_c, like this: >> >> diff --git a/scripts/check-installed-headers.sh b/scripts/check-installed-headers.sh >> index 8e7beffd82..f9e39eb995 100644 >> --- a/scripts/check-installed-headers.sh >> +++ b/scripts/check-installed-headers.sh >> @@ -144,6 +144,12 @@ EOF >> inappropriate for this test. */ >> #undef _LIBC >> #undef _GNU_SOURCE >> + >> +/* Obsolete type names. */ >> +#pragma GCC poison ushort >> +#pragma GCC poison ulong >> +#pragma GCC poison uint >> + >> /* The library mode is selected here rather than on the command line to >> ensure that this selection wins. */ >> $expanded_lib_mode >> >> And then get rid of the grep. > > I like this solution, but it drastically complicates the test. It does not work because use in an unexpanded macro does not count as use for poisoning. I find that rather surprising, based on the documentation of the pragma. Thanks, Florian
On Wed, Jan 16, 2019 at 4:11 PM Carlos O'Donell <carlos@redhat.com> wrote: > > In the latest Fedora Rawhide weekly glibc rebase I'm seeing > a failure across all arches. ... > BUILDSTDERR: *** Obsolete types detected: > BUILDSTDERR: /usr/include/linux/sysctl.h: KERN_PANIC_PRINT=78, /* ulong: bitmask to print system info on panic */ ... > This is a false positive from the regexp used to search for obsolete types. I propose to fix this by separating the obsolete-typedefs test to a new program, implemented in Python, which lexes C accurately enough that it won't get false positives on the contents of strings and comments. It will also not look at headers that aren't under our control. Patch shortly. zw
diff --git a/include/uapi/linux/sysctl.h b/include/uapi/linux/sysctl.h index d71013fffaf6..87aa2a6d9125 100644 --- a/include/uapi/linux/sysctl.h +++ b/include/uapi/linux/sysctl.h @@ -153,6 +153,7 @@ enum KERN_NMI_WATCHDOG=75, /* int: enable/disable nmi watchdog */ KERN_PANIC_ON_NMI=76, /* int: whether we will panic on an unrecovered */ KERN_PANIC_ON_WARN=77, /* int: call panic() in WARN() functions */ + KERN_PANIC_PRINT=78, /* ulong: bitmask to print system info on panic */ }; --- The obsolete type is shorthand in a comment. Should we expand the regexp in some way to ignore text within single-line comments that we can easily detect? Or is the accepted fix something like this (untested)? Seems a shame not to check sysctl.h. However use of legacy types seem fairly rare these days. diff --git a/scripts/check-installed-headers.sh b/scripts/check-installed-headers.sh index 8e7beffd82..1742c2bf20 100644 --- a/scripts/check-installed-headers.sh +++ b/scripts/check-installed-headers.sh @@ -127,6 +127,12 @@ EOF ;; esac ;; + + # linux/sysctl.h is unsupported for all arches because it contains + # a comment with 'ulong' as shorthand for a type. + (sys/sysctl.h) + continue;; + esac echo :: "$header"