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

Message ID n7qj1p$1em$1@ger.gmane.org
State Superseded
Headers

Commit Message

Stefan Liebler Jan. 21, 2016, 12:30 p.m. UTC
  Hi,

Building string/tst-endian.c with gcc 6 produces an build warning/error 
on s390:
gcc tst-endian.c -c -std=gnu11 -fgnu89-inline  -O2 or -O3 ...
tst-endian.c: In function ‘do_test’:
tst-endian.c:16:30: error: self-comparison always evaluates to false 
[-Werror=tautological-compare]
     if (htobe16 (be16toh (i)) != i)
                               ^~

tst-endian.c:48:30: error: self-comparison always evaluates to false 
[-Werror=tautological-compare]
     if (htobe32 (be32toh (i)) != i)
                               ^~

tst-endian.c:78:33: error: self-comparison always evaluates to false 
[-Werror=tautological-compare]
        if (htobe64 (be64toh (i)) != i)
                                  ^~

See definitions of htobexx, bexxtoh in string/endian.h:
# if __BYTE_ORDER == __LITTLE_ENDIAN
...
# else
#  define htobe16(x) (x)
#  define be16toh(x) (x)
#  define htobe32(x) (x)
#  define be32toh(x) (x)
#  define htobe64(x) (x)
#  define be64toh(x) (x)
# endif

This patch makes these if-statements conditional on __BYTE_ORDER == 
__LITTLE_ENDIAN.

Ok to commit?

Bye Stefan

ChangeLog:

	* string/tst-endian.c (do_test): Make htobexx( bexxtoh (i)) != i
	if-statements conditional on __BYTE_ORDER == __LITTLE_ENDIAN.
  

Comments

Andreas Schwab Jan. 21, 2016, 1:11 p.m. UTC | #1
Stefan Liebler <stli@linux.vnet.ibm.com> writes:

> diff --git a/string/tst-endian.c b/string/tst-endian.c
> index 8684bb2..d093587 100644
> --- a/string/tst-endian.c
> +++ b/string/tst-endian.c
> @@ -13,12 +13,14 @@ do_test (void)
>      {
>        if (i < UINT64_C (65536))
>  	{
> +#if __BYTE_ORDER == __LITTLE_ENDIAN
>  	  if (htobe16 (be16toh (i)) != i)
>  	    {
>  	      printf ("htobe16 (be16toh (%" PRIx64 ")) == %" PRIx16 "\n",
>  		      i, (uint16_t) htobe16 (be16toh (i)));
>  	      result = 1;
>  	    }
> +#endif

Does that also work when using if (__BYTE_ORDER == __LITTLE_ENDIAN)?

Andreas.
  
Stefan Liebler Jan. 21, 2016, 1:47 p.m. UTC | #2
On 01/21/2016 02:11 PM, Andreas Schwab wrote:
> Stefan Liebler <stli@linux.vnet.ibm.com> writes:
>
>> diff --git a/string/tst-endian.c b/string/tst-endian.c
>> index 8684bb2..d093587 100644
>> --- a/string/tst-endian.c
>> +++ b/string/tst-endian.c
>> @@ -13,12 +13,14 @@ do_test (void)
>>       {
>>         if (i < UINT64_C (65536))
>>   	{
>> +#if __BYTE_ORDER == __LITTLE_ENDIAN
>>   	  if (htobe16 (be16toh (i)) != i)
>>   	    {
>>   	      printf ("htobe16 (be16toh (%" PRIx64 ")) == %" PRIx16 "\n",
>>   		      i, (uint16_t) htobe16 (be16toh (i)));
>>   	      result = 1;
>>   	    }
>> +#endif
>
> Does that also work when using if (__BYTE_ORDER == __LITTLE_ENDIAN)?
>
> Andreas.
>
No it does not work:
tst-endian.c: In function ‘do_test’:
tst-endian.c:17:65: error: self-comparison always evaluates to false 
[-Werror=tautological-compare]
     if (__BYTE_ORDER == __LITTLE_ENDIAN && htobe16 (be16toh (i)) != i)
                                                                  ^~

cc1: all warnings being treated as errors
  
Joseph Myers Jan. 21, 2016, 2:05 p.m. UTC | #3
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.

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.
  

Patch

diff --git a/string/tst-endian.c b/string/tst-endian.c
index 8684bb2..d093587 100644
--- a/string/tst-endian.c
+++ b/string/tst-endian.c
@@ -13,12 +13,14 @@  do_test (void)
     {
       if (i < UINT64_C (65536))
 	{
+#if __BYTE_ORDER == __LITTLE_ENDIAN
 	  if (htobe16 (be16toh (i)) != i)
 	    {
 	      printf ("htobe16 (be16toh (%" PRIx64 ")) == %" PRIx16 "\n",
 		      i, (uint16_t) htobe16 (be16toh (i)));
 	      result = 1;
 	    }
+#endif
 	  if (htole16 (le16toh (i)) != i)
 	    {
 	      printf ("htole16 (le16toh (%" PRIx64 ")) == %" PRIx16 "\n",
@@ -45,12 +47,14 @@  do_test (void)
 
       if (i < UINT64_C (4294967296))
 	{
+#if __BYTE_ORDER == __LITTLE_ENDIAN
 	  if (htobe32 (be32toh (i)) != i)
 	    {
 	      printf ("htobe32 (be32toh (%" PRIx64 ")) == %" PRIx32 "\n",
 		      i, (uint32_t) htobe32 (be32toh (i)));
 	      result = 1;
 	    }
+#endif
 	  if (htole32 (le32toh (i)) != i)
 	    {
 	      printf ("htole32 (le32toh (%" PRIx64 ")) == %" PRIx32 "\n",
@@ -75,12 +79,14 @@  do_test (void)
 	    }
 	}
 
+#if __BYTE_ORDER == __LITTLE_ENDIAN
       if (htobe64 (be64toh (i)) != i)
 	{
 	  printf ("htobe64 (be64toh (%" PRIx64 ")) == %" PRIx64 "\n",
 		  i, htobe64 (be64toh (i)));
 	  result = 1;
 	}
+#endif
       if (htole64 (le64toh (i)) != i)
 	{
 	  printf ("htole64 (le64toh (%" PRIx64 ")) == %" PRIx64 "\n",