GNOME Bugzilla – Bug 580873
Documentation of register type functions incomplete
Last modified: 2012-02-27 03:21:24 UTC
Documentation Section: gobject gtype g_type_register_fundamental Does in most cases not mention that the function can return 0 in case of failure. Correct version: Should mention error cases Other information: I hope that I can add the patch later.
Created attachment 133651 [details] [review] Proposed patch
I think the intention is that if you hit any of the code paths that cause a warning and G_TYPE_INVALID, you're in "programmer error, undefined behaviour" territory (see Bug #660809); the solution isn't to cope with a 0 return, it's to fix your program so 0 can't be returned. Where there are non-obvious preconditions, the preconditions themselves should be documented (to assist in writing non-broken programs), rather than documenting what will happen if your program is broken.
Hi again (after two years...) I think we agree that the documentation is incomplete as * Returns: The predefined type identifier. is only half of the truth. On the other hand it seems to be common to describe the possible return values in complete e.g. in the malloc documentation: For calloc() and malloc(), return a pointer to the allocated memory.... On error, these functions return NULL. NULL may also be returned by a successful call to malloc() with a size of zero, ... It doesn't say here: Better call malloc only if enough memory is available and don't try to use this function to allocate null bytes.
(In reply to comment #3) > On the other hand it seems to be common to describe the possible return values > in complete e.g. in the malloc documentation: malloc isn't in a library whose policy is to terminate on memory allocation failures; GObject is. > It doesn't say here: Better call malloc only if enough memory is available and > don't try to use this function to allocate null bytes. Neither does it say "if you have previously overrun a heap buffer, malloc might crash", which is equally true. The C standard formalizes this sort of thing as "undefined behaviour", e.g. for realloc: If ptr does not match a pointer returned earlier by calloc(), malloc() or realloc() or if the space has previously been deallocated by a call to free() or realloc(), the behaviour is undefined. Some libraries word a similar situation as "it is an error if...". If g_type_register_fundamental documented its error conditions, it should look more like this: If @type_id is already registered, or a type named @type_name is already registered, the behaviour is undefined or It is an error to call this function for a @type_id or @type_name that already exists.
Only partly agree but your last two proposals are fine. An ASSERT would also help.
Created attachment 208101 [details] [review] improve g_type_register_fundamental() description
(In reply to comment #6) > Created an attachment (id=208101) > improve g_type_register_fundamental() description I'm not a GLib reviewer, but this looks fine to me.
Review of attachment 208101 [details] [review]: sure, looks fine
The following fix has been pushed: 500aafd docs: Clarify g_type_register_fundamental() behaviour
Created attachment 208465 [details] [review] docs: Clarify g_type_register_fundamental() behaviour