After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 760215 - G_LIKELY/_UNLIKELY macros need more parentheses
G_LIKELY/_UNLIKELY macros need more parentheses
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: general
unspecified
Other All
: Normal minor
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2016-01-06 13:25 UTC by Stephan Bergmann
Modified: 2016-10-11 11:44 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
fix (602 bytes, patch)
2016-01-06 13:25 UTC, Stephan Bergmann
none Details | Review
fix (1.04 KB, patch)
2016-01-06 15:31 UTC, Stephan Bergmann
committed Details | Review

Description Stephan Bergmann 2016-01-06 13:25:20 UTC
Created attachment 318321 [details] [review]
fix

> $ cat test.c
> #include "glib.h"
> #define COMMA ,
> void f(void) { if (G_LIKELY(0 COMMA 1)); }

> $ gcc -O -fsyntax-only $(pkg-config --cflags glib-2.0) test.c
> test.c: In function ‘f’:
> test.c:3:38: error: macro "_G_BOOLEAN_EXPR" passed 2 arguments, but takes just 1
Comment 1 Colin Walters 2016-01-06 15:15:27 UTC
Review of attachment 318321 [details] [review]:

Please submit a `git format-patch` patch, see https://wiki.gnome.org/Newcomers/SubmittingPatches
Comment 2 Stephan Bergmann 2016-01-06 15:31:10 UTC
Created attachment 318340 [details] [review]
fix

ah, conflicting instructions between <https://wiki.gnome.org/Newcomers/SubmittingPatches> and <https://developer.gnome.org/glib/2.47/glib-resources.html#id-1.2.9.4>...
Comment 3 Colin Walters 2016-01-06 15:33:30 UTC
Review of attachment 318340 [details] [review]:

It's not worth it for this patch, but in the future I'd like to have descriptions of *real world* use cases too.  In this case I'm going to guess it's something OpenOffice is doing - that would be useful to know.
Comment 4 Stephan Bergmann 2016-01-06 15:42:05 UTC
(In reply to Colin Walters from comment #3)
> It's not worth it for this patch, but in the future I'd like to have
> descriptions of *real world* use cases too.  In this case I'm going to guess
> it's something OpenOffice is doing - that would be useful to know.

No, it's just something I noticed when somebody took that code and copied it to the LibreOffice code base, and I did a code review there, see <http://cgit.freedesktop.org/libreoffice/core/commit/?id=55a5ac48c2abb4c7a97a829d3e9e2db86fafc081> "Get parenthesisation right."  I'm not aware of any real-world code affected by this issue.
Comment 5 Colin Walters 2016-01-06 15:43:49 UTC
(In reply to Stephan Bergmann from comment #4)

> No, it's just something I noticed when somebody took that code and copied it
> to the LibreOffice code base, and I did a code review there, see
> <http://cgit.freedesktop.org/libreoffice/core/commit/
> ?id=55a5ac48c2abb4c7a97a829d3e9e2db86fafc081> "Get parenthesisation right." 
> I'm not aware of any real-world code affected by this issue.

Ah hah!  That's exactly the kind of useful information I like to see in commit messages.
Comment 6 Thomas Haller 2016-10-11 11:44:23 UTC
there is the comment

 * The _G_BOOLEAN_EXPR macro is intended to trigger a gcc warning when
 * putting assignments in g_return_if_fail ().

https://git.gnome.org/browse/glib/tree/glib/gmacros.h?id=e44ea516afeb41d22cebf968b3ea5d9543856df2#n363


I didn't actually test this, but I think with this change of putting parentheses around (expr), the gcc warning is no longer present. Which makes the comment wrong and the _G_BOOLEAN_EXPR() macro unnecessary.