GNOME Bugzilla – Bug 703861
Add missing annotations for string functions
Last modified: 2015-02-07 17:02:59 UTC
Many string functions on glib only returns the passed string, which means they shouldn't be freed. But these functions don't have "transfer none" annotations, preventing their use in JavaScript and other programming languages relying on gobject-introspection. The attached patches will add those missing annotations. The first three are for glib while the last one is for gobject-introspection.
Created attachment 248711 [details] [review] gstring: Add missing annotations Add "transfer none" annotation to return value of functions that only return the passed object.
Created attachment 248713 [details] [review] gstrfuncs: Add missing annotations Add "transfer none" annotation to return value of functions that only return a passed pointer or a pointer to a position within the string. Add array annotations to g_strsplit, g_strdupv and g_strjoinv.
Created attachment 248714 [details] [review] gutf8: Add missing annotations Add "transfer none" annotation to return value of functions that only return a passed pointer or a pointer to a position within the string. Add "allow-none" annotations.
Created attachment 248715 [details] [review] Update glib annotations for gstrfuncs, gstring and gutf8
Comment on attachment 248711 [details] [review] gstring: Add missing annotations This is awkward, bindings-wise, since it means the binding will make a new copy of the string to represent the return value, but the modifications will have happened to the "original" string too. I think the general expectation is that GString is a utility for C programmers, and bindings aren't expected to expose it directly. What binding are you using?
Comment on attachment 248713 [details] [review] gstrfuncs: Add missing annotations >+ * Returns: (transfer none): @string this (g_strstrip) is a macro, so it's not really bindable anyway >+ * Returns: (transfer none): a pointer to trailing nul byte. > **/ > gchar * > g_stpcpy (gchar *dest, This should probably be (skip), since its semantics can't usefully be expressed in introspection notation. (From most GC'ed bindings, this would simply have no visible effect, and return an empty string.) >+ * Returns: (transfer none): The pointer to the buffer with the >+ * converted string. > **/ > gchar * > g_ascii_dtostr (gchar *buffer, Functions that take user-provided buffers are also generally not usefully-bindable, though in this case there could be a version that returned an allocated string instead. > g_ascii_formatd (gchar *buffer, likewise >+ * Returns: (transfer none): the @string > * > * Deprecated:2.2: This function is totally broken for the reasons discussed > * in the g_strncasecmp() docs - use g_ascii_strdown() or g_utf8_strdown() (g_strdown). This has been deprecated forever and should be marked (skip). Likewise g_strup. >+ * Returns: (transfer none): the same pointer passed in as @string > */ > gchar* > g_strreverse (gchar *string) This one is probably fine, though in most bindings it won't have the C semantics of mutating the original string. > g_strdelimit (gchar *string, > g_strcanon (gchar *string, > g_strchug (gchar *string) > g_strchomp (gchar *string) likewise >+ * Returns: (transfer full) (array zero-terminated=1) (element-type utf8): >+ * a newly-allocated %NULL-terminated array of strings. Use > * g_strfreev() to free it. (g_strsplit). This one is definitely right. Likewise g_strsplit_set. (Well... actually, it doesn't care if the strings are utf8 or filename, but...) > * g_strdupv: The annotations here are correct, but it's not clear they're really useful; languages generally ought to be able to duplicate arrays on their own... > * g_strjoinv: This is correct. > g_strstr_len (const gchar *haystack, > g_strrstr (const gchar *haystack, > g_strrstr_len (const gchar *haystack, not really useful from bindings
Comment on attachment 248714 [details] [review] gutf8: Add missing annotations similar comments as the previous patch...
Thank you for reviewing my patches :D (In reply to comment #5) > (From update of attachment 248711 [details] [review]) > This is awkward, bindings-wise, since it means the binding will make a new copy > of the string to represent the return value, but the modifications will have > happened to the "original" string too. Yes, I can't even use g_utf8_pointer_to_offset because the returned values are always copies of the original in GJS. > I think the general expectation is that GString is a utility for C programmers, > and bindings aren't expected to expose it directly. What binding are you using? I'm using GJS/JavaScript. Some of the functions with annotations changed are used in https://bugzilla.gnome.org/show_bug.cgi?id=703475 and I used GString to be able to read and change Unicode strings. The code is a port from C of media art storage functions of Tracker to JavaScript, used in GNOME Music. I can't use JavaScript functions like replace() and toLowerCase() because it segfaults when there are non-ASCII characters in the string (like Japanese characters, as for the strings I tested). So, I'm forced to use GLib functions there, especially GString. As for the string functions that just returns the passed pointer, GJS keeps freeing the returned values, even if I don't use the returned value, unless I add "transfer none" annotations to those. All string functions returning the same passed pointer are currently unusuable in GJS/JavaScript. For example, this won't work: GLib.strchug(str); Even if I didn't use the returned value. (In reply to comment #6) > This one is probably fine, though in most bindings it won't have the C > semantics of mutating the original string. Indeed, I have to use the returned values. I used g_strdelimit, g_strchug and g_strchomp, and they worked in GJS. The GNOME Music test suite has tests for them in "tests/tests_albumArt.js" (spaces test, symbols test, etc.). Maybe I could change the documentation to tell the users of bindings to use the returned values? As for the other annotations that are not useful, macros, deprecated and uses buffers, I'll remove them in a future patch.
I checked again and seems like the problems with the use of GJS's string functions are caused by a wrong code. I checked the available functions, and saw that they are nearly enough, and seems like these patches won't improve the situation much either. So I'm closing this bug now.
[Mass-moving gobject-introspection tickets to its own Bugzilla product - see bug 708029. Mass-filter your bugmail for this message: introspection20150207 ]