| Message ID | 002701d0f256$461365d0$d23a3170$@com |
|---|---|
| State | New |
| Headers | show |
On Fri, 18 Sep 2015, Wilco Dijkstra wrote: > OK, I've reverted the private.h change and changed timezone/Makefile instead > (see below). I take it you made sure the -Wno-error= options work with GCC versions that don't have the new warning option? > I'd like to ensure GLIBC remains buildable with up to date GCC versions - That doesn't affect anything I said. In the GNU Project we need to work out the appropriate place to fix things involving more than one project rather than jumping to conclusions about which place to change. And in glibc changes need consensus - the fact that something fixes a warning with new GCC does not make it (in general - unused variables in glibc-local code are a reasonable exception) an obvious fix that can bypass the process of obtaining consensus (which includes considering whether the warning should actually be in -Wall at all, and which of possibly several approaches to a fix in glibc makes the most sense if fixing in glibc). glibc releases should be buildable with GCC releases. It's expected that there will often be breakage with GCC development versions that will take time to reach consensus on a fix. > 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. If you want to detect glibc regressions, you should be building glibc with stable binutils and GCC versions. If you want to detect regressions in GCC, upgrade GCC while keeping the versions of the other projects fixed. If you find such a regression in GCC (new GCC no longer builds a glibc version older GCC built) then you get to seek consensus about the right fix. Upgrading more than one component at a time means it may be less clear which component a regression is in.
On Fri, 18 Sep 2015, Wilco Dijkstra wrote: > 2015-09-18 Wilco Dijkstra <wdijkstr@arm.com> > > * timezone/Makefile: Ignore unused variable errors due to private.h > (time_t_min) and (time_t_max). I've reverted this timezone/Makefile change since it broke the build for all released versions of GCC, which is more important than the build with GCC 6 (where we have several months before a release to work out what warnings should be there at all). Please recalibrate your notion of obviousness so you are much more wary of committing patches that haven't been reviewed. See the list of obvious types of changes at <https://sourceware.org/glibc/wiki/Consensus>. Someone indicating a general direction without having seen a patch is *not* approval for some arbitrary future patch following that direction, unless they specifically say e.g. "OK with that change".
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.