Re: g_error_free() warning on null pointer

[Nicolas George]

Le duodi 12 fructidor, an CCXXIII, Michael McConville a écrit :
>  1) Sometimes you have to audit other people's code for memory leaks but
> don't have the time/understanding for logic additions.

Well, in that case, you should be happy: if you have to audit other people's
code and see a GError being freed unconditionally, then you know something
is wrong.

> 
>  2) Even if you are handling the error, the free call is clutter.
> Keeping it in the function's footer contributes readability.

Freeing resources is clutter, but doing so at the logical place is less
clutter and better for readability. "Everything in the footer" is not the
logical place, it is the brainless place.

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gtk-devel-list

Re: g_error_free() warning on null pointer

[Simon McVittie]

On 01/09/15 04:03, Michael McConville wrote:
> Simon McVittie wrote:
>> The intention throughout the GLib-based stack is that if a
>> g_return[_val]_if_fail() is hit, it indicates that the caller has
>> called the function incorrectly, in a way that is considered to be
>> undefined behaviour
> 
> I still don't understand how this is relevant. I already understand what
> undefined behavior is and how it relates to compilers/optimization.
> However, this sort of smoke and mirrors inexactness doesn't apply to
> GLib - there's a single implementation and we can just go and look at
> the source code and see what it does.

You can't look at the source code of all future versions of GLib,
because they don't exist yet.

If something is documented to work, works in GLib 2.44, and doesn't work
in GLib 2.45, then that's a regression (a GLib bug) and should be fixed.

If something is not documented to work, and in GLib 2.44 it emits
warnings about how it shouldn't work but your program happens to survive
anyway, whereas in GLib 2.45 your program crashes, then that is not a
GLib bug and is unlikely to be fixed.

It's essentially the same principle as undefined behaviour in compilers:
even if the only compiler you care about is gcc, "this undefined
behaviour happens to produce the result I wanted in gcc-4.9.1" does not
imply anything about how gcc-5 will compile the same source code.

-- 
Simon McVittie
Collabora Ltd. 

___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gtk-devel-list

Re: g_error_free() warning on null pointer

[Michael McConville]

Simon McVittie wrote:
> On 16/08/15 20:23, Michael McConville wrote:
> > Emmanuele Bassi wrote:
> >> You expected the *_free() functions in GLib to be NULL-safe. They
> >> aren't, except for g_free().
> > 
> > g_error_free is. Maybe others too, I haven't checked. It just prints
> > annoying console warnings.
> 
> The intention throughout the GLib-based stack is that if a
> g_return[_val]_if_fail() is hit, it indicates that the caller has
> called the function incorrectly, in a way that is considered to be
> undefined behaviour
> . Undefined
> behaviour does not mean "your code will crash"; it means "any result
> is valid, and it is up to the compiler or library author what
> happens". It does in practice end up meaning "your code is wrong",
> because the majority of the possible implementations of undefined
> behaviour are not what you wanted.

I still don't understand how this is relevant. I already understand what
undefined behavior is and how it relates to compilers/optimization.
However, this sort of smoke and mirrors inexactness doesn't apply to
GLib - there's a single implementation and we can just go and look at
the source code and see what it does. In the case of g_error_free, this
is:

 1) print an annoying warning
 2) call g_free(), which is NULL-safe
 3) call g_slice_free(), which is NULL-safe
___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gtk-devel-list

Re: g_error_free() warning on null pointer

[Sébastien Wilmet]

On Mon, Aug 31, 2015 at 11:03:40PM -0400, Michael McConville wrote:
>  1) print an annoying warning

Not always, as Simon explained already, the g_return_if_fail()/...
functions can be disabled, in which case the program continues its
execution and will usually crash.

See the --enable-debug configure option of GLib:
https://developer.gnome.org/glib/unstable/glib-building.html

In the docs, if it is not mentioned whether NULL is accepted or not, it
means that it is not supported. Except when the documentation needs to
be fixed, of course, which can happen from time to time.

--
Sébastien
___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gtk-devel-list

Re: g_error_free() warning on null pointer

[Michael McConville]

Nicolas George wrote:
 But maybe I am forgetting another case: can you imagine a code snippet
 where g_error_free(error) would make sense with error == NULL?

I may have already mentioned this, but the simplest example is just
adding a g_error_free() at the end of a function when adding a GError*
declaration at the beginning. This prevents an irrelevant line from
cluttering the error-handling logic and makes it more trivially obvious
to auditors that there isn't a memory leak.
___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gtk-devel-list

Re: g_error_free() warning on null pointer

[Jasper St. Pierre]

