GNOME Bugzilla – Bug 592715
Document that g_str_hash() and g_int_hash() are not NULL safe
Last modified: 2011-10-04 13:58:54 UTC
0) I'm spending some time to understand bug #592618. Progress is, as usual, slow. 1) A first (trivial) step in solving this bug would be to document that g_str_hash() and g_int_hash() are not NULL safe. Draft patch should follow shortly.
Created attachment 141426 [details] [review] Trivial doc patch
In the meantime I've noticed some other *_hash functions (g_int64_hash() and g_double_hash()) and some *_equal functions (g_int_equal(), g_int64_equal(), g_double_equal()) that could benefit from a line (similar to the line used in the draft patch) about their NULL safety. g_str_equal() has a line implicitly warning that it is not NULL safe. Maybe an explicit warning could be added there too.
Review of attachment 141426 [details] [review]: Can you write a patch that covers all the affected equal and hash functions ?
(In reply to comment #3) > Can you write a patch that covers all the affected equal and hash functions ? This issue was opened when I was still active on evolution. I don't think I have contributed to evolution since the beginning of 2010 (ie, in the last 18 months) and have actually contributed very little to the rest of gnome in that same period. I'm afraid it's unlikely that I'll restart working on this, at least in the near future.
Created attachment 198118 [details] [review] [1/4] GHashTable: be more clear what g_int_hash wants Using g_int_hash, g_int_equal with keys like GINT_TO_POINTER (n) seems to be a reasonably common GLib-novice mistake. It doesn't help that the documentation for GHashFunc was ambiguous about this.
Created attachment 198119 [details] [review] =2/4] Be completely clear about what g_direct_hash, g_direct_equal do Also annotate them as (allow-none), more for the benefit of gtk-doc readers than introspection.
Created attachment 198120 [details] [review] [3/4] Be clear that g_str_hash etc. don't accept NULL This covers the str, double, int, int64 hash and equal functions, but not anything that takes an "object", since the convention is that "object methods" never accept NULL anyway.
Created attachment 198121 [details] [review] [optional 4/4] Be really, pedantically, clear about NULL hash keys I don't think we need to go quite this far, but maybe we should...
Review of attachment 198118 [details] [review]: Looks good, thanks!
Review of attachment 198119 [details] [review]: Looks good.
Review of attachment 198120 [details] [review]: Looks good.
Review of attachment 198121 [details] [review]: People rarely complain about the length or correctness of documentation =) If someone you know has hit this, fine by me to add.
(In reply to comment #12) > People rarely complain about the length or correctness of documentation =) If > someone you know has hit this, fine by me to add. I think just saying "non-NULL" is enough, so I didn't push these. I've pushed the other three to master for 2.31.0. Could you close the bug, or give me suitable permission bits to do so myself, please?