Message ID | 5363562D.6030902@redhat.com |
---|---|
State | Not applicable |
Headers |
Return-Path: <x14307373@homiemail-mx22.g.dreamhost.com> X-Original-To: siddhesh@wilcox.dreamhost.com Delivered-To: siddhesh@wilcox.dreamhost.com Received: from homiemail-mx22.g.dreamhost.com (mx2.sub5.homie.mail.dreamhost.com [208.113.200.128]) by wilcox.dreamhost.com (Postfix) with ESMTP id 4832736007D for <siddhesh@wilcox.dreamhost.com>; Fri, 2 May 2014 01:24:20 -0700 (PDT) Received: by homiemail-mx22.g.dreamhost.com (Postfix, from userid 14307373) id E361451BB207; Fri, 2 May 2014 01:24:19 -0700 (PDT) X-Original-To: glibc@patchwork.siddhesh.in Delivered-To: x14307373@homiemail-mx22.g.dreamhost.com Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by homiemail-mx22.g.dreamhost.com (Postfix) with ESMTPS id 8FA36508C738 for <glibc@patchwork.siddhesh.in>; Fri, 2 May 2014 01:24:19 -0700 (PDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:message-id:date:from:mime-version:to:subject :content-type:content-transfer-encoding; q=dns; s=default; b=cS2 vdSWxs66HzT2CutYZjWDWXxIqJ+KFypogzRR5i5CwtvrgZJD9hADk6C0qGvfCq9N K7HesbiyDiHSoNqUWRjeIWRqMwGNpJnhoZwhQB125rbMl4xB+oHtOJFFcb0W/wU5 h5WPxAgPHFWmNwR5fGjS6NiUr5xbKtyyPHJO6LDg= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:message-id:date:from:mime-version:to:subject :content-type:content-transfer-encoding; s=default; bh=O+0p2zV8E TwlMe/EAM7VXLp/ZN0=; b=Y0r38SqCVogA88YIzKKU5z4WNrfeZT0EfbIyo5Al5 DgBC2eINMEEeaDfs0lcpbOAvvFXUDQKQvmQY8M+PyuM5J8tsT7DWFZ1Y77bBZzdH FpscGyk3Y/CfH0/be3WzfsN3sdvr4EnmGzs6Q4DZbk7ae94X3PWWW4ZcfFgEIvM4 Xg= Received: (qmail 25734 invoked by alias); 2 May 2014 08:24:17 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-glibc=patchwork.siddhesh.in@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 25722 invoked by uid 89); 2 May 2014 08:24:17 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.0 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Message-ID: <5363562D.6030902@redhat.com> Date: Fri, 02 May 2014 04:24:13 -0400 From: "Carlos O'Donell" <carlos@redhat.com> User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.3.0 MIME-Version: 1.0 To: GNU C Library <libc-alpha@sourceware.org>, Roland McGrath <roland@hack.frob.com> Subject: [WIP] Fix HAVE_CONFIG_H -Wundef warnings. Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-DH-Original-To: glibc@patchwork.siddhesh.in |
Commit Message
Carlos O'Donell
May 2, 2014, 8:24 a.m. UTC
Roland, Is fixing the HAVE_CONFIG_H -Wundef warnings as easy as this? --- 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. Cheers, Carlos.
Comments
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