Has anyone else had difficulty building button.c since I made this change?
-----Forwarded Message----- From: MenTaLguY <mental@...3...> To: bulia byak <archiver_1@...19...> Subject: Re: button.c Date: Sun, 07 Dec 2003 19:07:50 -0500
On Sun, 2003-12-07 at 02:32, bulia byak wrote:
Mental, your latest button.c only compiles if I change
g_signal_handlers_block_by_func (G_OBJECT (button), G_CALLBACK (sp_button_perform_action), NULL); gtk_toggle_button_set_active (GTK_TOGGLE_BUTTON (button), down); g_signal_handlers_unblock_by_func (G_OBJECT (button), G_CALLBACK (sp_button_perform_action), NULL);
by
g_signal_handlers_block_by_func (G_OBJECT (button), (void*) G_CALLBACK (sp_button_perform_action), NULL); gtk_toggle_button_set_active (GTK_TOGGLE_BUTTON (button), down); g_signal_handlers_unblock_by_func (G_OBJECT (button), (void*) G_CALLBACK (sp_button_perform_action), NULL);
Is that the right fix?
No.
Casting a function pointer to a normal pointer isn't safe or portable -- you can get away with it on x86, but on many architectures (like e.g. Itanium), a function pointer is really a much larger "function descriptor" datatype.
Actually, this should compile as-is (it did compile for me), so it's something funky with one of our build configurations (not sure whose yet).
I'm using gcc 3.3.2 with CFLAGS="-g -Wall", and building with glib 2.2.3.
What compiler, compiler options, and version of GLib are you using, and what error(s) do you get when you try to compile?
-mental
On Sun, 7 Dec 2003, MenTaLguY wrote:
Has anyone else had difficulty building button.c since I made this change?
I'm getting these build errors:
if ccache g++ -DHAVE_CONFIG_H -I. -I/home/bryce/src/Inkscape/inkscape/src/widgets -I../.. -I.. -I/home/bryce/src/Inkscape/inkscape/src -I/usr/include/gtk-2.0 -I/usr/lib/gtk-2.0/include -I/usr/include/atk-1.0 -I/usr/include/pango-1.0 -I/usr/X11R6/include -I/usr/include/freetype2 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/libart-2.0 -I/usr/include/libxml2 -DINKSCAPE_PIXMAPDIR=""/usr/local/share/inkscape"" -O0 -Wall -MT button.o -MD -MP -MF ".deps/button.Tpo" \ -c -o button.o `test -f '/home/bryce/src/Inkscape/inkscape/src/widgets/button.c' || echo '/home/bryce/src/Inkscape/inkscape/src/widgets/'`/home/bryce/src/Inkscape/inkscape/src/widgets/button.c; \ then mv ".deps/button.Tpo" ".deps/button.Po"; \ else rm -f ".deps/button.Tpo"; exit 1; \ fi /home/bryce/src/Inkscape/inkscape/src/widgets/button.c: In function `void sp_button_toggle_set_down(SPButton*, int)': /home/bryce/src/Inkscape/inkscape/src/widgets/button.c:189: invalid conversion from `void (*)()' to `void*' /home/bryce/src/Inkscape/inkscape/src/widgets/button.c:191: invalid conversion from `void (*)()' to `void*' /home/bryce/src/Inkscape/inkscape/src/widgets/button.c: In function `void sp_button_action_set_active(SPAction*, unsigned int, void*)': /home/bryce/src/Inkscape/inkscape/src/widgets/button.c:233: warning: comparison between signed and unsigned integer expressions make[3]: *** [button.o] Error 1
Bryce
-----Forwarded Message----- From: MenTaLguY <mental@...3...> To: bulia byak <archiver_1@...19...> Subject: Re: button.c Date: Sun, 07 Dec 2003 19:07:50 -0500
On Sun, 2003-12-07 at 02:32, bulia byak wrote:
Mental, your latest button.c only compiles if I change
g_signal_handlers_block_by_func (G_OBJECT (button), G_CALLBACK (sp_button_perform_action), NULL); gtk_toggle_button_set_active (GTK_TOGGLE_BUTTON (button), down); g_signal_handlers_unblock_by_func (G_OBJECT (button), G_CALLBACK (sp_button_perform_action), NULL);
by
g_signal_handlers_block_by_func (G_OBJECT (button), (void*) G_CALLBACK (sp_button_perform_action), NULL); gtk_toggle_button_set_active (GTK_TOGGLE_BUTTON (button), down); g_signal_handlers_unblock_by_func (G_OBJECT (button), (void*) G_CALLBACK (sp_button_perform_action), NULL);
Is that the right fix?
No.
Casting a function pointer to a normal pointer isn't safe or portable -- you can get away with it on x86, but on many architectures (like e.g. Itanium), a function pointer is really a much larger "function descriptor" datatype.
Actually, this should compile as-is (it did compile for me), so it's something funky with one of our build configurations (not sure whose yet).
I'm using gcc 3.3.2 with CFLAGS="-g -Wall", and building with glib 2.2.3.
What compiler, compiler options, and version of GLib are you using, and what error(s) do you get when you try to compile?
-mental
On Sun, 2003-12-07 at 19:34, Bryce Harrington wrote:
/home/bryce/src/Inkscape/inkscape/src/widgets/button.c: In function `void sp_button_toggle_set_down(SPButton*, int)': /home/bryce/src/Inkscape/inkscape/src/widgets/button.c:189: invalid conversion from `void (*)()' to `void*' /home/bryce/src/Inkscape/inkscape/src/widgets/button.c:191: invalid conversion from `void (*)()' to `void*'
Hmm, that's just not right.
Looking into this, it appears that GClosure actually uses a gpointer for the callback function.
GClosure * g_cclosure_new (GCallback callback_func, gpointer user_data, GClosureNotify destroy_data) { GClosure *closure;
g_return_val_if_fail (callback_func != NULL, NULL);
closure = g_closure_new_simple (sizeof (GCClosure), user_data); if (destroy_data) g_closure_add_finalize_notifier (closure, user_data, destroy_data); ((GCClosure *) closure)->callback = (gpointer) callback_func;
return closure; }
Apparently nobody ever ran lint on the GLib source.
One wonders what contortions they have to go through to get this to work on IA-64.
I guess for now we must cast to gpointer, but I guess the next question is how to deal with this upstream...
-mental
On Sun, 2003-12-07 at 22:27, MenTaLguY wrote:
Looking into this, it appears that GClosure actually uses a gpointer for the callback function.
Update: I've filed a bug against GLib/GObject.
http://bugzilla.gnome.org/show_bug.cgi?id=128782
Indeed, it appears that even with the explicit cast, it would be valid for a compiler to generate an error in this case.
From the ANSI C Rationale document:
* Even with an explicit cast, it is invalid to convert a function pointer to an object pointer or a pointer to void, or vice-versa.
-mental
On Sun, 2003-12-07 at 19:12, MenTaLguY wrote:
Has anyone else had difficulty building button.c since I made this change?
I get the same errors as Bryce when I use a C++ compiler, but they go away when I use a C compiler. I'm using g++/gcc 3.3.2 and glib 2.2.3 on Debian Unstable.
The GObject documentation notes that the "func" argument which is "The C closure callback of the handlers" of g_signal_handlers_[un]block_by_func(instance, func, data) is "useless for non-C closures". Does that mean the way "func" is handled changes with a different type of compiler?
Mike
On Sun, 2003-12-07 at 21:54, Michael D. Seymour wrote:
The GObject documentation notes that the "func" argument which is "The C closure callback of the handlers" of g_signal_handlers_[un]block_by_func(instance, func, data) is "useless for non-C closures". Does that mean the way "func" is handled changes with a different type of compiler?
No, there are actually two types of closures, GClosure, which is a generic closure that might be implemented in any number of languages, and GCClosure, which is designed specifically to be a binding for a C (or C++) callback function.
In any case, problem is that the GCClosure code is actually violating the ANSI C specification and making non-portable assumptions. See my other message.
-mental
participants (3)
-
Bryce Harrington -
MenTaLguY -
Michael D. Seymour