But if you have an error, shouldn't you either want to a) handle it,
or b) propagate it up?

I can't imagine a case where you want to free a GError without first
attempting to handle it. If you do want to ignore errors, you can
simply pass NULL for a GError ** pointer to a function, since those
should always handle NULL correctly.

On Sat, Aug 29, 2015 at 11:13 AM, Michael McConville
mmcco...@sccs.swarthmore.edu wrote:
 Nicolas George wrote:
 But maybe I am forgetting another case: can you imagine a code snippet
 where g_error_free(error) would make sense with error == NULL?

 I may have already mentioned this, but the simplest example is just
 adding a g_error_free() at the end of a function when adding a GError*
 declaration at the beginning. This prevents an irrelevant line from
 cluttering the error-handling logic and makes it more trivially obvious
 to auditors that there isn't a memory leak.
 ___
 gtk-devel-list mailing list
 gtk-devel-list@gnome.org
 https://mail.gnome.org/mailman/listinfo/gtk-devel-list



-- 
  Jasper
___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gtk-devel-list

Re: g_error_free() warning on null pointer

[Michael McConville]

Jasper St. Pierre wrote:
 Michael McConville wrote:
  Nicolas George wrote:
  But maybe I am forgetting another case: can you imagine a code snippet
  where g_error_free(error) would make sense with error == NULL?
 
  I may have already mentioned this, but the simplest example is just
  adding a g_error_free() at the end of a function when adding a GError*
  declaration at the beginning. This prevents an irrelevant line from
  cluttering the error-handling logic and makes it more trivially obvious
  to auditors that there isn't a memory leak.
 
 But if you have an error, shouldn't you either want to a) handle it,
 or b) propagate it up?

I suspect that this is almost always the case. However:

 1) Sometimes you have to audit other people's code for memory leaks but
don't have the time/understanding for logic additions.

 2) Even if you are handling the error, the free call is clutter.
Keeping it in the function's footer contributes readability.
___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gtk-devel-list

Re: g_error_free() warning on null pointer

[Simon McVittie]

On 16/08/15 20:23, Michael McConville wrote:
 Emmanuele Bassi wrote:
 You expected the *_free() functions in GLib to be NULL-safe. They
 aren't, except for g_free().
 
 g_error_free is. Maybe others too, I haven't checked. It just prints
 annoying console warnings.

The intention throughout the GLib-based stack is that if a
g_return[_val]_if_fail() is hit, it indicates that the caller has called
the function incorrectly, in a way that is considered to be undefined
behaviour https://en.wikipedia.org/wiki/Undefined_behavior. Undefined
behaviour does not mean your code will crash; it means any result is
valid, and it is up to the compiler or library author what happens. It
does in practice end up meaning your code is wrong, because the
majority of the possible implementations of undefined behaviour are not
what you wanted.

As a convenience to developers and users, if checks are enabled in
GLib, it tries to cope with certain classes of undefined behaviour
somewhat gracefully, and emit a diagnostic (complain on stderr) rather
than just crashing. However, this is not part of GLib's API, and in
particular, if checks are not enabled, it will usually crash instead.

See https://bugzilla.gnome.org/show_bug.cgi?id=660809 for more on this.

-- 
Simon McVittie
Collabora Ltd. http://www.collabora.com/

___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gtk-devel-list

Re: g_error_free() warning on null pointer

[Sébastien Wilmet]

On Sat, Aug 15, 2015 at 01:52:02PM -0700, Jasper St. Pierre wrote:
 Lots of things in GLib fail when passed a NULL pointer, like g_object_unref.
 
 The idea is that passing a NULL pointer is probably a sign of a bug
 somewhere, so you shouldn't do it.

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.

--
Sébastien
___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gtk-devel-list

Re: g_error_free() warning on null pointer

[Emmanuele Bassi]

Hi;

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.

On 16 August 2015 at 09:25, Sébastien Wilmet swil...@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.

unref() is not a free function, so there goes the consistency
argument for it. All the reference counting functions must *not*
accept NULL; if you're unreffing a NULL value then there is reference
counting bug in your code, because it means something is holding a
pointer to an object, nullifying it, and still trying to access it; if
that happens, your code should keep an actual reference. Silently
discarding NULL is going to mask this bug.

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.

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.

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.

Ciao,
 Emmanuele.

-- 
https://www.bassi.io
[@] ebassi [@gmail.com]
___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gtk-devel-list

Re: g_error_free() warning on null pointer

[Sébastien Wilmet]

