Sébastien Wilmet:
No, accepting a NULL pointer for "free functions" is for convenience. At the beginning of the block, you initialize your variables at NULL, and at the end of the block (maybe after a goto label) you free the variables that need to be freed. It's not a sign of a bug… Consistency is important for a library. It's much simpler to remember "all free/unref functions accept NULL" instead of "free/g_free accept NULL, g_object_unref not, g_clear_object yes, etc etc".
Consistency is good, but it is not the only aspect of convenience. Catching common bugs is another aspect, pulling in the other direction. When freeing something, there is always the question "If I am freeing NULL here, is it a bug in my program?". If the answer is more frequently "no" then the free function should accept NULL silently; if the answer is more frequently "yes", then the free function should complain about NULL. The answer is not the same for all free functions. For generic memory buffers, the answer is frequently no: having a cleanup snippet that frees everything is both common and convenient. On the other hand, if a program is freeing an error without knowing that it is not NULL, I take it as a sign it is ignoring the error, and that is not to be encouraged. I mean: what are the common correct patterns of error management? I see two: handling or forwarding. Forwarding: gboolean do_something(params, GError **error) { ... if (!do_something_else(params, error)) return FALSE; ... return TRUE; } Handling: void do_something(params) { GError *error = NULL; ... if (!do_something_else(params, &error)) { fprintf(stderr, "That did not work: %s\n", error->message); g_error_free(error); return; } ... } In the forwarding case, the error belongs to the caller, it must not be freed. In the handling case, we know the error is not NULL, we can free it. In any other case, the error is NULL, we do not need to free it. But maybe I am forgetting another case: can you imagine a code snippet where g_error_free(error) would make sense with error == NULL? As a side note, the glib error API allows "do_something_else(params, NULL)" to ignore the error; but the above discussion would still be valid if it did not. Regards, -- Nicolas George
Attachment:
signature.asc
Description: Digital signature