Re: Use GCC's -Wlogical-op if possible
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
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/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
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/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
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
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
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
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
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
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
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
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
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
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
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/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
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.