On Sun, Aug 16, 2015 at 10:49:56AM +0100, Emmanuele Bassi 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.
 
 unref() is not a free function, so there goes the consistency
 argument for it.

A free function can be seen as a special case of unref where the pointed
memory can have at most one reference. free and unref are used for the
same purpose, to release memory. Consistency between the two makes
sense. If you need to remember different rules for two similar cases,
then you have an inconsistency.

 All the reference counting functions must *not*
 accept NULL; if you're unreffing a NULL value then there is reference
 counting bug in your code, because it means something is holding a
 pointer to an object, nullifying it, and still trying to access it; if
 that happens, your code should keep an actual reference. Silently
 discarding NULL is going to mask this bug.

If you have a NULL pointer and you unref it, it can mean:
- that the object was never initialized
- that unref has already been called

For the first case, I've given a typical use case: at the end of a
function you release the variables that you don't need anymore; some of
them are maybe not initialized, e.g. if you have an early error and goto
directly to the end of the function.

For the second case, if some code is still trying to access the object,
you'll get warnings when using other public functions, that is, other
functions that do some actual work on the object. The job of unref() is
just to release the reference, if the pointer is NULL it means that the
job is already done. Lots of public functions do a NOP when the job is
already done. Being able to do several times the same operation, to be
sure that it's done, can simplify the code. You don't need to add some
logic to know if the operation is already done or not, you just call the
function.

 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.

Calling g_object_unref() also generally implies to nullify the pointer,
except if the variable goes out of scope.

What we see nowadays is to call g_clear_*() at the end of a function,
just for convenience to avoid the if(foo != NULL). Even if nullifying
the pointer is useless. It's sort of a small hack. It would work equally
fine if we can do in dispose():

g_object_unref (foo);
foo = NULL;

In C the latter is more natural imo. Setting a variable to NULL after
freeing it is a common pattern.

(it's just one more line, but it would already be more convenient than
having conditions)

--
Sébastien
___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gtk-devel-list

Re: g_error_free() warning on null pointer

[Emmanuele Bassi]

Hi;

On 16 August 2015 at 16:57, Michael McConville
mmcco...@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 swil...@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]
___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gtk-devel-list

Re: g_error_free() warning on null pointer

[Michael McConville]

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:

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.

 Sébastien Wilmet swil...@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. The source code's there, so we
can read it. And there's only one kind of NULL, so we can just test it
too. And there's only one GLib.

 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, forcing NULL checks often leads to memory leaks because
people get unduly conservative about when they free.

 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.
___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gtk-devel-list

Re: g_error_free() warning on null pointer

[Nicolas George]

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


signature.asc
Description: Digital signature
___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gtk-devel-list

Re: g_error_free() warning on null pointer

[Sébastien Wilmet]

On Sun, Aug 16, 2015 at 11:53:16AM +0200, Nicolas George wrote:
 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.

So if you design a library like that, your users will need to read the
documentation everytime they use as simple functions as free/unref.

Consistency is important for a programming language or for a library to
save time to the programmer (and not turning crazy).

To take another example, I always use bitfields to add booleans in a
struct. So, instead of:

struct
{
gboolean blah;
};

I always write:

struct
{
guint blah : 1;
};

A bitfield of 1 bit can only be used for a boolean, so the code is
almost as clear as using the more precise type.

Since gboolean is a typedef to a gint, for heavily-used structs it's
better to pack booleans at the end of the struct with bitfields, to
save some memory. For less-heavily used structs (the extreme case being
a singleton), other people prefer to use gboolean because it's slightly
clearer, and fields can be grouped more logically instead of being
forced to put booleans at the end. But in that case, you always need to
think is my struct heavily used or not?. The answer may be different
over time, after adding a new feature for example. And what does
heavily-used mean exactly? It also depends on how many booleans you
have in the struct.

It's far simpler and quicker to always apply the same convention.

--
Sébastien
___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gtk-devel-list

Re: g_error_free() warning on null pointer

[Michael McConville]

Emmanuele Bassi wrote:
  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.

I doubt they bother. I was annoyed by these console warnings for five
months but never bothered to look into it until I had to rework a number
of GError uses. Other Pidgin developers were also surprised by this.

 You expected the *_free() functions in GLib to be NULL-safe. They
 aren't, except for g_free().

g_error_free is. Maybe others too, I haven't checked. It just prints
annoying console warnings. This really isn't a big deal - it's strange
that some are being so political about it.

 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.

Agreed, a lot of people don't know about free being NULL-safe. A lot of
people do, though.
___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gtk-devel-list

Re: g_error_free() warning on null pointer

[Nicolas George]

