Re: Compile warning on OS X

2013-12-15 Thread Willy Tarreau 
Hi Cyril,

On Sun, Dec 15, 2013 at 10:39:56PM +0100, Cyril Bonté wrote:
> Hi Willy,
> 
> Le 13/12/2013 10:45, Willy Tarreau a écrit :
> >Hi Igor,
> >
> >On Fri, Dec 13, 2013 at 05:13:51PM +0800, Igor wrote:
> >>Hi, Willy, the patch fixed the reported warning,
> >
> >Thanks for testing! I'm merging it then.
> >
> >>but seems introduce new warning, the log: http://pastebin.com/dBfHGV2S
> >
> >No it's not the same. Gcc is getting *really* annoying. It reports stupid
> >warnings all the time and forces you to write your code a certain way to
> >shut them down. It's really unbelievable. Have you seen the comment in the
> >code? It already says that this ugly construct was made *only* to shut gcc
> >down. But it seems that your new version is even smarter and now requires
> >the semi-colon to be put on a distinct line. It does not make any sense
> >any more, this compiler decides on the *form* of your code, not the 
> >semantics.
> >One day we'll see "Warning, you used parenthesis in sizeof which is an 
> >operator
> >and not a function" or "your 'if' statement was indented with spaces 
> >instead of
> >tabs, which might confuse readers with tab size different from 8".
> >
> >I think that gcc is more and more developped by monkeys for monkeys.
> >
> >Each version is slower and buggier than the previous one, and more annoying
> >on legacy code.
> >
> >I don't want to put the -Wno-x flags in the default build options 
> >because
> >some older compilers available on certain platforms don't support them all.
> >At the moment haproxy builds back to gcc 2.95. We can decide to support 3.x
> >and above in the future but that does not mean we'll gain more options to
> >silence it.
> >
> >That said, if someone is willing to enumerate the list of -Wno-xxx options
> >that are supported from 2.95 to 4.8 and which allow us to live in peace
> >with this boring compiler, I'm totally open to add them.
> 
> This is not necessarily a gcc issue. From what I see, the older code
> 
>   write(1, trash.str, trash.len);
> 
> doesn't produce any warning here (tested on Debian Wheezy/gcc 4.7.2/libc 
> 2.13 and clang 3.0, Ubuntu 13.10/gcc 4.8.1/libc 2.17 and current Ubuntu 
> development branch/gcc 4.8.2/libc d.17).
> 
> From the glibc include files, write() is declared as is :
> extern ssize_t write (int __fd, __const void *__buf, size_t __n) __wur;
> 
> __wur is what can produce the warning...or not.
> It sets the function attribute __attribute_warn_unused_result__ *only* 
> if _FORTIFY_SOURCE is greater than 0 and (Optimization level is greater 
> than 0 too).
> 
> _FORTIFY_SOURCE=2 looks to become the default for distribution 
> packagers. Maybe we could decide to enforce _FORTIFY_SOURCE to 0 in the 
> Makefile.

To be exact, I know that it's the includes which indicate what syscall
need to be checked, but what's really scary is that gcc suggests to put
the semi-colon on a different line ! I mean, in C, spaces have zero
meaning, and now we start seeing warnings related to the position of
spaces only. *that* is becoming scary.

Anyway I have fixed the issue based on an idea from Emeric consisting
in draining all these unused return values into a global variable which
holds a nice name...

Cheers,
Willy

Re: Compile warning on OS X

2013-12-15 Thread Cyril Bonté 

Hi Willy,

Le 13/12/2013 10:45, Willy Tarreau a écrit :

Hi Igor,

On Fri, Dec 13, 2013 at 05:13:51PM +0800, Igor wrote:

Hi, Willy, the patch fixed the reported warning,


Thanks for testing! I'm merging it then.


but seems introduce new warning, the log: http://pastebin.com/dBfHGV2S


No it's not the same. Gcc is getting *really* annoying. It reports stupid
warnings all the time and forces you to write your code a certain way to
shut them down. It's really unbelievable. Have you seen the comment in the
code? It already says that this ugly construct was made *only* to shut gcc
down. But it seems that your new version is even smarter and now requires
the semi-colon to be put on a distinct line. It does not make any sense
any more, this compiler decides on the *form* of your code, not the semantics.
One day we'll see "Warning, you used parenthesis in sizeof which is an operator
and not a function" or "your 'if' statement was indented with spaces instead of
tabs, which might confuse readers with tab size different from 8".

I think that gcc is more and more developped by monkeys for monkeys.

Each version is slower and buggier than the previous one, and more annoying
on legacy code.

I don't want to put the -Wno-x flags in the default build options because
some older compilers available on certain platforms don't support them all.
At the moment haproxy builds back to gcc 2.95. We can decide to support 3.x
and above in the future but that does not mean we'll gain more options to
silence it.

That said, if someone is willing to enumerate the list of -Wno-xxx options
that are supported from 2.95 to 4.8 and which allow us to live in peace
with this boring compiler, I'm totally open to add them.


