Re: g_error_free() warning on null pointer
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
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
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
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
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
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
Some Dead Keys not supported
I’m not sure this is the correct place to report this, but I have the following issue with keyboard input in GTK (and this extends to almost my entire Linux). Recently, I installed the german T3 keyboard, which has amongst many special characters also some additional dead keys, namely for the comma below and the stroke. Comma below is used for typing Romanian (ș≠ş, ț≠ţ), the stroke is used for (ħ, ŧ, used in Maltese). In xterm and basic X programs, this works as expected, but in GTK applications, this dead key is simply ignored, so I get an s instead of ș. There is a definition for the Windows keyboard that also implements the T3 keyboard (or rather a subset of it, the T2 keyboard), which uses a special dead key sequence for giving access to Level 5-8 of the keyboard (under Linux, this is done by pressing AltGr+Shift, so no deadkey needed there). Windows-GTK-programs have the same issue. the deadkeys stroke, comma below and this special dead key sequence are ignored (and other keyboard layouts which also use differing keyboard mappings, such as the german NEO layout have similar problems) What I gathered is that GTK brings its very own keyboard stroke to character engine, and apparently simply ignores the dead key sequences it doesn't know about. Is this assumption correct? And is there anything one can do to either let X11 do its thing or have GTK learn about the other deadkey sequences? (the latter one would help with my Windows-GTK-Problems at Work, since thatʹs the only place I have Windows running) -- Cheers, Alex ___ 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
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
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
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