Le nonidi 29 thermidor, an CCXXIII, Sébastien Wilmet a écrit :
 So if you design a library like that, your users will need to read the
 documentation everytime they use as simple functions as free/unref.

No, only when they consider abusing them.

 Consistency is important for a programming language or for a library to
 save time to the programmer (and not turning crazy).

Detecting bugs as early and efficiently as possible saves more time to the
programmer than any kind of consistency.

I have nothing more to add to my first mail, and your example about boolean
bitfields does not convince me of anything.

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gtk-devel-list

Re: g_error_free() warning on null pointer

[Michael McConville]

Paul wrote:
 Michael McConville wrote:
  Paul wrote:
   Recently I was re-educated by the Java people that using a null on
   free is how they regularly force various Java handling routines
   they rely on SEGFAULT.
 
  Is that relevant? This is C, not Java.

 What do you Java is written in?

I'm pretty sure you were misunderstanding them... do you have an example
of a C standard library implementation that does this?
___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gtk-devel-list

Re: g_error_free() warning on null pointer

[Michael McConville]

Jasper St. Pierre wrote:
 Lots of things in GLib fail when passed a NULL pointer, like
 g_object_unref.

I'm talking about free()-family functions specifically, though. They
have a POSIX-specified behavior that people expect.

 The idea is that passing a NULL pointer is probably a sign of a bug
 somewhere, so you shouldn't do it.

I definitely don't think it's evidence of a bug. This is common practice
in POSIX-compliant C. More to follow.

 While the C standard's free() is NULL-safe, I'd say that this is quite
 strange for the C standard, since any other invalid pointer passed to
 free() would (most likely, though yes, implementation-defined) crash.

Rather than strange, I'd say it was intentional and insightful. In
function bodies, you often have pointers that are used in conditions and
may or may not be null. There's a very easy and clean way to deal with
this: initialize them to NULL at the beginning and free them at the end.
Freeing conditionally in the body can be awkward and often leads to
memory leaks. I can send you a few examples from the Pidgin codebase if
you're interested.

More circumstantially, the entire POSIX description for free() is four
sentences. I doubt that allowing null pointers was added as a
complicating distraction. It's a core part of a simple interface.

I'm not insisting that you code this way. Most simply, I'd say:

 * many programmers already assume POSIX compliance
 * g_free() already complies with POSIX on this
 * people clearly associate g_error_free() et al. with free() and
   g_free()
 * these functions might as well behave the same

I really can't imagine anyone using these warnings. I can imagine people
being distracted or misled by them, though - I was.
___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gtk-devel-list

Re: g_error_free() warning on null pointer

[Michael McConville]

Christian Hergert wrote:
 Try g_clear_error(error) instead of g_error_free(error) to get the
 effect you want.

I had discovered this workaround.  :-)  However, I think it would be
really nice if this (very trivial) change were made in glib. For
example, with Pidgin we'd either have to replace every g_error_free() in
~300,000 lines of code (might cause unexpected breakage, people might be
irked) or just keep putting up with the warnings.
___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gtk-devel-list

Re: g_error_free() warning on null pointer

[Paul]

Recently I was re-educated by the Java people that using a null on free
is how they regularly force various Java handling routines they rely on 
SEGFAULT.





On 8/15/2015 4:54 PM, Michael McConville wrote:

Jasper St. Pierre wrote:

Lots of things in GLib fail when passed a NULL pointer, like
g_object_unref.

I'm talking about free()-family functions specifically, though. They
have a POSIX-specified behavior that people expect.


The idea is that passing a NULL pointer is probably a sign of a bug
somewhere, so you shouldn't do it.

I definitely don't think it's evidence of a bug. This is common practice
in POSIX-compliant C. More to follow.


While the C standard's free() is NULL-safe, I'd say that this is quite
strange for the C standard, since any other invalid pointer passed to
free() would (most likely, though yes, implementation-defined) crash.

Rather than strange, I'd say it was intentional and insightful. In
function bodies, you often have pointers that are used in conditions and
may or may not be null. There's a very easy and clean way to deal with
this: initialize them to NULL at the beginning and free them at the end.
Freeing conditionally in the body can be awkward and often leads to
memory leaks. I can send you a few examples from the Pidgin codebase if
you're interested.

More circumstantially, the entire POSIX description for free() is four
sentences. I doubt that allowing null pointers was added as a
complicating distraction. It's a core part of a simple interface.

I'm not insisting that you code this way. Most simply, I'd say:

  * many programmers already assume POSIX compliance
  * g_free() already complies with POSIX on this
  * people clearly associate g_error_free() et al. with free() and
