diff mbox

[PATCHv2] S390: Fix build failure in test string/tst-endian.c with gcc 6.

Message ID n7r1iu$tdi$1@ger.gmane.org
State Superseded
Headers

Commit Message

Stefan Liebler Jan. 21, 2016, 4:38 p.m. UTC 
  On 01/21/2016 03:05 PM, Joseph Myers wrote:
> On Thu, 21 Jan 2016, Stefan Liebler wrote:
>
>> This patch makes these if-statements conditional on __BYTE_ORDER ==
>> __LITTLE_ENDIAN.
>
> I don't think that's the right fix.
>
> This test is verifying assertions that should be true for both
> endiannesses.  It just so happens that some of those assertions can be
> verified at compile time in some cases, but they are still appropriate
> assertions for it to verify.
Agreed

> This is one of the cases where the nature of what is being tested requires
> doing something that GCC warns about.  In such a case, rather than
> disabling the testing of those assertions, you should disable the warning
> (with DIAG_* macros if possible - if glibc supports building with GCC
> versions without the relevant -W option, they'll need to be appropriately
> conditional), with appropriate comments explaining the issue.
ok

This patch suppresses these warnings with DIAG_* macros.
The conditional #if __GNUC_PREREQ (6, 0) is needed, because an older gcc 
would warn about:
tst-endian.c: In function ‘do_test’:
tst-endian.c:18:4: warning: unknown option after ‘#pragma GCC 
diagnostic’ kind [-Wpragmas]
     DIAG_IGNORE_NEEDS_COMMENT (6, "-Wtautological-compare");
     ^

I've extended the patch in order to suppress those warnings on little
endian systems for "if (htoleXX (leXXtoh (i)) != i)", too.
Those macros are defined to (x) in string/endian.h for little endian 
systems and produce the same warning.

Ok to commit?

Bye Stefan

ChangeLog:

	* string/tst-endian.c: Include <libc-internal.h>
	(do_test): Ignore tautological-compare warnings around
	"htobeXX (beXXtoh (i)) != i" and
	"htoleXX (leXXtoh (i)) != i" if-statements.

Comments

Mike Frysinger Jan. 21, 2016, 4:44 p.m. UTC | #1 
On 21 Jan 2016 17:38, Stefan Liebler wrote:
> This patch suppresses these warnings with DIAG_* macros.
> The conditional #if __GNUC_PREREQ (6, 0) is needed, because an older gcc 
> would warn about:
> tst-endian.c: In function ‘do_test’:
> tst-endian.c:18:4: warning: unknown option after ‘#pragma GCC 
> diagnostic’ kind [-Wpragmas]
>      DIAG_IGNORE_NEEDS_COMMENT (6, "-Wtautological-compare");
>      ^

you can at least do it once at the top via a local define:
/* big comment block */
#if __GNUC_PREREQ (6, 0)
# define DIAG_IGNORE_NEEDS_COMMENT_AUTOLOGICAL_COMPARE() \
  DIAG_IGNORE_NEEDS_COMMENT (6, "-Wtautological-compare");
#else
# define DIAG_IGNORE_NEEDS_COMMENT_AUTOLOGICAL_COMPARE()
#endif

then the inline usage is simple
-mike
diff mbox

Patch

diff --git a/string/tst-endian.c b/string/tst-endian.c
index 8684bb2..358232c 100644
--- a/string/tst-endian.c
+++ b/string/tst-endian.c
@@ -3,6 +3,7 @@ 
 #include <inttypes.h>
 #include <stdio.h>
 #include <stdint.h>
+#include <libc-internal.h>
 
 static int
 do_test (void)
@@ -13,6 +14,17 @@  do_test (void)
     {
       if (i < UINT64_C (65536))
 	{
+	  DIAG_PUSH_NEEDS_COMMENT;
+#if __GNUC_PREREQ (6, 0)
+	  /* GCC 6.0 warns on big endian systems about:
+	     htobe16 (be16toh (i)) != i
+	     warning: self-comparison always evaluates to false
+	     [-Wtautological-compare]
+	     because htobe16(x) and be16toh(x) is defined to (x)
+	     in string/endian.h on big endian systems.
+	     The same applies to htole16/le16toh on little endian systems.  */
+	  DIAG_IGNORE_NEEDS_COMMENT (6, "-Wtautological-compare");
+#endif
 	  if (htobe16 (be16toh (i)) != i)
 	    {
 	      printf ("htobe16 (be16toh (%" PRIx64 ")) == %" PRIx16 "\n",
@@ -25,6 +37,7 @@  do_test (void)
 		      i, (uint16_t) htole16 (le16toh (i)));
 	      result = 1;
 	    }
+	  DIAG_POP_NEEDS_COMMENT;
 
 	  uint16_t n[2];
 	  n[__BYTE_ORDER == __LITTLE_ENDIAN] = bswap_16 (i);
@@ -45,6 +58,17 @@  do_test (void)
 
       if (i < UINT64_C (4294967296))
 	{
+	  DIAG_PUSH_NEEDS_COMMENT;
+#if __GNUC_PREREQ (6, 0)
+	  /* GCC 6.0 warns on big endian systems about:
+	     htobe32 (be32toh (i)) != i
+	     warning: self-comparison always evaluates to false
+	     [-Wtautological-compare]
+	     because htobe32(x) and be32toh(x) is defined to (x)
+	     in string/endian.h on big endian systems.
+	     The same applies to htole32/le32toh on little endian systems.  */
+	  DIAG_IGNORE_NEEDS_COMMENT (6, "-Wtautological-compare");
+#endif
 	  if (htobe32 (be32toh (i)) != i)
 	    {
 	      printf ("htobe32 (be32toh (%" PRIx64 ")) == %" PRIx32 "\n",
@@ -57,6 +81,7 @@  do_test (void)
 		      i, (uint32_t) htole32 (le32toh (i)));
 	      result = 1;
 	    }
+	  DIAG_POP_NEEDS_COMMENT;
 
 	  uint32_t n[2];
 	  n[__BYTE_ORDER == __LITTLE_ENDIAN] = bswap_32 (i);
@@ -75,6 +100,17 @@  do_test (void)
 	    }
 	}
 
+      DIAG_PUSH_NEEDS_COMMENT;
+#if __GNUC_PREREQ (6, 0)
+      /* GCC 6.0 warns on big endian systems about:
+	 htobe64 (be64toh (i)) != i
+	 warning: self-comparison always evaluates to false
+	 [-Wtautological-compare]
+	 because htobe64(x) and be64toh(x) is defined to (x)
+	 in string/endian.h on big endian systems.
+	 The same applies to htole64/le64toh on little endian systems.  */
+      DIAG_IGNORE_NEEDS_COMMENT (6, "-Wtautological-compare");
+#endif
       if (htobe64 (be64toh (i)) != i)
 	{
 	  printf ("htobe64 (be64toh (%" PRIx64 ")) == %" PRIx64 "\n",
@@ -87,6 +123,7 @@  do_test (void)
 		  i, htole64 (le64toh (i)));
 	  result = 1;
 	}
+      DIAG_POP_NEEDS_COMMENT;
 
       uint64_t n[2];
       n[__BYTE_ORDER == __LITTLE_ENDIAN] = bswap_64 (i);