Re: Use GCC's -Wlogical-op if possible

2010-09-19 Thread Gerald Pfeifer
On Mon, 4 Jan 2010, Henri Verbeet wrote:
 I dont see a reason to take that warning serious, as its not a problem 
 in any case. in kernel32 it just  happens because we use a Macro. 
 Taking that case(v==1) out of the Macro leads to harder readable code. 
 So IMHO i would not make -Wlogical-op the default.
 Not using that macro at all would be even more readable. I'm probably
 missing something obvious, but it seems to just do a wsprintfW() with
 #%d as format.

I see this has been comitted via b45d4aa161fbbb905fa9142d2757ff3f7952566d.
Thanks!  Looks like this warning is useful after all. ;-)

In fact, doing a build with -Wlogical-op right now it turns out that with
the change above and the work last year there aren't any open issues on
that front any more and I'll submit a patch to make this used by default.

Gerald


Re: Use GCC's -Wlogical-op if possible

2010-01-04 Thread Paul Vriens

On 01/03/2010 10:12 PM, Gerald Pfeifer wrote:

On Sun, 3 Jan 2010, Austin English wrote:

On my FreeBSD test system I am see no warnings triggered by -Wlogical-op
any more.  How does it look on your side?

ole32:
usrmarshal.c:485: warning: logical ?? with non-zero constant will
always evaluate as true
usrmarshal.c:1098: warning: logical ?? with non-zero constant will
always evaluate as true
usrmarshal.c:1290: warning: logical ?? with non-zero constant will
always evaluate as true

oleaut32:
variant.c:2090: warning: logical ?||? with non-zero constant will
always evaluate as true
variant.c:2090: warning: logical ?? with non-zero constant will
always evaluate as true

comctl32/tests:
tab.c:520: warning: logical ?? with non-zero constant will always
evaluate as true
tab.c:540: warning: logical ?? with non-zero constant will always
evaluate as true
tab.c:563: warning: logical ?? with non-zero constant will always
evaluate as true
tab.c:978: warning: logical ?? with non-zero constant will always
evaluate as true


I had a patch for this one (comctl32/tests) which I received feedback on
and need to brush up.  I should be able to do so coming week.  Anybody
volunteering to look into the other ones?


kernel32/tests:
atom.c:70: warning: logical ?||? with non-zero constant will always
evaluate as true


Gerald


The attached is what I have on my F12 box (gcc 4.4.2 20091222):

--
Cheers,

Paul.
make[2]: Entering directory '/wine/wine-git/tools/winebuild'
parser.c:82: warning: logical '' with non-zero constant will always evaluate 
as true
parser.c:87: warning: logical '' with non-zero constant will always evaluate 
as true

make[2]: Entering directory '/wine/wine-git/dlls/msvcrt'
mbcs.c:109: warning: logical '' with non-zero constant will always evaluate 
as true
string.c:102: warning: logical '' with non-zero constant will always evaluate 
as true
string.c:105: warning: logical '' with non-zero constant will always evaluate 
as true
wcs.c:398: warning: logical '' with non-zero constant will always evaluate as 
true
wcs.c:406: warning: logical '' with non-zero constant will always evaluate as 
true
wcs.c:414: warning: logical '' with non-zero constant will always evaluate as 
true

make[2]: Entering directory '/wine/wine-git/dlls/ntdll'
path.c:886: warning: logical '' with non-zero constant will always evaluate 
as true
printf.c:247: warning: logical '' with non-zero constant will always evaluate 
as true
printf.c:255: warning: logical '' with non-zero constant will always evaluate 
as true
printf.c:263: warning: logical '' with non-zero constant will always evaluate 
as true
string.c:130: warning: logical '' with non-zero constant will always evaluate 
as true

make[2]: Entering directory '/wine/wine-git/dlls/ole32'
usrmarshal.c:485: warning: logical '' with non-zero constant will always 
evaluate as true
usrmarshal.c:1098: warning: logical '' with non-zero constant will always 
evaluate as true
usrmarshal.c:1290: warning: logical '' with non-zero constant will always 
evaluate as true

make[2]: Entering directory '/wine/wine-git/dlls/oleaut32'
variant.c:2090: warning: logical '||' with non-zero constant will always 
evaluate as true
variant.c:2090: warning: logical '' with non-zero constant will always 
evaluate as true

