Closed Bug 818629 Opened 7 years ago Closed 7 years ago

widget/gtkxtbin/gtk2xtbin.c:140:3: warning: initialization from incompatible pointer type ... (near initialization for ‘xt_event_funcs.finalize’)

Categories

(Core :: Widget: Gtk, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla20

People

(Reporter: dholbert, Assigned: dholbert)

References

(Blocks 1 open bug)

Details

(Keywords: memory-leak)

Attachments

(1 file, 1 obsolete file)

Build warning:
{
widget/gtkxtbin/gtk2xtbin.c:140:3: warning: initialization from incompatible pointer type [enabled by default]

widget/gtkxtbin/gtk2xtbin.c:140:3: warning: (near initialization for ‘xt_event_funcs.finalize’) [enabled by default]
}

This is for this chunk of code:
> 136 static GSourceFuncs xt_event_funcs = {
> 137   xt_event_prepare,
> 138   xt_event_check,
> 139   xt_event_dispatch,
> 140   g_free,
> 141   (GSourceFunc)NULL,
> 142   (GSourceDummyMarshal)NULL
> 143 };
https://mxr.mozilla.org/mozilla-central/source/widget/gtkxtbin/gtk2xtbin.c#136

This code dates back to bug 121615 (its patch: attachment 97117 [details] [diff] [review])

A quick google search shows that GSourceFuncs is declared with its 4th function-ptr needing to have this signature:
>  void     (*finalize) (GSource    *source);
http://developer.gnome.org/glib/2.31/glib-The-Main-Event-Loop.html#GSourceFuncs

...whereas we're passing g_free, which has this signature:
> void g_free(gpointer mem);
http://developer.gnome.org/glib/2.31/glib-Memory-Allocation.html#g-free

(Note that gpointer is simply a typedef for void*.)


So: GSourceFuncs expects a finalize function that takes a GSource*, but we're giving it a function that takes a void*.  So we're passing the wrong type of function.

We can fix this by just adding a simple wrapper-function that accepts a GSource* and g_free()s it. (and then passing that function-pointer, in place of g_free, in the code quoted above)
Attached patch fix (obsolete) — Splinter Review
Attachment #688892 - Flags: review?(karlt)
(Side note: It looks like the 2 NULL lines (at lines 141 and 142 in the code quoted above) don't map to anything in the gtk 2.3.1 GSourceFuncs struct-definition that I linked in comment 0... but that doesn't seem to be causing any harm, and I'm guessing it might be there for backwards-compat with an older GTK version)
(In reply to Daniel Holbert [:dholbert] from comment #2)
> (Side note: It looks like the 2 NULL lines [...]

(Ah yes, those are included here:
http://developer.gnome.org/glib/2.28/glib-The-Main-Event-Loop.html#GSourceFuncs
...so, never mind about those)
Comment on attachment 688892 [details] [diff] [review]
fix

The compiler warning is actually picking up a bug here.

The intention of the finalize method is to allow a derived object to release its resources. e.g.
http://git.gnome.org/browse/glib/tree/glib/giounix.c?h=glib-2-34#n172

The memory allocated by g_source_new will be released when the last reference is removed.

The g_free here would currently result in a double free, but that is not happening because the GSource is never unref'ed.  That can be done immediately after attach, as at http://hg.mozilla.org/mozilla-central/annotate/277998cf11cd/widget/gtk2/nsAppShell.cpp#l114

Here there are no resources to release, and so finalize can be NULL.
(GDK doesn't bother to cast the NULL - don't know whether our compiler flags would produce a warning with plain NULL.)
Attachment #688892 - Flags: review?(karlt) → review-
Keywords: mlk
Attached patch fix v2Splinter Review
Ah, thanks for noticing that!

Changed the patch to pass NULL and call unref after attaching.
Attachment #688892 - Attachment is obsolete: true
Attachment #688968 - Flags: review?(karlt)
Comment on attachment 688968 [details] [diff] [review]
fix v2

Thanks for reporting and fixing!
Attachment #688968 - Flags: review?(karlt) → review+
https://hg.mozilla.org/mozilla-central/rev/a16704573ef7
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
Duplicate of this bug: 581481
You need to log in before you can comment on or make changes to this bug.