Re: HELP: fixing nullable annotations for return values

2015-08-17 Thread Philip Withnall
Hey,

On Fri, 2015-08-14 at 17:13 +0100, Alberto Ruiz wrote:
 During GUADEC I decided to revive a small effort I took a while ago. 
 I wrote a script using the private GObject Introspection AST API to 
 check if %NULL was mentioned in the documentation string of any 
 return value in any way that indicated that such function/method was 
 likely to miss the (nullable) introspection annotation.

Sorry to have missed this at GUADEC. :-(

I have a couple of bug reports open about incorrect annotations
already:
 • https://bugzilla.gnome.org/show_bug.cgi?id=742903
 • https://bugzilla.gnome.org/show_bug.cgi?id=719966

#719966 is a big one and is worth looking at because it depends on 
https://bugzilla.gnome.org/show_bug.cgi?id=729660 which suggests fixing
things by changing some of the GIR default assumptions about what’s
nullable and what’s not (regarding closures).

 I came up with a list of 143 functions that I'm trying to fix on a 
 branch (wip/aruiz/nullable-annotations), the plan is to collect and 
 review all the fixes there and then rebase/sqash into master 
 eventually. I'm tracking the effort on bgo#753520 [0] and using this 
 Google Spreadsheet[1].
 
 Anyone interested in helping fixing any of the listed functions, just 
 add your IRC nickname in the owner list in the spreadsheet[1] and 
 push into that branch. I only ask of a few things before pushing:
 - Check if (transfer none/full) applies
 - Rename NULL or #NULL as %NULL if you have the chance.
 - Check if the function is actually nullable, my script may be 
 mislead by whatever is in the document.
 
 If we have any clang hacker, it'll be great if we could tool this 
 into the compiler to check at compile time if a given function can 
 return NULL at some point. To have something like this integrated in 
 Builder and gobject introspection would certainly prevent new APIs to 
 suffer from this problem.

https://people.collabora.com/~pwith/tartan/

If I ever get time to finish it. It can already do a lot of this —
that’s how I found all the problems in bug #719966. Help very much
welcome.

Philip

signature.asc
Description: This is a digitally signed message part
___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gtk-devel-list


Re: Null check in g_hash_table_lookup()

2015-08-17 Thread Simon McVittie
On 15/08/15 22:14, Jasper St. Pierre wrote:
 It will fix it in that it will warn and then return NULL (and only if
 you build glib with the flag that turns those return_val_if_fail's on,
 which it does by default), but it's considered improper behavior to
 call g_hash_table_lookup with a NULL key.

As far as I understand it, adding this return_val_if_fail would be an
API break. At the moment, NULL is accepted, but is only correct to store
or look up if the hash table's hash and equality functions also accept
it. Some GHashTable functions even document this, for instance

g_hash_table_lookup_extended:
...
You can actually pass NULL for lookup_key to test whether the NULL
key exists, provided the hash and equal functions of hash_table are
NULL-safe.

g_str_equal, g_str_hash do not accept NULL as an argument, but for
instance g_direct_equal, g_direct_hash do. If you write your own hash
and equality functions, it's up to you whether NULL is accepted or not.

Not accepting NULL would break the relatively common case of hash tables
that use GINT_TO_POINTER(some_integer) keys, where the runtime value of
some_integer happens to be 0 sometimes. [1]

g_hash_table_lookup[_extended] do not annotate key as (allow-none),
which seems like a bug to me.

S

[1] Strictly speaking this assumes a CPU architecture where NULL is
all-bits-zero, which is not actually guaranteed by ISO C; but in
practice, nothing in the GLib stack is going to work on architectures
where this assumption does not hold, because there's a pervasive
assumption that a struct with all-bits-zero has all its pointer members
set to NULL.

-- 
Simon McVittie
Collabora Ltd. http://www.collabora.com/

___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gtk-devel-list


Re: g_error_free() warning on null pointer

2015-08-17 Thread Simon McVittie
On 16/08/15 20:23, Michael McConville wrote:
 Emmanuele Bassi wrote:
 You expected the *_free() functions in GLib to be NULL-safe. They
 aren't, except for g_free().
 
 g_error_free is. Maybe others too, I haven't checked. It just prints
 annoying console warnings.

The intention throughout the GLib-based stack is that if a
g_return[_val]_if_fail() is hit, it indicates that the caller has called
the function incorrectly, in a way that is considered to be undefined
behaviour https://en.wikipedia.org/wiki/Undefined_behavior. Undefined
behaviour does not mean your code will crash; it means any result is
valid, and it is up to the compiler or library author what happens. It
does in practice end up meaning your code is wrong, because the
majority of the possible implementations of undefined behaviour are not
what you wanted.

As a convenience to developers and users, if checks are enabled in
GLib, it tries to cope with certain classes of undefined behaviour
somewhat gracefully, and emit a diagnostic (complain on stderr) rather
than just crashing. However, this is not part of GLib's API, and in
particular, if checks are not enabled, it will usually crash instead.

See https://bugzilla.gnome.org/show_bug.cgi?id=660809 for more on this.

-- 
Simon McVittie
Collabora Ltd. http://www.collabora.com/

___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gtk-devel-list