make[2]: Entering directory '/wine/wine-git/dlls/riched20'
editor.c:3256: warning: logical '' with non-zero constant will always 
evaluate as true
editor.c:3256: warning: logical '' with non-zero constant will always 
evaluate as true
editor.c:3256: warning: logical '' with non-zero constant will always 
evaluate as true
editor.c:3257: warning: logical '' with non-zero constant will always 
evaluate as true
editor.c:3257: warning: logical '' with non-zero constant will always 
evaluate as true
editor.c:3257: warning: logical '' with non-zero constant will always 
evaluate as true
editor.c:3535: warning: logical '' with non-zero constant will always 
evaluate as true
editor.c:3535: warning: logical '' with non-zero constant will always 
evaluate as true
editor.c:3535: warning: logical '' with non-zero constant will always 
evaluate as true
editor.c:3536: warning: logical '' with non-zero constant will always 
evaluate as true
editor.c:3536: warning: logical '' with non-zero constant will always 
evaluate as true
editor.c:3536: warning: logical '' with non-zero constant will always 
evaluate as true

make[2]: Entering directory '/wine/wine-git/dlls/shell32'
shv_bg_cmenu.c:418: warning: logical '' with non-zero constant will always 
evaluate as true
shv_bg_cmenu.c:418: warning: logical '' with non-zero constant will always 
evaluate as true
shv_bg_cmenu.c:418: warning: logical '' with non-zero constant will always 
evaluate as true
shv_bg_cmenu.c:419: warning: logical '' with non-zero constant will always 
evaluate as true
shv_bg_cmenu.c:419: warning: logical '' with non-zero constant will always 
evaluate as true
shv_bg_cmenu.c:419: warning: logical '' with non-zero constant will always 
evaluate 

Re: Use GCC's -Wlogical-op if possible

2010-01-04 Thread Nicolas Le Cam
2010/1/4 Paul Vriens paul.vriens.w...@gmail.com:
 On 01/03/2010 10:12 PM, Gerald Pfeifer wrote:

 On Sun, 3 Jan 2010, Austin English wrote:

 On my FreeBSD test system I am see no warnings triggered by -Wlogical-op
 any more.  How does it look on your side?

 ole32:
 usrmarshal.c:485: warning: logical ?? with non-zero constant will
 always evaluate as true
 usrmarshal.c:1098: warning: logical ?? with non-zero constant will
 always evaluate as true
 usrmarshal.c:1290: warning: logical ?? with non-zero constant will
 always evaluate as true

 oleaut32:
 variant.c:2090: warning: logical ?||? with non-zero constant will
 always evaluate as true
 variant.c:2090: warning: logical ?? with non-zero constant will
 always evaluate as true

 comctl32/tests:
 tab.c:520: warning: logical ?? with non-zero constant will always
 evaluate as true
 tab.c:540: warning: logical ?? with non-zero constant will always
 evaluate as true
 tab.c:563: warning: logical ?? with non-zero constant will always
 evaluate as true
 tab.c:978: warning: logical ?? with non-zero constant will always
 evaluate as true

 I had a patch for this one (comctl32/tests) which I received feedback on
 and need to brush up.  I should be able to do so coming week.  Anybody
 volunteering to look into the other ones?

 kernel32/tests:
 atom.c:70: warning: logical ?||? with non-zero constant will always
 evaluate as true

 Gerald

 The attached is what I have on my F12 box (gcc 4.4.2 20091222):

 --
 Cheers,

 Paul.




Most of them are false positives because of a problem between gcc
4.{3,4}.x and the strchr being defined as a macro in libc, see gcc bug
#36513 [1]. My karmic box seems to output the same.

[1] http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36513


-- 
Nicolas Le Cam




Re: Use GCC's -Wlogical-op if possible

2010-01-04 Thread André Hentschel
Gerald Pfeifer schrieb:
 I had a patch for this one (comctl32/tests) which I received feedback on 
 and need to brush up.  I should be able to do so coming week.  Anybody 
 volunteering to look into the other ones?
 
 kernel32/tests:
 atom.c:70: warning: logical ?||? with non-zero constant will always
 evaluate as true
 
 Gerald

I dont see a reason to take that warning serious, as its not a problem in any 
case. in kernel32 it just  happens because we use a Macro. Taking that 
case(v==1) out of the Macro leads to harder readable code.
So IMHO i would not make -Wlogical-op the default.

-- 