This is not necessarily a gcc issue. From what I see, the older code

  write(1, trash.str, trash.len);

doesn't produce any warning here (tested on Debian Wheezy/gcc 4.7.2/libc 
2.13 and clang 3.0, Ubuntu 13.10/gcc 4.8.1/libc 2.17 and current Ubuntu 
development branch/gcc 4.8.2/libc d.17).


From the glibc include files, write() is declared as is :
extern ssize_t write (int __fd, __const void *__buf, size_t __n) __wur;

__wur is what can produce the warning...or not.
It sets the function attribute __attribute_warn_unused_result__ *only* 
if _FORTIFY_SOURCE is greater than 0 and (Optimization level is greater 
than 0 too).


_FORTIFY_SOURCE=2 looks to become the default for distribution 
packagers. Maybe we could decide to enforce _FORTIFY_SOURCE to 0 in the 
Makefile.


--
Cyril Bonté

Re: Compile warning on OS X

2013-12-13 Thread Igor 
I see, thanks for the very clear explanation. :)

Bests,
-Igor


On Fri, Dec 13, 2013 at 5:45 PM, Willy Tarreau  wrote:
> Hi Igor,
>
> On Fri, Dec 13, 2013 at 05:13:51PM +0800, Igor wrote:
>> Hi, Willy, the patch fixed the reported warning,
>
> Thanks for testing! I'm merging it then.
>
>> but seems introduce new warning, the log: http://pastebin.com/dBfHGV2S
>
> No it's not the same. Gcc is getting *really* annoying. It reports stupid
> warnings all the time and forces you to write your code a certain way to
> shut them down. It's really unbelievable. Have you seen the comment in the
> code? It already says that this ugly construct was made *only* to shut gcc
> down. But it seems that your new version is even smarter and now requires
> the semi-colon to be put on a distinct line. It does not make any sense
> any more, this compiler decides on the *form* of your code, not the semantics.
> One day we'll see "Warning, you used parenthesis in sizeof which is an 
> operator
> and not a function" or "your 'if' statement was indented with spaces instead 
> of
> tabs, which might confuse readers with tab size different from 8".
>
> I think that gcc is more and more developped by monkeys for monkeys.
>
> Each version is slower and buggier than the previous one, and more annoying
> on legacy code.
>
> I don't want to put the -Wno-x flags in the default build options because
> some older compilers available on certain platforms don't support them all.
> At the moment haproxy builds back to gcc 2.95. We can decide to support 3.x
> and above in the future but that does not mean we'll gain more options to
> silence it.
>
> That said, if someone is willing to enumerate the list of -Wno-xxx options
> that are supported from 2.95 to 4.8 and which allow us to live in peace
> with this boring compiler, I'm totally open to add them.
>
> Thanks,
> Willy
>

Re: Compile warning on OS X

2013-12-13 Thread Willy Tarreau 
Hi Igor,

On Fri, Dec 13, 2013 at 05:13:51PM +0800, Igor wrote:
> Hi, Willy, the patch fixed the reported warning,

Thanks for testing! I'm merging it then.

> but seems introduce new warning, the log: http://pastebin.com/dBfHGV2S

No it's not the same. Gcc is getting *really* annoying. It reports stupid
warnings all the time and forces you to write your code a certain way to
shut them down. It's really unbelievable. Have you seen the comment in the
code? It already says that this ugly construct was made *only* to shut gcc
down. But it seems that your new version is even smarter and now requires
the semi-colon to be put on a distinct line. It does not make any sense
any more, this compiler decides on the *form* of your code, not the semantics.
One day we'll see "Warning, you used parenthesis in sizeof which is an operator
and not a function" or "your 'if' statement was indented with spaces instead of
tabs, which might confuse readers with tab size different from 8".

I think that gcc is more and more developped by monkeys for monkeys.

Each version is slower and buggier than the previous one, and more annoying
on legacy code.

I don't want to put the -Wno-x flags in the default build options because
some older compilers available on certain platforms don't support them all.
At the moment haproxy builds back to gcc 2.95. We can decide to support 3.x
and above in the future but that does not mean we'll gain more options to
silence it.

That said, if someone is willing to enumerate the list of -Wno-xxx options
that are supported from 2.95 to 4.8 and which allow us to live in peace
with this boring compiler, I'm totally open to add them.

Thanks,
Willy

Re: Compile warning on OS X

2013-12-13 Thread Igor 
Hi, Willy, the patch fixed the reported warning, but seems introduce
new warning, the log: http://pastebin.com/dBfHGV2S

Thanks.



Bests,
-Igor


