Re: g_error_free() warning on null pointer



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



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]