Best Regards, André Hentschel




Re: Use GCC's -Wlogical-op if possible

2010-01-04 Thread Henri Verbeet
2010/1/4 André Hentschel n...@dawncrow.de:
 I dont see a reason to take that warning serious, as its not a problem in any 
 case. in kernel32 it just  happens because we use a Macro. Taking that 
 case(v==1) out of the Macro leads to harder readable code.
 So IMHO i would not make -Wlogical-op the default.

Not using that macro at all would be even more readable. I'm probably
missing something obvious, but it seems to just do a wsprintfW() with
#%d as format.




Re: Use GCC's -Wlogical-op if possible

2010-01-04 Thread André Hentschel
Henri Verbeet schrieb:
 2010/1/4 André Hentschel n...@dawncrow.de:
 I dont see a reason to take that warning serious, as its not a problem in 
 any case. in kernel32 it just  happens because we use a Macro. Taking that 
 case(v==1) out of the Macro leads to harder readable code.
 So IMHO i would not make -Wlogical-op the default.

 Not using that macro at all would be even more readable. I'm probably
 missing something obvious, but it seems to just do a wsprintfW() with
 #%d as format.
i can confirm that you are right.

-- 

Best Regards, André Hentschel




Re: Use GCC's -Wlogical-op if possible

2010-01-03 Thread Gerald Pfeifer
On Tue, 23 Jun 2009, Paul Vriens wrote:
 For those interested, here's the make log in Monday's git. 94 warnings 
 for me.
 Sent 3 patches based on your log.
 
 I have less (55) warnings on 4.3.2 btw, but most look like false positives.

On my FreeBSD test system I am see no warnings triggered by -Wlogical-op
any more.  How does it look on your side?

Depending on your findings, we may want to enable -Wlogical-op by default
and I'd submit a patch to that end.

Gerald




Re: Use GCC's -Wlogical-op if possible

2010-01-03 Thread Austin English
On Sun, Jan 3, 2010 at 4:13 AM, Gerald Pfeifer ger...@pfeifer.com wrote:
 On Tue, 23 Jun 2009, Paul Vriens wrote:
 For those interested, here's the make log in Monday's git. 94 warnings
 for me.
 Sent 3 patches based on your log.

 I have less (55) warnings on 4.3.2 btw, but most look like false positives.

 On my FreeBSD test system I am see no warnings triggered by -Wlogical-op
 any more.  How does it look on your side?

ole32:
usrmarshal.c:485: warning: logical ‘’ with non-zero constant will
always evaluate as true
usrmarshal.c:1098: warning: logical ‘’ with non-zero constant will
always evaluate as true
usrmarshal.c:1290: warning: logical ‘’ with non-zero constant will
always evaluate as true

oleaut32:
variant.c:2090: warning: logical ‘||’ with non-zero constant will
always evaluate as true
variant.c:2090: warning: logical ‘’ with non-zero constant will
always evaluate as true

comctl32/tests:
tab.c:520: warning: logical ‘’ with non-zero constant will always
evaluate as true
tab.c:540: warning: logical ‘’ with non-zero constant will always
evaluate as true
tab.c:563: warning: logical ‘’ with non-zero constant will always
evaluate as true
tab.c:978: warning: logical ‘’ with non-zero constant will always
evaluate as true

kernel32/tests:
atom.c:70: warning: logical ‘||’ with non-zero constant will always
evaluate as true

-- 
-Austin




Re: Use GCC's -Wlogical-op if possible

2010-01-03 Thread Gerald Pfeifer
On Sun, 3 Jan 2010, Austin English wrote:
 On my FreeBSD test system I am see no warnings triggered by -Wlogical-op
 any more.  How does it look on your side?
 ole32:
 usrmarshal.c:485: warning: logical ?? with non-zero constant will
 always evaluate as true
 usrmarshal.c:1098: warning: logical ?? with non-zero constant will
 always evaluate as true
 usrmarshal.c:1290: warning: logical ?? with non-zero constant will
 always evaluate as true
 
 oleaut32:
 variant.c:2090: warning: logical ?||? with non-zero constant will
 always evaluate as true
 variant.c:2090: warning: logical ?? with non-zero constant will
 always evaluate as true
 
 comctl32/tests:
 tab.c:520: warning: logical ?? with non-zero constant will always
 evaluate as true
 tab.c:540: warning: logical ?? with non-zero constant will always
 evaluate as true
 tab.c:563: warning: logical ?? with non-zero constant will always
 evaluate as true
 tab.c:978: warning: logical ?? with non-zero constant will always
 evaluate as true

