From patchwork Fri Sep 18 21:09:14 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Wilco Dijkstra X-Patchwork-Id: 8802 Received: (qmail 35088 invoked by alias); 18 Sep 2015 21:09:27 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 35077 invoked by uid 89); 18 Sep 2015 21:09:26 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL, BAYES_00, SPF_PASS autolearn=ham version=3.3.2 X-HELO: eu-smtp-delivery-143.mimecast.com From: "Wilco Dijkstra" To: "'Joseph Myers'" Cc: "'GNU C Library'" References: <002501d0f24b$8fc25110$af46f330$@com> <002601d0f24d$13032080$39096180$@com> In-Reply-To: Subject: RE: [COMMITTED] Fix GCC6 build errors due to unused statics Date: Fri, 18 Sep 2015 22:09:14 +0100 Message-ID: <002701d0f256$461365d0$d23a3170$@com> MIME-Version: 1.0 X-MC-Unique: TD0QE3wvRceuzuUTJIJi5Q-1 > Joseph Myers wrote: > On Fri, 18 Sep 2015, Wilco Dijkstra wrote: > > > > Joseph Myers wrote: > > > On Fri, 18 Sep 2015, Wilco Dijkstra wrote: > > > > > > > * timezone/private.h (time_t_min): Likewise. (time_t_max): > > > > Likewise. > > > > > > The timezone/ code is imported verbatim from upstream tzcode releases and > > > should not be changed locally. Please revert this change and work with > > > Paul on getting a fix into upstream tzcode. When there's a new tzcode > > > release, you can then import it into glibc. (Some Makefile changes will > > > be needed as part of the import of new code; see > > > . Do not try > > > to combine this with importing a new version of tzdata.) > > > > Would it be OK to change the Makefile instead to ignore this error? > > That (specific to the timezone/ directory, of course) would be a > reasonable temporary fix until a new tzcode release is out, if it takes a > while to get a fixed release out. OK, I've reverted the private.h change and changed timezone/Makefile instead (see below). > In general: when you see a build failure resulting from a new GCC warning, > the first reaction should *not* be to check in a glibc change as fast as > possible. First, you should consider whether the warning is in fact > desirable for glibc - and whether it belongs in -Wall / enabled-by-default > warnings at all, or whether the warnings seen building glibc are typical > of widespread usage and provide evidence that the coding style constraint > implied is inappropriate for -Wall. Then provide the evidence in the > gcc-patches discussion, and engage with the discussion there to help reach > a conclusion on the appropriateness of the warning. > > In this case, (a) the gcc-patches discussion has people saying they think > inclusion in -Wall is a bad idea, and (b) I specifically said in that > discussion not to change timezone/private.h locally in glibc > . > > Now, removing unused variables seems a reasonable cleanup in glibc, > independent of the warning's inclusion in -Wall - *provided* that all the > other constraints regarding generated files, files imported from other > sources etc. are met. But in general for new warnings fixing them in > glibc may not be the right thing at all, if it seems plausible the warning > might be removed from -Wall, and so you need to wait for the gcc-patches > discussion to reach consensus before making changes that can't be > justified independently of the warning. I'd like to ensure GLIBC remains buildable with up to date GCC versions - otherwise it becomes hard to run automated builds and tests on trunk. Even just building/testing GLIBC is becoming problematic due to the large number of spurious errors one encounters after updating GLIC and GCC (which after all one is forced to do when checking in changes...). If it hasn't been decided which warnings to enable in GCC6 (sorry I haven't followed all the discussions...) then maybe we should not use -Werror at all when building GLIBC with trunk GCC and only print warnings until all this has been decided and any issues fixed. Wilco 2015-09-18 Wilco Dijkstra * timezone/Makefile: Ignore unused variable errors due to private.h (time_t_min) and (time_t_max). * timezone/private.h (time_t_min): Revert removal. (time_t_max): Likewise. diff --git a/timezone/Makefile b/timezone/Makefile index bfb3463..afd9e88 100644 --- a/timezone/Makefile +++ b/timezone/Makefile @@ -61,10 +61,12 @@ tz-cflags = -DTZDIR='"$(zonedir)"' \ -DTZDEFRULES='"$(posixrules-file)"' \ -DTM_GMTOFF=tm_gmtoff -DTM_ZONE=tm_zone -CFLAGS-zdump.c = -fwrapv -DNOID $(tz-cflags) -DHAVE_GETTEXT -CFLAGS-zic.c = -DNOID $(tz-cflags) -DHAVE_GETTEXT -CFLAGS-ialloc.c = -DNOID -DHAVE_GETTEXT -CFLAGS-scheck.c = -DNOID -DHAVE_GETTEXT +CFLAGS-zdump.c = -fwrapv -DNOID $(tz-cflags) -DHAVE_GETTEXT \ + -Wno-error=unused-const-variable +CFLAGS-zic.c = -DNOID $(tz-cflags) -DHAVE_GETTEXT \ + -Wno-error=unused-const-variable +CFLAGS-ialloc.c = -DNOID -DHAVE_GETTEXT -Wno-error=unused-const-variable +CFLAGS-scheck.c = -DNOID -DHAVE_GETTEXT -Wno-error=unused-const-variable # We have to make sure the data for testing the tz functions is available. # Don't add leapseconds here since test-tz made checks that work only without diff --git a/timezone/private.h b/timezone/private.h index ed19e06..4e8f4ae 100644 --- a/timezone/private.h +++ b/timezone/private.h @@ -326,6 +326,16 @@ const char * scheck(const char * string, const char * format); #define TYPE_SIGNED(type) (((type) -1) < 0) #endif /* !defined TYPE_SIGNED */ +/* The minimum and maximum finite time values. */ +static time_t const time_t_min = + (TYPE_SIGNED(time_t) + ? (time_t) -1 << (CHAR_BIT * sizeof (time_t) - 1) + : 0); +static time_t const time_t_max = + (TYPE_SIGNED(time_t) + ? - (~ 0 < 0) - ((time_t) -1 << (CHAR_BIT * sizeof (time_t) - 1)) + : -1); + #ifndef INT_STRLEN_MAXIMUM /* ** 302 / 1000 is log10(2.0) rounded up.