GNOME Bugzilla – Bug 666700
Add some missing (allow-none) annotations
Last modified: 2012-01-11 20:50:04 UTC
Patch coming to add a few missing (allow-none) annotations on GContentType, GIcon and GHashTable methods.
Created attachment 204068 [details] [review] Add some missing (allow-none) annotations
Review of attachment 204068 [details] [review]: ::: gio/gcontenttype.c @@ +1654,3 @@ * * Returns: (transfer full) (array zero-terminated=1): an %NULL-terminated + * array of zero or more content types. Free with g_strfreev() Here you're REMOVING a %NULL from the docs, and you haven't added an (allow-none). ::: gio/gicon.c @@ +96,3 @@ * g_icon_equal: + * @icon1: (allow-none): pointer to the first #GIcon. + * @icon2: (allow-none): pointer to the second #GIcon. Looks fine. ::: glib/ghash.c @@ +1045,3 @@ * g_hash_table_lookup_extended(). * + * Return value: (allow-none): the associated value, or %NULL if the key is not found So (allow-none) for input values makes sense, but the scanner IIRC will ignore (allow-none) on return values; see: https://bugzilla.gnome.org/show_bug.cgi?id=660879 What do you want this for? Are you coding in Vala? @@ +1068,3 @@ * @lookup_key: the key to look up + * @orig_key: (allow-none): return location for the original key, or %NULL + * @value: (allow-none): return location for the value associated with the key, or %NULL Looks fine. @@ +1485,3 @@ * values in a hash table ends up needing O(n*n) operations). * + * Return value: (allow-none): The value of the first key/value pair is returned, See above.
(In reply to comment #2) > Review of attachment 204068 [details] [review]: > > ::: gio/gcontenttype.c > @@ +1654,3 @@ > * > * Returns: (transfer full) (array zero-terminated=1): an %NULL-terminated > + * array of zero or more content types. Free with g_strfreev() > > Here you're REMOVING a %NULL from the docs, and you haven't added an > (allow-none). Yes. I probably should've mentioned that in the patch description. I couldn't see any way for g_content_type_guess_for_tree() to return NULL on its own; just an empty vector. The documentation was misleading. Basically, the aim of the patch was more to make the documentation/annotations and the code match up re. NULL parameters/returns (which mostly involves adding (allow-none)s, but in this case didn't). > ::: glib/ghash.c > @@ +1045,3 @@ > * g_hash_table_lookup_extended(). > * > + * Return value: (allow-none): the associated value, or %NULL if the key is > not found > > So (allow-none) for input values makes sense, but the scanner IIRC will ignore > (allow-none) on return values; see: > https://bugzilla.gnome.org/show_bug.cgi?id=660879 > > What do you want this for? Are you coding in Vala? Yes, I'm coding in Vala. I was playing around with Vala's --enable-experimental-non-null nullability checking in libfolks, and I found a few places where missing annotations were making valac's analysis incorrect. I've filed a bug report against Vala (bug #666699) about fixing the .vapi files, but I thought it would be worthwhile pushing the annotations upstream as well. It should be harmless to add the (allow-none) annotations to return types at the moment, right? They can then be ready for if/when GIR gets support for them.
Review of attachment 204068 [details] [review]: Ok, makes sense.
Comment on attachment 204068 [details] [review] Add some missing (allow-none) annotations commit e98f17e5cf1696d418444b23cb092be0eaba3008 Author: Philip Withnall <philip@tecnocode.co.uk> Date: Thu Dec 22 00:16:06 2011 +0000 Bug 666700 — Add some missing (allow-none) annotations Add some missing (allow-none) annotations to GContentType, GIcon and GHashTable methods. Closes: bgo#666700 gio/gcontenttype.c | 2 +- gio/gicon.c | 4 ++-- glib/ghash.c | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-)