I had a patch for this one (comctl32/tests) which I received feedback on 
and need to brush up.  I should be able to do so coming week.  Anybody 
volunteering to look into the other ones?

 kernel32/tests:
 atom.c:70: warning: logical ?||? with non-zero constant will always
 evaluate as true

Gerald


Re: Use GCC's -Wlogical-op if possible

2009-06-23 Thread Paul Vriens

Austin English wrote:

On Fri, Jun 19, 2009 at 12:35 PM, Gerald Pfeiferger...@pfeifer.com wrote:

On Fri, 19 Jun 2009, Paul Vriens wrote:

I have about 70 warnings with 4.3.2.

I already sent two patches and found a few more bugs. If you want I can
start sending in patches for those as well (don't want to duplicate work
of course).

Thanks for checking to help avoid duplicate work, Paul; I really
appreciate that.  Austin was kind enough to share his logs, and I
now submitted patches for (more or less) everything I had seen on
my side and could reproduce based on this input.

If you want to go for the rest you are seeing after those few patches
of mine, that would be great.  If you could keep us updated on how it
goes (ideally bringing warnings down to zero, of course ;-) that would
be nice, too.

Happy hacking! :)

Gerald



For those interested, here's the make log in Monday's git. 94 warnings for me.


Sent 3 patches based on your log.

I have less (55) warnings on 4.3.2 btw, but most look like false positives.

--
Cheers,

Paul.




Re: Use GCC's -Wlogical-op if possible

2009-06-19 Thread Gerald Pfeifer
On Fri, 19 Jun 2009, Ben Klein wrote:
 diff --git a/configure.ac b/configure.ac
 index bef311e..3f7a657 100644
 --- a/configure.ac
 +++ b/configure.ac
 @@ -1385,8 +1385,9 @@ then
   WINE_TRY_CFLAGS([-fno-builtin],[AC_SUBST(BUILTINFLAG,-fno-builtin)])
   WINE_TRY_CFLAGS([-fno-strict-aliasing])
   WINE_TRY_CFLAGS([-Wdeclaration-after-statement])
 -  WINE_TRY_CFLAGS([-Wwrite-strings])
 +  WINE_TRY_CFLAGS([-Wlogical-op])
   WINE_TRY_CFLAGS([-Wtype-limits])
 +  WINE_TRY_CFLAGS([-Wwrite-strings])
 Is there a reason why -Wwrite-strings is moved?

Alphabetical sorting, now the list gets longer. :-)

Gerald


Re: Use GCC's -Wlogical-op if possible

2009-06-19 Thread Gerald Pfeifer
On Thu, 18 Jun 2009, Austin English wrote:
 Causes 106 more warnings on 4.3.3 of this sort:
 tab.c:693: warning: logical ?? with non-zero constant will always
 evaluate as true
 cert.c:1627: warning: logical ?||? with non-zero constant will always
 evaluate as true

This is strange.  In that case, let's hold off and I will work on 
addressing these first (testing with even more versions of GCC than
I had done).

Gerald





Re: Use GCC's -Wlogical-op if possible

2009-06-19 Thread Paul Vriens

Gerald Pfeifer wrote:

On Thu, 18 Jun 2009, Austin English wrote:

Causes 106 more warnings on 4.3.3 of this sort:
tab.c:693: warning: logical ?? with non-zero constant will always
evaluate as true
cert.c:1627: warning: logical ?||? with non-zero constant will always
evaluate as true


This is strange.  In that case, let's hold off and I will work on 
addressing these first (testing with even more versions of GCC than

I had done).

Gerald




I have about 70 warnings with 4.3.2.

I already sent two patches and found a few more bugs. If you want I can 
start sending in patches for those as well (don't want to duplicate work 
of course).


--
Cheers,

Paul.




Re: Use GCC's -Wlogical-op if possible

2009-06-19 Thread Gerald Pfeifer
On Fri, 19 Jun 2009, Rein Klazes wrote:
 cert.c:1627: warning: logical ?||? with non-zero constant will always
 evaluate as true
 That (in dlls/crypt32/tests) looks like a real bug to me.

Indeed.  And Alexandre committed a fix of mine in the last 24 hours. ;-)

