GNOME Bugzilla – Bug 762863
Variadic functions g_object_get() and g_ptr_array_add() should not use NULL as sentinels
Last modified: 2016-02-29 18:09:34 UTC
Created attachment 322643 [details] Replace NULL with (void*)0 when used as sentinel in variadic functions NULL is defined as a null pointer constant, but this causes problemsvwhen used in variadic functions, leading to hard to diagnose crashes. In fact, musl intentionally defines NULL to be 0L to throw errors in these cases. This patch replaces NULL used as sentinels in variadic functions with (void *)0. See [1] http://ewontfix.com/11/ [2] https://gustedt.wordpress.com/2010/11/07/dont-use-null/
Downstream bug report at https://bugs.gentoo.org/show_bug.cgi?id=575984
Should use nullptr instead (void*)0.
Dafuq did I just read? :) Seriously, the problem with this is that I believe it's a totally uncommon knowledge (entirely new to me as well, and I have to admin I didn't take time to read and understand it carefully), and we might fix these one-offs right now, but what's the protection against similar new ones sneaking in? Any gcc warnings or maybe other tricks (like redefining NULL to 0L for debug builds) to catch these? Oh, by the way, is "(void *)NULL" intended in the patch or is it a typo? Doesn't matter if we go for "nullptr" though, so let's do this.
s/admin/admit Also, I'd add a short comment (e.g. referring to this bug) to the relevant lines. Otherwise, I'd never figure out just by looking at the source.
Just want to add that this has been a known issue within the GNOME C++ community. At least, that's where I learned about ~8 years ago on Planet GNOME.
Yes, I've known this issue. However, afaik this isn't a problem with gcc on glibc, since it does the right thing. (From a quick test, after preprocessing, the NULL has turned into __null.) But let's go with nullptr everywhere. I'll add -Wstrict-null-sentinel to the set of warnings we use.
Fixed on master and 0-44.
(In reply to Christian Persch from comment #7) > Fixed on master and 0-44. Tested and it works fine on musl. Thanks!