On Fri, Dec 13, 2013 at 4:25 PM, Willy Tarreau  wrote:
> On Tue, Dec 10, 2013 at 12:13:09AM +0100, Lukas Tribus wrote:
>> Hi Igor,
>>
>>
>> > include/common/time.h:111:29: warning: implicit conversion from
>> > 'unsigned long' to '__darwin_suseconds_t' (aka 'int') changes value
>> > from
>> > 18446744073709551615 to -1 [-Wconstant-conversion]
>> > tv->tv_sec = tv->tv_usec = TV_ETERNITY;
>> > ~ ^~~
>> > include/common/time.h:32:26: note: expanded from macro 'TV_ETERNITY'
>> >
>> > Can I ignore this warning even the compile succeed? Thanks for any 
>> > suggestion.
>>
>>
>> Not sure, could you git bisect this?
>
> Last change here dates from 2007. Compilers tend to report more and more
> warnings in recent versions, and systems use a wide variety of types for
> time.
>
> Igor, could you please confirm that the attached patch fixes the issue ?
> If so, I'll merge it.
>
> Thanks,
> Willy
>

Re: Compile warning on OS X

2013-12-13 Thread Willy Tarreau 
On Tue, Dec 10, 2013 at 12:13:09AM +0100, Lukas Tribus wrote:
> Hi Igor,
> 
> 
> > include/common/time.h:111:29: warning: implicit conversion from
> > 'unsigned long' to '__darwin_suseconds_t' (aka 'int') changes value
> > from
> > 18446744073709551615 to -1 [-Wconstant-conversion]
> > tv->tv_sec = tv->tv_usec = TV_ETERNITY;
> > ~ ^~~
> > include/common/time.h:32:26: note: expanded from macro 'TV_ETERNITY'
> >
> > Can I ignore this warning even the compile succeed? Thanks for any 
> > suggestion.
> 
> 
> Not sure, could you git bisect this?

Last change here dates from 2007. Compilers tend to report more and more
warnings in recent versions, and systems use a wide variety of types for
time.

Igor, could you please confirm that the attached patch fixes the issue ?
If so, I'll merge it.

Thanks,
Willy

>From 5f3f15f6186d27343b9d346714ea2ad1aa9a8e46 Mon Sep 17 00:00:00 2001
From: Willy Tarreau 
Date: Fri, 13 Dec 2013 09:22:23 +0100
Subject: BUILD: time: adapt the type of TV_ETERNITY to the local system

Some systems use different types for tv_sec/tv_usec, some are
signed others not. From time to time new warnings are reported
about implicit casts being done.

This patch ensures that TV_ETERNITY is cast to the appropriate
type in assignments and conversions.
---
 include/common/time.h | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/include/common/time.h b/include/common/time.h
index 5d86ad5..8be4718 100644
--- a/include/common/time.h
+++ b/include/common/time.h
@@ -108,7 +108,8 @@ REGPRM2 void tv_update_date(int max_wait, int interrupted);
  */
 REGPRM1 static inline struct timeval *tv_eternity(struct timeval *tv)
 {
-   tv->tv_sec = tv->tv_usec = TV_ETERNITY;
+   tv->tv_sec  = (typeof(tv->tv_sec))TV_ETERNITY;
+   tv->tv_usec = (typeof(tv->tv_usec))TV_ETERNITY;
return tv;
 }
 
@@ -124,12 +125,12 @@ REGPRM1 static inline struct timeval *tv_zero(struct 
timeval *tv) {
 /*
  * returns non null if tv is [eternity], otherwise 0.
  */
-#define tv_iseternity(tv)   ((tv)->tv_usec == TV_ETERNITY)
+#define tv_iseternity(tv)   ((tv)->tv_usec == 
(typeof((tv)->tv_usec))TV_ETERNITY)
 
 /*
  * returns 0 if tv is [eternity], otherwise non-zero.
  */
-#define tv_isset(tv)   ((tv)->tv_usec != TV_ETERNITY)
+#define tv_isset(tv)   ((tv)->tv_usec != 
(typeof((tv)->tv_usec))TV_ETERNITY)
 
 /*
  * returns non null if tv is [0], otherwise 0.
-- 
1.7.12.2.21.g234cd45.dirty

RE: Compile warning on OS X

2013-12-09 Thread Lukas Tribus 
Hi Igor,


> include/common/time.h:111:29: warning: implicit conversion from
> 'unsigned long' to '__darwin_suseconds_t' (aka 'int') changes value
> from
> 18446744073709551615 to -1 [-Wconstant-conversion]
> tv->tv_sec = tv->tv_usec = TV_ETERNITY;
> ~ ^~~
> include/common/time.h:32:26: note: expanded from macro 'TV_ETERNITY'
>
> Can I ignore this warning even the compile succeed? Thanks for any suggestion.


Not sure, could you git bisect this?


Lukas

Compile warning on OS X

2013-12-09 Thread Igor 
include/common/time.h:111:29: warning: implicit conversion from
'unsigned long' to '__darwin_suseconds_t' (aka 'int') changes value
from
  18446744073709551615 to -1 [-Wconstant-conversion]
tv->tv_sec = tv->tv_usec = TV_ETERNITY;
 ~ ^~~
include/common/time.h:32:26: note: expanded from macro 'TV_ETERNITY'

Can I ignore this warning even the compile succeed? Thanks for any suggestion.

Bests,
-Igor