Gerald




Re: Use GCC's -Wlogical-op if possible

2009-06-19 Thread Gerald Pfeifer
On Fri, 19 Jun 2009, Paul Vriens wrote:
 I have about 70 warnings with 4.3.2.
 
 I already sent two patches and found a few more bugs. If you want I can 
 start sending in patches for those as well (don't want to duplicate work 
 of course).

Thanks for checking to help avoid duplicate work, Paul; I really 
appreciate that.  Austin was kind enough to share his logs, and I
now submitted patches for (more or less) everything I had seen on
my side and could reproduce based on this input.

If you want to go for the rest you are seeing after those few patches
of mine, that would be great.  If you could keep us updated on how it
goes (ideally bringing warnings down to zero, of course ;-) that would
be nice, too.

Happy hacking! :)

Gerald




Re: Use GCC's -Wlogical-op if possible

2009-06-18 Thread Austin English
On Thu, Jun 18, 2009 at 12:32 PM, Gerald Pfeiferger...@pfeifer.com wrote:
 I verified this does not cause any extra warnings with GCC 4.4, whereas
 GCC 4.5 will become quite a bit more useful in that regard and thus help
 spot any issues.

 As with -Wtype-limits that I suggested last year, I pledge to keep close
 an eye on this and to address any issues proactively as part of my nightly
 test builds.

 Gerald

 ChangeLog:
 Use GCC's -Wlogical-op if possible.

 diff --git a/configure.ac b/configure.ac
 index bef311e..3f7a657 100644
 --- a/configure.ac
 +++ b/configure.ac
 @@ -1385,8 +1385,9 @@ then
   WINE_TRY_CFLAGS([-fno-builtin],[AC_SUBST(BUILTINFLAG,-fno-builtin)])
   WINE_TRY_CFLAGS([-fno-strict-aliasing])
   WINE_TRY_CFLAGS([-Wdeclaration-after-statement])
 -  WINE_TRY_CFLAGS([-Wwrite-strings])
 +  WINE_TRY_CFLAGS([-Wlogical-op])
   WINE_TRY_CFLAGS([-Wtype-limits])
 +  WINE_TRY_CFLAGS([-Wwrite-strings])

   dnl Check for noisy string.h
   saved_CFLAGS=$CFLAGS




Causes 106 more warnings on 4.3.3 of this sort:
tab.c:693: warning: logical ‘’ with non-zero constant will always
evaluate as true
cert.c:1627: warning: logical ‘||’ with non-zero constant will always
evaluate as true

-- 
-Austin




Re: Use GCC's -Wlogical-op if possible

2009-06-18 Thread Ben Klein
2009/6/19 Austin English austinengl...@gmail.com:
 On Thu, Jun 18, 2009 at 12:32 PM, Gerald Pfeiferger...@pfeifer.com wrote:
 I verified this does not cause any extra warnings with GCC 4.4, whereas
 GCC 4.5 will become quite a bit more useful in that regard and thus help
 spot any issues.

 As with -Wtype-limits that I suggested last year, I pledge to keep close
 an eye on this and to address any issues proactively as part of my nightly
 test builds.

 Gerald

 ChangeLog:
 Use GCC's -Wlogical-op if possible.

 diff --git a/configure.ac b/configure.ac
 index bef311e..3f7a657 100644
 --- a/configure.ac
 +++ b/configure.ac
 @@ -1385,8 +1385,9 @@ then
   WINE_TRY_CFLAGS([-fno-builtin],[AC_SUBST(BUILTINFLAG,-fno-builtin)])
   WINE_TRY_CFLAGS([-fno-strict-aliasing])
   WINE_TRY_CFLAGS([-Wdeclaration-after-statement])
 -  WINE_TRY_CFLAGS([-Wwrite-strings])
 +  WINE_TRY_CFLAGS([-Wlogical-op])
   WINE_TRY_CFLAGS([-Wtype-limits])
 +  WINE_TRY_CFLAGS([-Wwrite-strings])

Is there a reason why -Wwrite-strings is moved?




Re: Use GCC's -Wlogical-op if possible

2009-06-18 Thread Rein Klazes
On Thu, 18 Jun 2009 16:01:20 -0500, you wrote:


cert.c:1627: warning: logical ‘||’ with non-zero constant will always
evaluate as true

That (in dlls/crypt32/tests) looks like a real bug to me.

Rein.