Am 03.11.2015 um 21:43 schrieb Takashi Iwai:

On Tue, 03 Nov 2015 21:08:00 +0100, Stefan Seyfried wrote:

...

Am 03.11.2015 um 19:23 schrieb Dmitriy Perlow:

...

https://build.opensuse.org/package/show/X11:xfce/xfdashboard fails to build for openSUSE:Factory target with such an output: [ 370s] ... running 01-check-debuginfo [ 370s] ... testing for empty debuginfo packages [ 370s] ... running 02-check-gcc-output [ 370s] ... testing for serious compiler warnings [ 370s] (using /usr/lib/build/checks-data/check_gcc_output) [ 370s] (using //.build.log) [ 370s] E: xfdashboard 64bit-portability-issue /usr/include/glib-2.0/glib/gmessages.h:382

the error is: [ 315s] quicklaunch.c: In function '_xfdashboard_quicklaunch_get_actor_for_appinfo': [ 315s] /usr/include/glib-2.0/glib/gmessages.h:382:10: warning: return makes pointer from integer without a cast [-Wint-conversion] [ 315s] return (val); \ [ 315s] ^ [ 315s] quicklaunch.c:161:2: note: in expansion of macro 'g_return_val_if_fail' [ 315s] g_return_val_if_fail(XFDASHBOARD_IS_QUICKLAUNCH(self), TRUE); [ 315s] ^ [ 315s] /usr/include/glib-2.0/glib/gmessages.h:382:10: warning: return makes pointer from integer without a cast [-Wint-conversion] [ 315s] return (val); \ [ 315s] ^ [ 315s] quicklaunch.c:162:2: note: in expansion of macro 'g_return_val_if_fail' [ 315s] g_return_val_if_fail(G_IS_APP_INFO(inAppInfo), TRUE); [ 315s] ^

This is the function:

...

151 /* Get actor for desktop application information */ 152 static ClutterActor* _xfdashboard_quicklaunch_get_actor_for_appinfo(XfdashboardQuicklaunch *self, 153 GAppInfo *inAppInfo) 154 { 155 XfdashboardQuicklaunchPrivate *priv; 156 ClutterActorIter iter; 157 ClutterActor *child; 158 GAppInfo *desktopAppInfo; 159 GFile *desktopFile; 160 161 g_return_val_if_fail(XFDASHBOARD_IS_QUICKLAUNCH(self), TRUE); 162 g_return_val_if_fail(G_IS_APP_INFO(inAppInfo), TRUE);

Now the "g_return_val_if_fail" macro will return TRUE if the condition is false. But the return value of the function is "ClutterActor*", so maybe returning NULL would be better, but I cannot tell for sure, I'm not familiar with the code.

Returning an integer or boolean value isn't necessarily wrong in many codes, and this can be such a case, I suppose (although I can't say exactly as I also haven't looked at the code).

But it should be cast to the proper pointer type at least, I guess, or the portability issue of the former warning, now error, might occur.

The more important question is why this is treated as a fatal error. I took a glance through the log, too, and indeed there is no other errors. Was the build check changed on FACTORY to be more strict?

I think so, I remember this as a warning from former times. -- Stefan Seyfried "For a successful technology, reality must take precedence over public relations, for nature cannot be fooled." -- Richard Feynman -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org

On Wed, 04 Nov 2015 08:46:04 +0100, Andreas Schwab wrote:

Takashi Iwai writes:

...

Returning an integer or boolean value isn't necessarily wrong in many codes, and this can be such a case, I suppose (although I can't say exactly as I also haven't looked at the code).

It is never ok to return a boolean in a function that returns a pointer.

It is OK if it's cast. The returned value itself is same, just a different expression.

...

The more important question is why this is treated as a fatal error.

Because it's a fatal error.

What's the definition of "fatal"? If it's really a fatal error, why does it build for other than FACTORY? :) Don't get me wrong: I'm not against this action, it's for a good sake. But then it'd be better to suggest more understandable texts for novice packagers what to do for it. Takashi -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org

On Wed, 04 Nov 2015 09:52:24 +0100, Dominique Leuenberger / DimStar wrote:

On Wed, 2015-11-04 at 09:01 +0100, Takashi Iwai wrote:

...

On Wed, 04 Nov 2015 08:46:04 +0100, Andreas Schwab wrote:

...

Takashi Iwai writes:

...

Returning an integer or boolean value isn't necessarily wrong in many codes, and this can be such a case, I suppose (although I can't say exactly as I also haven't looked at the code).

It is never ok to return a boolean in a function that returns a pointer.

It is OK if it's cast.  The returned value itself is same, just a different expression.

Returning a boolean FALSE (0) won't cause an issue - but TRUE? That translates to '1'.

So in fact, you return a pointer on the address 0x1 and hope to get anything useful from there when accessing it.

Sounds in most cases very fatal.

Well, SIG_IGN is defined like #define SIG_IGN ((__sighandler_t) 1) Is this a fatal error? Oh no, we have to submit a bug report! :) Returning a bool packed as a pointer is a bad programming practice, and as already mentioned, I myself find the check good. But my point is that it's not always the thing like "never OK, it's a fatal error". If the caller and callee know the expected value and are careful enough, passing such a value is also acceptable. Takashi -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org

On Wed, 04 Nov 2015 10:32:57 +0100, Andreas Schwab wrote:

Takashi Iwai writes:

...

Well, SIG_IGN is defined like

#define SIG_IGN ((__sighandler_t) 1)

