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 87821 - Assignment of handler ids returned by g_signal_new () and friends should be 64-bit clean
Assignment of handler ids returned by g_signal_new () and friends should be 6...
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: Other
2.0.x
Other All
: Normal normal
: 3.0
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks: 347771
 
 
Reported: 2002-07-10 11:48 UTC by Shivram U
Modified: 2010-10-22 17:05 UTC
See Also:
GNOME target: ---
GNOME version: 2.9/2.10


Attachments
Log containing all the occurances of the warnings. (5.15 KB, text/plain)
2002-07-10 11:51 UTC, Shivram U
  Details
The parts of the log I wasn't able to fix easily. (2.97 KB, text/plain)
2002-07-10 18:29 UTC, Owen Taylor
  Details
Patch (12.86 KB, patch)
2010-10-08 01:31 UTC, Alberto Garcia
accepted-commit_now Details | Review

Description Shivram U 2002-07-10 11:48:57 UTC
Handler ids returned by g_signal_new () and friends are being treated as 
32-bit quantities. Creating the bug as discussed in the gtk-devel-list. 
Please also see 
http://mail.gnome.org/archives/gtk-devel-list/2002-July/msg00041.html

Attaching the output of the lint source checker i used. The output contains 
 all the occurances of the warnings.
Comment 1 Shivram U 2002-07-10 11:50:20 UTC
Changing the version to 2.0.x. Sorry for the mistake.
Comment 2 Shivram U 2002-07-10 11:51:58 UTC
Created attachment 9770 [details]
Log containing all the occurances of the warnings.
Comment 3 Owen Taylor 2002-07-10 18:29:51 UTC
Created attachment 9782 [details]
The parts of the log I wasn't able to fix easily.
Comment 4 Owen Taylor 2002-07-10 18:33:50 UTC
I've fixed the part that is easy to fix; the remainder
would need to be fixed by leaving the old field in place
and storing the handler ID somewhere else, which is,
I think, too ugly to do for the small problem that a potential
signal handler wraparound imposes.

(Note that the only problems  here would also occur on 32-bit
platforms, so until most GTK+ usage is on 64-bit platforms,
I wouldn't consider this issue urgent.)

Wed Jul 10 14:27:14 2002  Owen Taylor  <otaylor@redhat.com>

        * modules/input/gtkimcontextxim.c gtk/gtkcolorsel.c
        gtk/gtkdialog.c gtk/gtktextbtree.c: Fix some cases
        where signal connection IDs where being assigned to
        guint rather than gulong. (part of #87281, Shivram U)
Comment 5 Javier Jardón (IRC: jjardon) 2010-07-19 16:14:30 UTC
Is this still an issue? It can be easily fixable now as we don't have public members now
Comment 6 Matthias Clasen 2010-08-05 00:46:24 UTC
Yes, still an issue, and yes, easily fixable now. Just needs somebody to write a patch
Comment 7 Alberto Garcia 2010-10-08 01:31:42 UTC
Created attachment 171934 [details] [review]
Patch

This patch fixes all cases of g_signal_connect() and friends being
assigned to something different from gulong.

It's trivial changes everywhere except for a couple of cases
-gtkbutton and gtkimagemenuitem- where the signal handler ID was being
stored inside an object using g_object_set_data().

You cannot cast a gulong into a 32-bit gpointer, but since that value
was only being used to detect whether the signal handler had already
been connected, I changed it for a simpler g_signal_handler_find(),
which also allows us to remove a couple of static strings that were
otherwise useless.

Then there's gailcontainer.c which was storing signal handler IDs for
GtkContainer::add and GtkContainer::remove "in case some objects need
to remove these handlers", but since no one appears to be using that,
I just removed them.

Please review and commit. If everything's ok I think we can close the
bug.
Comment 8 Christian Dywan 2010-10-08 13:36:52 UTC
Review of attachment 171934 [details] [review]:

Looks good to me. For what I want, the use cases in GtkButton and GtkImageMenuItem are bad hacks, but that's what it was before anyway.
Comment 9 Alberto Garcia 2010-10-11 15:37:59 UTC
(In reply to comment #8)
> Looks good to me. For what I want, the use cases in GtkButton and
> GtkImageMenuItem are bad hacks, but that's what it was before
> anyway.

If you have a better idea we can still change it :)
Comment 10 Alberto Garcia 2010-10-20 08:47:43 UTC
ping
Comment 11 Christian Dywan 2010-10-22 16:53:46 UTC
Comment on attachment 171934 [details] [review]
Patch

(In reply to comment #9)
> If you have a better idea we can still change it :)

I think leave it as-is and re-consider it separately since the patch is fairly large and we don't want to sneak in behavioural changes. I'd say let's do this.