|Summary:||x11-libs/vte-0.42.4 uses NULL as sentinel in variadic functions, fails to emerge with musl|
|Product:||Gentoo Linux||Reporter:||Anthony Basile <blueness>|
|Component:||[OLD] Library||Assignee:||Gentoo Linux Gnome Desktop Team <gnome>|
|Package list:||Runtime testing required:||---|
|Bug Depends on:|
|Attachments:||Replace NULL with (void*)0 when used as sentinel in variadic functions|
Description Anthony Basile 2016-02-29 10:36:20 UTC
Created attachment 426976 [details] Replace NULL with (void*)0 when used as sentinel in variadic functions NULL is defined as a null pointer constant, but this causes problems when 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  http://ewontfix.com/11/  https://gustedt.wordpress.com/2010/11/07/dont-use-null/
Comment 1 Anthony Basile 2016-02-29 10:39:12 UTC
I have tested the patch on glibc/uclibc and musl and it works. I have submitted the patch upstream.
Comment 2 Tolga Dalman 2016-02-29 12:32:23 UTC
Umm ... while your patch looks correct, I don't think this is the right place to fix. My reading is that musl's NULL implementation (0L) is wrong (albeit complying to C99/C11) and can lead to various interesting problems. From glib API point-of-view, this also seems wrong. The API expects NULL as terminator, not (void*) 0. Note, that this is the definition for __attribute__((sentinel)): --------------------------------- sentinel This function attribute ensures that a parameter in a function call is an explicit NULL. The attribute is only valid on variadic functions. By default, the sentinel is located at position zero, the last parameter of the function call. If an optional integer position argument P is supplied to the attribute, the sentinel must be located at position P counting backwards from the end of the argument list. __attribute__ ((sentinel)) is equivalent to __attribute__ ((sentinel(0))) The attribute is automatically set with a position of 0 for the built-in functions execl and execlp. The built-in function execle has the attribute set with a position of 1. A valid NULL in this context is defined as zero with any pointer type. If your system defines the NULL macro with an integer type then you need to add an explicit cast. GCC replaces stddef.h with a copy that redefines NULL appropriately. The warnings for missing or incorrect sentinels are enabled with -Wformat. --------------------------------- (https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#Common-Function-Attributes)
Comment 3 Anthony Basile 2016-02-29 18:20:59 UTC
Upsteam has included the fix in git/master and 0.44.
Comment 4 Pacho Ramos 2016-02-29 19:42:18 UTC
(In reply to Anthony Basile from comment #3) > Upsteam has included the fix in git/master and 0.44. Feel free to go ahead and commit it yourself if you need it just now :) (please add a comment with a link to the bug for example for future reference) Thanks
Comment 5 Anthony Basile 2016-02-29 22:44:36 UTC
(In reply to Pacho Ramos from comment #4) > (In reply to Anthony Basile from comment #3) > > Upsteam has included the fix in git/master and 0.44. > > Feel free to go ahead and commit it yourself if you need it just now :) > (please add a comment with a link to the bug for example for future > reference) > > Thanks Unfortunately nullptr is only in c++11 and above , and we don't want to be introducing c++11 yet. If I back port this, I would only be able to do so as ((void*)0). I think we can wait, but can the next stabilization candidate be 0.44 so we don't have to wait forever?  http://en.cppreference.com/w/cpp/language/nullptr
Comment 6 Pacho Ramos 2016-03-05 10:24:53 UTC
Well, I cannot promise we will be able to stabilize vte-0.44 for Gnome 3.18 as they are usually tied to gnome versions (and, then, you would need to wait for Gnome 3.20)... but if 3.18 terminal and co. work with newer vte, no problem
Comment 7 Anthony Basile 2016-03-05 12:27:23 UTC
(In reply to Pacho Ramos from comment #6) > Well, I cannot promise we will be able to stabilize vte-0.44 for Gnome 3.18 > as they are usually tied to gnome versions (and, then, you would need to > wait for Gnome 3.20)... but if 3.18 terminal and co. work with newer vte, no > problem Okay, let's play it by ear. The musl overlay is pretty big, and while the hope is to get everything into the tree, this really means teaching a lot of upstreams a lot about proper standards. musl is interesting because of its very close adherence to POSIX, SUSv3/4, etc as well as secure programming so it exposes a lot of these problems.