pragma change breaks build

Message ID 553EBBA8.6050902@cs.ucla.edu
State Committed
Headers

Commit Message

Paul Eggert April 27, 2015, 10:43 p.m. UTC
  David Miller wrote:
> instead of just adding a "= 0" initializer to vallen so
> that all gcc's will not warn

That could cause glibc to have a useless machine instruction, no?  Horrors!

Does the attached patch work for you?
  

Comments

Mike Frysinger April 28, 2015, 1:53 a.m. UTC | #1
On 27 Apr 2015 15:43, Paul Eggert wrote:
> David Miller wrote:
> > instead of just adding a "= 0" initializer to vallen so
> > that all gcc's will not warn
> 
> That could cause glibc to have a useless machine instruction, no?  Horrors!
> 
> Does the attached patch work for you?

why can't __GNUC_PREREQ be used as Joseph suggested ?
-mike
  
David Miller April 28, 2015, 2:45 a.m. UTC | #2
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Mon, 27 Apr 2015 15:43:52 -0700

> David Miller wrote:
>> instead of just adding a "= 0" initializer to vallen so
>> that all gcc's will not warn
> 
> That could cause glibc to have a useless machine instruction, no?
> Horrors!

Good you thought about that since setenv() is a performance critical
library routine...

> Does the attached patch work for you?

I'll give this a test later, thanks.
  
Paul Eggert April 28, 2015, 5:26 a.m. UTC | #3
Mike Frysinger wrote:
> why can't __GNUC_PREREQ be used as Joseph suggested ?

The #pragma should precede all code and macro definitions so that it applies 
uniformly across the entire compilation unit.  (We tried using the pragma more 
selectively, but that didn't work.)  So the pragma should appear before 
__GNUC_PREREQ is defined, just as it should appear before the other macro defn 
mentioned in the comment.

Possibly we could get away with including <errno.h>, then using __GNUC_PREREQ 
around the #pragma (this is because <errno.h> is fairly limited), but then we'd 
have to add another two or three lines to that comment and it's bad enough as it is.

I should mention that I also don't like all this futzing-around with pragmas, 
and would prefer the IF_LINT approach used in Gnulib and in GNU utilities (see 
<https://sourceware.org/ml/libc-alpha/2014-11/msg00611.html>).  However, that 
got shot down (see <https://sourceware.org/ml/libc-alpha/2014-11/msg00635.html>) 
by an argument similar to my satirical "Horrors!" comment earlier in this thread.
  
Andreas Schwab April 28, 2015, 7:51 a.m. UTC | #4
Paul Eggert <eggert@cs.ucla.edu> writes:

> David Miller wrote:
>> instead of just adding a "= 0" initializer to vallen so
>> that all gcc's will not warn
>
> That could cause glibc to have a useless machine instruction, no?  Horrors!

Moreover, by disabling the warning we won't be notified of new
violations.  Win-Win!

Andreas.
  

Patch

From 3e14ca767b43554c34f3f95c64ac793bdbd733fa Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Mon, 27 Apr 2015 15:30:43 -0700
Subject: [PATCH] * stdlib/setenv.c: Pacify only GCC 4.7 and later.

Problem with -Wmaybe-uninitialized reported by David Miller in:
https://sourceware.org/ml/libc-alpha/2015-04/msg00345.html
---
 ChangeLog       |  6 ++++++
 stdlib/setenv.c | 14 ++++++++------
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 39f74cc..706b5e3 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@ 
+2015-04-27  Paul Eggert  <eggert@cs.ucla.edu>
+
+	* stdlib/setenv.c: Pacify only GCC 4.7 and later.
+	Problem with -Wmaybe-uninitialized reported by David Miller in:
+	https://sourceware.org/ml/libc-alpha/2015-04/msg00345.html
+
 2015-04-27  H.J. Lu  <hongjiu.lu@intel.com>
 
 	[BZ#18333]
diff --git a/stdlib/setenv.c b/stdlib/setenv.c
index 184a8cd..ee45458 100644
--- a/stdlib/setenv.c
+++ b/stdlib/setenv.c
@@ -19,12 +19,14 @@ 
 # include <config.h>
 #endif
 
-/* Pacify GCC; see the commentary about VALLEN below.  This is needed
-   at least through GCC 4.9.2.  Pacify GCC for the entire file, as
-   there seems to be no way to pacify GCC selectively, only for the
-   place where it's needed.  Do not use DIAG_IGNORE_NEEDS_COMMENT
-   here, as it's not defined yet.  */
-#pragma GCC diagnostic ignored "-Wmaybe-uninitialized"
+/* Pacify GCC 4.7 and later; see the commentary about VALLEN below.
+   This is needed at least through GCC 5.1.0.  Pacify GCC for the
+   entire file, as there seems to be no way to pacify GCC selectively,
+   only for the place where it's needed.  Do not use macros like
+   DIAG_IGNORE_NEEDS_COMMENT here, as they're not defined yet.  */
+#if 4 < __GNUC__ + (7 <= __GNUC_MINOR__)
+# pragma GCC diagnostic ignored "-Wmaybe-uninitialized"
+#endif
 
 #include <errno.h>
 #if !_LIBC
-- 
2.1.0