g_free()
  * these functions might as well behave the same

I really can't imagine anyone using these warnings. I can imagine people
being distracted or misled by them, though - I was.
___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gtk-devel-list




___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gtk-devel-list

Re: g_error_free() warning on null pointer

[Allin Cottrell]

On Sat, 15 Aug 2015, Jasper St. Pierre wrote:


Lots of things in GLib fail when passed a NULL pointer, like g_object_unref.

The idea is that passing a NULL pointer is probably a sign of a bug
somewhere, so you shouldn't do it. [...]


On the one hand, the function is question is specialized, it's not 
free(), nor g_free(), and no standard mandates that it should accept 
a NULL pointer as a no-op.


On the other hand, if this function in fact accepts a NULL without 
error it's perverse that it should make a fuss about it. It seems to 
me that by far the most likely case of g_error_free() getting a NULL 
argument is a coder expecting NULL - no-op semantics by analogy 
with C's free(). What sort of bug would you have in mind?


@McConville: the standard most relevant to your argument is the ISO 
C standard, not POSIX.


Allin Cottrell
___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gtk-devel-list

Re: g_error_free() warning on null pointer

[Michael McConville]

Allin Cottrell wrote:
 On the one hand, the function is question is specialized, it's not
 free(), nor g_free(), and no standard mandates that it should accept a
 NULL pointer as a no-op.

Agreed. 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. This
probably applies to other free()-like functions as well (I'm very new to
glib).

 On the other hand, if this function in fact accepts a NULL without
 error it's perverse that it should make a fuss about it. It seems to
 me that by far the most likely case of g_error_free() getting a NULL
 argument is a coder expecting NULL - no-op semantics by analogy with
 C's free(). What sort of bug would you have in mind?
___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gtk-devel-list

Re: g_error_free() warning on null pointer

[Paul]

On 8/15/2015 7:05 PM, Michael McConville wrote:

Paul wrote:

Recently I was re-educated by the Java people that using a null on
free is how they regularly force various Java handling routines they
rely on SEGFAULT.

Is that relevant? This is C, not Java.



What do you Java is written in?
___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gtk-devel-list

Re: g_error_free() warning on null pointer

[Jasper St. Pierre]

Lots of things in GLib fail when passed a NULL pointer, like g_object_unref.

The idea is that passing a NULL pointer is probably a sign of a bug
somewhere, so you shouldn't do it. While the C standard's free() is
NULL-safe, I'd say that this is quite strange for the C standard,
since any other invalid pointer passed to free() would (most likely,
though yes, implementation-defined) crash.

On Sat, Aug 15, 2015 at 1:28 PM, Michael McConville
mmcco...@sccs.swarthmore.edu wrote:
 If you call g_error_free() on a null pointer, you'll see something like
 this:

 (process:345): GLib-CRITICAL **: g_error_free: assertion 'error != NULL' 
 failed

 However, POSIX specifies:

 If ptr is a null pointer, no action shall occur.

 http://pubs.opengroup.org/onlinepubs/009695399/functions/free.html

 From what I've seen, all major Unices comply with this and mention it in
 their man page. While printing that warning may not be a program action,
 it's definitely disconcerting and dilutes debug logs. It seems to me
 that POSIX intends free() to be used in situations in which the pointer
 may be null. This is common practice.

 Additionally, g_clear_error() doesn't print a warning when passed NULL.

 Fixing this would just involve removing this line:

 https://github.com/GNOME/glib/blob/master/glib/gerror.c#L467

 Thoughts?
 ___
 gtk-devel-list mailing list
 gtk-devel-list@gnome.org
 https://mail.gnome.org/mailman/listinfo/gtk-devel-list



-- 
  Jasper
___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gtk-devel-list

Re: g_error_free() warning on null pointer

[Christian Hergert]

Hi,

On 08/15/2015 02:54 PM, Michael McConville wrote:
 Rather than strange, I'd say it was intentional and insightful. In
 function bodies, you often have pointers that are used in conditions and
 may or may not be null. There's a very easy and clean way to deal with
 this: initialize them to NULL at the beginning and free them at the end.
 Freeing conditionally in the body can be awkward and often leads to
 memory leaks. I can send you a few examples from the Pidgin codebase if
 you're interested.

Try g_clear_error(error) instead of g_error_free(error) to get the
effect you want.

If you have a modern compiler/GLib, you can use g_autoptr() as well. I
usually avoid this with GError though, in case it gets used in multiple
paths.

-- Christian
___
gtk-devel-list mailing list
gtk-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gtk-devel-list