Because xfce4-dev-tools-4.7.2 adds CPPFLAGS="-DG_DISABLE_CAST_CHECKS -DG_DISABLE_ASSERT -DG_DISABLE_CHECKS" it will break current release of xfdesktop, 4.6.1, so that all the icons will overlap on the up-left corner. Obviously it's bad idea to add CPPFLAGS that Xfce4 doesn't support yet to the macros... Patching these CPPFLAGS out, or downgrading back to 4.7.0 solves the issue. Screenshot: http://bugs.gentoo.org/attachment.cgi?id=205080 Downstream bug: http://bugs.gentoo.org/show_bug.cgi?id=286166
Configuring with --disable-debug or --enable-debug=no is not recommended.
g_return_if_fail(xfdesktop_icon_get_position(icon, &row, &col)); breaks with --disable-debug, then row & col are set to 0 seems like wrong usage of asserts in xfdesktop. that said, why is --disable-debug not recommended? that's the default we pass for all xfce pkgs in gentoo and so far this is the first package with problems. we've been doing this since 4.2.
Created attachment 2551 handle asserts correctly
Patch by Alexis Ballier <aballier at gentoo.org>
--disable-debug never set G_DISABLE_CAST_CHECKS or G_DISABLE_CHECKS before (except for exo and thunar, because Benny set them up with extra crap in configure.ac). Personally I disagree with both of these macros, but Benny thought they made sense for the "super optimized" case where you really don't want extraneous checks, and I'm ok with that. My 'philosophy' is that *everyone* should build with (at the very least) --enable-debug=minimum (which is the default if you don't specify --{enable,disable}-debug at all), and people who care the least about debugging should use --enable-debug=yes. If you're *actively* debugging something, or developing something, you should use --enable-debug=full. You're correct that xfdesktop misuses the g_return_*() macros in some places, and they should probably be fixed. I think I was just lazy, since I like how g_return_*() prints out a message if the assertion fails. lines like that should really be something like if(!xfdesktop_icon_get_position(icon, &row, &col)) { g_critical("some useful message"); return; } The attached patch is incorrect -- it doesn't semantically change the code at all and won't fix these problems; it will fail in exactly the same way as the original code does with --disable-debug.
(In reply to comment #5) > You're correct that xfdesktop misuses the g_return_*() macros in some places, > and they should probably be fixed. I think I was just lazy, since I like how > g_return_*() prints out a message if the assertion fails. lines like that > should really be something like > > if(!xfdesktop_icon_get_position(icon, &row, &col)) { > g_critical("some useful message"); > return; > } Indeed; it's just a matter of style and up to you then. > The attached patch is incorrect -- it doesn't semantically change the code at > all and won't fix these problems; it will fail in exactly the same way as the > original code does with --disable-debug. But this is wrong, if you have a look at gmessages.h (glib 2.20.5 if that matters), you'll see: #ifdef G_DISABLE_CHECKS #define g_return_if_fail(expr) G_STMT_START{ (void)0; }G_STMT_END This means that the side effects of 'expr' (setting row & col here) won't happen when G_DISABLE_CHECKS is defined. These macros should be treated the same way as asserts: never rely on their side effects.
Ah right, yeah. But it's still incorrect. You want the return to happen if the call fails; wrapping the return value in g_return_if_fail() doesn't achieve that. So the patch is a half-fix.
Ok, I tried to clean xfdesktop up in general to remove abuses of those macros. The remainder should indicate actual bugs if code falls through them.