Is this a fatal error?

SIG_IGN is not a boolean.

Neither TRUE nor FALSE are. Takashi -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org

On Wed, 04 Nov 2015 12:10:28 +0100, Andreas Schwab wrote:

Takashi Iwai writes:

...

On Wed, 04 Nov 2015 10:32:57 +0100, Andreas Schwab wrote:

...

Takashi Iwai writes:

...

Well, SIG_IGN is defined like

#define SIG_IGN ((__sighandler_t) 1)

Is this a fatal error?

SIG_IGN is not a boolean.

Neither TRUE nor FALSE are.

Yes, they are.

Not always. You can define your own values, too. Takashi -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org

On Wed, 2015-11-04 at 12:18 +0100, Andreas Schwab wrote:

SIG_IGN is not a boolean.

...

...

...

Neither TRUE nor FALSE are.

Yes, they are.

Not always.  You can define your own values, too.

They are part of the gnome API.

from glib headers: ./glib-2.0/glib/gmacros.h:#define FALSE (0) ./glib-2.0/glib/gmacros.h:#define TRUE (!FALSE) Cheers, Dominique -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org

Takashi Iwai writes:

glib defines them only when not defined beforehand.

It doesn't matter. It's plain, simple, utterly broken code. Andreas. -- Andreas Schwab, SUSE Labs, schwab@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different." -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org

Takashi Iwai writes:

On Wed, 04 Nov 2015 12:27:42 +0100, Andreas Schwab wrote:

...

Takashi Iwai writes:

...

glib defines them only when not defined beforehand.

It doesn't matter. It's plain, simple, utterly broken code.

It works as expected, so it's not broken in that regard :)

It doesn't work at all. Andreas. -- Andreas Schwab, SUSE Labs, schwab@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different." -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org

Takashi Iwai writes:

On Wed, 04 Nov 2015 13:00:55 +0100, Andreas Schwab wrote:

...

Takashi Iwai writes:

...

On Wed, 04 Nov 2015 12:27:42 +0100, Andreas Schwab wrote:

...

Takashi Iwai writes:

...

glib defines them only when not defined beforehand.

It doesn't matter. It's plain, simple, utterly broken code.

It works as expected, so it's not broken in that regard :)

It doesn't work at all.

It print 2 as expected.

It crashes. Andreas. -- Andreas Schwab, SUSE Labs, schwab@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different." -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org

Takashi Iwai writes:

It compiles and runs fine on FACTORY x86_64.

That doesn't prove anything. Andreas. -- Andreas Schwab, SUSE Labs, schwab@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different." -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org

Takashi Iwai writes:

On Wed, 04 Nov 2015 14:40:13 +0100, Andreas Schwab wrote:

...

Takashi Iwai writes:

...

It compiles and runs fine on FACTORY x86_64.

That doesn't prove anything.

It proves that TRUE can have a different definition, not always a boolean, depending on the usage.

That has nothing to do with the subject. Andreas. -- Andreas Schwab, SUSE Labs, schwab@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different." -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org

On 11/04/2015 09:52 AM, Dominique Leuenberger / DimStar wrote:

On Wed, 2015-11-04 at 09:01 +0100, Takashi Iwai wrote:

...

It is OK if it's cast. The returned value itself is same, just a different expression.

Returning a boolean FALSE (0) won't cause an issue - but TRUE? That translates to '1'.

So in fact, you return a pointer on the address 0x1 and hope to get anything useful from there when accessing it.

Sounds in most cases very fatal.

The check is about 64bit portability. If you return (int)1 to a pointer, then the 32 bit int value will be assigned to either the lower or higher part of the 64-bit pointer adress, maybe leaving alone the other part. I think this is what the check is complaining about. At least I've seen programs running wild for such a reason when porting software to 64bit several years ago. Have a nice day, Berny -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org

On Wed, 04 Nov 2015 11:52:09 +0100, Bernhard Voelker wrote:

On 11/04/2015 09:52 AM, Dominique Leuenberger / DimStar wrote:

...

On Wed, 2015-11-04 at 09:01 +0100, Takashi Iwai wrote:

...

It is OK if it's cast. The returned value itself is same, just a different expression.

Returning a boolean FALSE (0) won't cause an issue - but TRUE? That translates to '1'.

So in fact, you return a pointer on the address 0x1 and hope to get anything useful from there when accessing it.

Sounds in most cases very fatal.

The check is about 64bit portability. If you return (int)1 to a pointer, then the 32 bit int value will be assigned to either the lower or higher part of the 64-bit pointer adress, maybe leaving alone the other part.

It returns the pointer addressing 0x1, and it works no matter whether 32/64bit or LE/BE architectures we care on OBS, at least. You can find tons of codes passing TRUE or any other integer value as a pointer in GNOME. Of course, they have usually explicit cast for avoiding the warning message. Takashi -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org

On Wednesday 2015-11-04 09:01, Takashi Iwai wrote:

On Wed, 04 Nov 2015 08:46:04 +0100, Andreas Schwab wrote:

...

Takashi Iwai writes:

...

Returning an integer or boolean value isn't necessarily wrong in many codes, and this can be such a case, I suppose (although I can't say exactly as I also haven't looked at the code).

It is never ok to return a boolean in a function that returns a pointer.

It is OK if it's cast.

Gagging compiler warnings with cast does not mean the problem is suddenly gone. You cannot infer from absence of diagnostics that the program is bugfree at the source level. -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org