Re: g_error_free() warning on null pointer

[Emmanuele Bassi <ebassi gmail com>]

Hi;

On 16 August 2015 at 16:57, Michael McConville
<mmcconv1 sccs swarthmore edu> wrote:
> Emmanuele Bassi wrote:
> > this whole discussion seems to assume that GLib is, somehow, bound to
> > the C and/or POSIX specs even for API that the C/POSIX specs do not
> > cover. That is patently false. If we want to discuss improving the G*
> > platform API, let's do it without using fallacies like appeal to
> > authority in the form of the POSIX and ISO C working groups.
>
> Seems like you didn't read all of it then. This came up at least twice:

I did read the whole thread.

> Michael McConville wrote:
> > I wasn't suggesting that it's officially specified. I just think
> > that this aspect of free() is intentional and useful, and that people
> > have a reasonable expectation that g_error_free() will conform.

No, "people" don't have a reasonable expectation. Otherwise we would
have had many bugs about this, or many other email threads in the past
20 years. Please, don't try to generalise your issues.

You expected the *_free() functions in GLib to be NULL-safe. They
aren't, except for g_free(). There is no explicit need to make them
NULL-safe, nor expectation of functionality. To be fair, a lot of
people to this day do not know that free() is NULL-safe; the amount of
code I've personally seen over the past 15 years that does:

  if (foo)
    free (foo);

is staggering.

> > Sébastien Wilmet <swilmet gnome org> wrote:
> > > 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".
> >
> > [...]
> >
> > But, to be even more specific: whether or not free functions in G* can
> > handle NULL is undefined behaviour — except for g_free(), which is a
> > direct wrapper around free(), and as such it has to handle NULL by
> > virtue of the C spec. In short: do *not* pass NULL to free functions
> > provided by GLib, unless they are explicitly documented to allow NULL.
>
> That's what we're trying to change.  ;)  I don't think 'undefined' is a
> good term for it, though. It's not some kind of mystery or
> non-deterministic optimization outcome.

I meant "undefined" in the same way as the C spec uses the term
"undefined". Obviously is deterministic, and obviously you have the
code to look at. You're likely to have the code of the compiler you
use, so, in essence, all compiler-specific or undefined behaviour at
the spec level is, in essence, defined in the compiler.

Having the code, though, does not imply that the behaviour of the code
is specified. It could change internally, and your code would break —
and rightfully so.

> > There's a general discussion to be had if we should modify all of our
> > free functions to be NULL-safe; even though I generally code my free
> > functions to accept NULL, I consider adding a NULL check everywhere
> > inside the G* platform to be pointless churn. It also can mask bugs
> > for data stored inside other data structures, as opposed to allocated
> > inside a function.
>
> I don't follow. Can you share an example? On the other hand, as I
> mentioned

I was replying to the example from Sébastien, where he presented the
default use case as something that gets allocated at the beginning of
the function, and deallocated at its end. That is not the only use
case; allocating memory for the key/value pairs inside a hash table is
a typical case. Or simply having an allocated data structure inside
another allocated data structure — like a GObject.

> forcing NULL checks often leads to memory leaks because
> people get unduly conservative about when they free.

That is a weird statement. Leak happen regardless of NULL safety.

> > As a side note: the g_clear_* family of functions is meant to be used
> > to nullify a pointer variable after freeing its contents; these
> > functions are NULL-safe because they have to, since they need to
> > nullify the variable at the given address.
>
> I don't understand what you mean here either.

Again, I wasn't replying to your email — I was replying to Sébastien's
categorization of free functions that included the unref() and
g_clear_* families of functions.

Anyway: at this point, we're at an impasse.

Personally, I've yet to see a patch for this to even consider dealing
with this stuff. I'm definitely not interested in going through all
the free() functions inside GLib, GObject, GIO, and upper layers of
the stack, to add NULL-safety. If you want to submit a patch for
review, feel free to do so. Then the maintainers of each libraries
will decide whether or not to accept it. I'm pretty sure NULL-safety
for GLib has already been shot down a couple of times in Bugzilla,
though.

Ciao,
 Emmanuele.

-- 
https://www.bassi.io
[ ] ebassi [ gmail com]