GNOME Bugzilla – Bug 691499
g_get_prgname() has wrong introspection mark-up
Last modified: 2015-02-07 17:01:32 UTC
When gobject-introspection parses glib's headers, it decides that g_get_prgname() should be marked as transfer full, so, ie, pygobject frees the value returned from it when it should not. I think that this could be fixed through introspection mark-up, but it seems to me that the correct thing to do would be to prototype the function as returning a const value, since it returns a string that is owned by glib. This would have the side-effect of making g-ir-scanner assume that the return value is not owned by the caller.
Created attachment 233183 [details] [review] Patch.
Review of attachment 233183 [details] [review]: This is clearly correct, but does it break the C++ bindings? danw?
(IANA C++ expert, this is just something I have a mental note to be paranoid about. Anyway...) As I understand it, changes made to C headers won't *directly* break C++, because they're all inside G_BEGIN_DECLS aka extern "C". But if glibmm translates the C signatures into C++ wrappers in a way that the change would be exposed in C++ as well, then it does break things. In this case, it looks like all of those gutils.h functions return "std::string" in glibmm, regardless of whether they have "const" or not, so it seems like it *should* be ok... Ignoring C++ though, this will cause compile failures for C code that does "char *foo = g_get_prgname ();" and uses certain -Werror options...
(In reply to comment #3) > In this case, it looks like all of those gutils.h functions return > "std::string" in glibmm, regardless of whether they have "const" or not, so it > seems like it *should* be ok... Ah, ok. > Ignoring C++ though, this will cause compile failures for C code that does > "char *foo = g_get_prgname ();" and uses certain -Werror options... Yeah...that is a concern, but IIRC we also have other precedents for changes like this?
So far, I found: gdk-pixbuf: queryloaders.c: In function ‘main’: queryloaders.c:333:17: warning: assignment discards ‘const’ qualifier from pointer target type [enabled by default] at-spi2-atk: registry.c:380:14: warning: assignment discards ‘const’ qualifier from pointer target type [enabled by default] gtk: gtktoplevelaccessible.c: In function 'gtk_toplevel_accessible_initialize': gtktoplevelaccessible.c:47:20: warning: assignment discards 'const' qualifier from pointer target type [enabled by default] vala: scanner.c:235:25: warning: assignment discards ‘const’ qualifier from pointer target type [enabled by default]
I think we could try to add the const in the next devel release and see how big the fallout is.
Comment on attachment 233183 [details] [review] Patch. This was committed by Benjamin: http://git.gnome.org/browse/glib/commit/?id=120834db5b1cc735530d2452440ffd8a3b6e48f7 I guess at this point let's just go with it.
Sorry I wasn't aware of the bug, I just had IRC discussions with Mike and Ryan about it. And we decided to go with it in there.
gdk-pixbuf: http://git.gnome.org/browse/gdk-pixbuf/commit/?id=516d5e0eee263bb311df1e081f4912e313788982 I couldn't find a call to g_get_prgname() in at-spi2-atk (or at-spi2-core). gtk3 looks OK from a quick code inspection. Will clone this bug for vala.
Jürg, can you look at updating vala for this?
(In reply to comment #10) > Jürg, can you look at updating vala for this? scanner.c in vala (from old g-i) doesn't use g_get_prgname, the warning mentioned above is not related to this bug. Vala applications are not affected either, as the manual GLib bindings have correct ownership information.
[Mass-moving gobject-introspection tickets to its own Bugzilla product - see bug 708029. Mass-filter your bugmail for this message: introspection20150207 ]