Message ID | 5363562D.6030902@redhat.com |
---|---|
State | New |
Headers | show |
On 2 May 2014 09:24, Carlos O'Donell <carlos@redhat.com> wrote: Hi Carlos, > Is fixing the HAVE_CONFIG_H -Wundef warnings as easy as this? > > diff --git a/Makeconfig b/Makeconfig > index f965398..64b64fc 100644 > --- a/Makeconfig > +++ b/Makeconfig > @@ -1081,6 +1081,9 @@ endif > sysd-rules-targets := $(sort $(foreach p,$(sysd-rules-patterns),\ > $(firstword $(subst :, ,$p)))) > > +# We always configure glibc such that config.h is available. > +defines += -DHAVE_CONFIG_H=1 > + > # A sysdeps Makeconfig fragment may set libc-reentrant to yes. > ifeq (yes,$(libc-reentrant)) > defines += -D_LIBC_REENTRANT > --- > > Followed by the mechanical change of all the `#idfef HAVE_CONFIG_H` > to `#if HAVE_CONFIG_H`. > > Then verify the built binaries are identical and pass the testsuite? > > We always create a config.h for glibc so this looks correct. > > I don't want to waste my time fixing all of this if I've got it wrong. A reasonable number of these are from files shared with gnulib. The gnulib versions of the files have in some cases switched to use #ifndef _LIBC instead of #if HAVE_CONFIG_H but I haven't delved into why that is yet.
On 05/02/2014 04:28 AM, Will Newton wrote: > On 2 May 2014 09:24, Carlos O'Donell <carlos@redhat.com> wrote: > > Hi Carlos, > >> Is fixing the HAVE_CONFIG_H -Wundef warnings as easy as this? >> >> diff --git a/Makeconfig b/Makeconfig >> index f965398..64b64fc 100644 >> --- a/Makeconfig >> +++ b/Makeconfig >> @@ -1081,6 +1081,9 @@ endif >> sysd-rules-targets := $(sort $(foreach p,$(sysd-rules-patterns),\ >> $(firstword $(subst :, ,$p)))) >> >> +# We always configure glibc such that config.h is available. >> +defines += -DHAVE_CONFIG_H=1 >> + >> # A sysdeps Makeconfig fragment may set libc-reentrant to yes. >> ifeq (yes,$(libc-reentrant)) >> defines += -D_LIBC_REENTRANT >> --- >> >> Followed by the mechanical change of all the `#idfef HAVE_CONFIG_H` >> to `#if HAVE_CONFIG_H`. >> >> Then verify the built binaries are identical and pass the testsuite? >> >> We always create a config.h for glibc so this looks correct. >> >> I don't want to waste my time fixing all of this if I've got it wrong. > > A reasonable number of these are from files shared with gnulib. The > gnulib versions of the files have in some cases switched to use > #ifndef _LIBC instead of #if HAVE_CONFIG_H but I haven't delved into > why that is yet. I can ignore the gnulib files, thanks to your nice list, and I can change only glibc ones to use `#if HAVE_CONFIG_H`, but still define `-DHAVE_CONFIG_H=1`, that way we're ready for the gnulib update. I've been looking at the gettext update today, but it's more work than I have time for right now (repeating myself today). Cheers, Carlos.
On 05/02/2014 01:28 AM, Will Newton wrote: > The > gnulib versions of the files have in some cases switched to use > #ifndef _LIBC instead of #if HAVE_CONFIG_H but I haven't delved into > why that is yet. Some years ago gnulib changed, and it now assumes that <config.h> always exists. We surround the include with "#ifndef _LIBC" only for files shared with glibc. If glibc is also going to assume config.h exists, I suggest getting rid of the #if, and using "#include <config.h>" unconditionally; that's simplest.
> Roland, > > Is fixing the HAVE_CONFIG_H -Wundef warnings as easy as this? It's relatively easy, but this is not it. Firstly, never ever use -D rather than #define unless you have a very serious justification. (It's hostile to incremental builds, since GNU make does not track command line changes.) We do have a config.h but it's already implicitly included for every file. libc-symbols.h does #include <config.h>, and every compile gets libc-symbols.h via the -include switch. So !HAVE_CONFIG_H is the correct state for libc. Our config.h does not have a multiple-inclusion guard. OTOH, it so happens that all the contents of config.h are things that are silently harmless to repeat (#undef of things already #undef'd, #define of things to the identical value already #define'd). That being the case, it's already harmless to have it included more than once--and it would also be harmless to give it a multiple inclusion guard, unnecessary though that is. But what's really right is to have it included only once, which means just the existing once via libc-symbols.h and never directly in any source file. The obvious thing is to put "#define HAVE_CONFIG_H 0" right after "#include <config.h>" in libc-symbols.h. That won't do what we want today, because there are many files that use #ifdef rather than #if for the test. Since any mention of <config.h> at all should only appear in source files shared with other projects for their benefit, it will require coordination to change them to #if and be sure that is OK for the other users. Conversely, if the projects that share these files and care about config.h all use #ifdef uniformly, then we could just change the small minority that use #if today to use #ifdef and never define HAVE_CONFIG_H anywhere in the libc build. Thanks, Roland
diff --git a/Makeconfig b/Makeconfig index f965398..64b64fc 100644 --- a/Makeconfig +++ b/Makeconfig @@ -1081,6 +1081,9 @@ endif sysd-rules-targets := $(sort $(foreach p,$(sysd-rules-patterns),\ $(firstword $(subst :, ,$p)))) +# We always configure glibc such that config.h is available. +defines += -DHAVE_CONFIG_H=1 + # A sysdeps Makeconfig fragment may set libc-reentrant to yes. ifeq (yes,$(libc-reentrant)) defines += -D_LIBC_REENTRANT