[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 9 Jul 2019 15:38:27 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: [PATCH] Define NULL as __null in C++ mode when using GCC
or Clang.
On Tue, Jul 09, 2019 at 03:19:00PM -0400, James Y Knight wrote:
> Both GCC and Clang ship their own stddef.h which does this (musl's
> stddef.h is simply ignored). But, musl also defines the macro in a
> number of other headers.
This was intentional at the time it was done. At least in the present
version of C++ at the time, __null was not a conforming definition.
For example, conforming applications could parse the stringification
of NULL and expect to get something that's a valid null pointer
constant. This could possibly be revisited, if it's changed, and if it
can be done in a way that doesn't involve compiler-specific stuff.
> Thus, depending on which header you include
> last, you'll end up with a different definition of NULL.
musl doesn't support use of the compiler versions of these headers,
for several reasons that have nothing to do with NULL.
> Mostly, getting musl's definition simply degrades warning diagnostics
> in C++ slightly -- e.g. GCC can no longer emit this warning:
> warning: passing NULL to non-pointer argument 1 of 'int foo(long int)'
> [-Wconversion-null]
The current definition however catches bugs where NULL is wrongly used
to terminate a variadic argument list where (void*)0 or (char*)0 is
actually needed. See commits 41d7c77d6a2e74294807d35062e4cd1d48ab72d3
and c8a9c22173f485c8c053709e1dfa0a617cb6be1a. So both choices are a
tradeoff in diagnostic capability.
> If you're using Clang's modules support, it can also break
> compilation. In that case, the conflicting definitions may be detected
> as a module incompatibility.
I'm not sure what this means. The conflicting definition should never
appear in code that's consistent with compiling and linking with musl.
> A different (potentially better) fix would be to always retrieve the
> definition of NULL from the compiler's stddef.h (via #define
> __need_NULL #include <stddef.h>). It may also be best to delete the
> musl stddef.h entirely for clarity, since it's currently ignored,
> anyhow.
We intentionally don't do things that way.
Rich
> But, this seemed the more minimal fix.
> From 3d898d4825c28f08ade92c40822fa5bfa2ef1f1f Mon Sep 17 00:00:00 2001
> From: James Y Knight <jyknight@...gle.com>
> Date: Tue, 9 Jul 2019 15:03:03 -0400
> Subject: [PATCH] Define NULL as __null in C++ mode when using GCC or Clang.
>
> Both GCC and Clang ship their own stddef.h which does this (musl's
> stddef.h is simply ignored). But, musl also defines the macro in a
> number of other headers. Thus, depending on which header you include
> last, you'll end up with a different definition of NULL.
>
> Mostly, getting musl's definition simply degrades warning diagnostics
> in C++ slightly -- e.g. GCC can no longer emit this warning:
> warning: passing NULL to non-pointer argument 1 of 'int foo(long int)' [-Wconversion-null]
>
> If you're using Clang's modules support, it can also break
> compilation. In that case, the conflicting definitions may be detected
> as a module incompatibility.
>
> A different (potentially better) fix would be to always retrieve the
> definition of NULL from the compiler's stddef.h (via #define
> __need_NULL #include <stddef.h>). It may also be best to delete the
> musl stddef.h entirely for clarity, since it's currently ignored,
> anyhow.
>
> But, this seemed the more minimal fix.
> ---
> include/locale.h | 4 ++++
> include/stddef.h | 4 ++++
> include/stdio.h | 4 ++++
> include/stdlib.h | 4 ++++
> include/string.h | 4 ++++
> include/time.h | 4 ++++
> include/unistd.h | 4 ++++
> include/wchar.h | 4 ++++
> 8 files changed, 32 insertions(+)
>
> diff --git a/include/locale.h b/include/locale.h
> index ce384381..80c2d2db 100644
> --- a/include/locale.h
> +++ b/include/locale.h
> @@ -8,7 +8,11 @@ extern "C" {
> #include <features.h>
>
> #ifdef __cplusplus
> +#ifdef __GNUC__
> +#define NULL __null
> +#else
> #define NULL 0L
> +#endif
> #else
> #define NULL ((void*)0)
> #endif
> diff --git a/include/stddef.h b/include/stddef.h
> index bd753853..415a2d91 100644
> --- a/include/stddef.h
> +++ b/include/stddef.h
> @@ -2,7 +2,11 @@
> #define _STDDEF_H
>
> #ifdef __cplusplus
> +#ifdef __GNUC__
> +#define NULL __null
> +#else
> #define NULL 0L
> +#endif
> #else
> #define NULL ((void*)0)
> #endif
> diff --git a/include/stdio.h b/include/stdio.h
> index 3604198c..9e30d1ad 100644
> --- a/include/stdio.h
> +++ b/include/stdio.h
> @@ -26,7 +26,11 @@ extern "C" {
> #include <bits/alltypes.h>
>
> #ifdef __cplusplus
> +#ifdef __GNUC__
> +#define NULL __null
> +#else
> #define NULL 0L
> +#endif
> #else
> #define NULL ((void*)0)
> #endif
> diff --git a/include/stdlib.h b/include/stdlib.h
> index 42ca8336..a272a4f4 100644
> --- a/include/stdlib.h
> +++ b/include/stdlib.h
> @@ -8,7 +8,11 @@ extern "C" {
> #include <features.h>
>
> #ifdef __cplusplus
> +#ifdef __GNUC__
> +#define NULL __null
> +#else
> #define NULL 0L
> +#endif
> #else
> #define NULL ((void*)0)
> #endif
> diff --git a/include/string.h b/include/string.h
> index 795a2abc..4d344d72 100644
> --- a/include/string.h
> +++ b/include/string.h
> @@ -8,7 +8,11 @@ extern "C" {
> #include <features.h>
>
> #ifdef __cplusplus
> +#ifdef __GNUC__
> +#define NULL __null
> +#else
> #define NULL 0L
> +#endif
> #else
> #define NULL ((void*)0)
> #endif
> diff --git a/include/time.h b/include/time.h
> index 672b3fc3..0eec373c 100644
> --- a/include/time.h
> +++ b/include/time.h
> @@ -8,7 +8,11 @@ extern "C" {
> #include <features.h>
>
> #ifdef __cplusplus
> +#ifdef __GNUC__
> +#define NULL __null
> +#else
> #define NULL 0L
> +#endif
> #else
> #define NULL ((void*)0)
> #endif
> diff --git a/include/unistd.h b/include/unistd.h
> index 9485da7a..391b58ba 100644
> --- a/include/unistd.h
> +++ b/include/unistd.h
> @@ -16,7 +16,11 @@ extern "C" {
> #define SEEK_END 2
>
> #ifdef __cplusplus
> +#ifdef __GNUC__
> +#define NULL __null
> +#else
> #define NULL 0L
> +#endif
> #else
> #define NULL ((void*)0)
> #endif
> diff --git a/include/wchar.h b/include/wchar.h
> index 88eb55b1..bfdbaebb 100644
> --- a/include/wchar.h
> +++ b/include/wchar.h
> @@ -39,7 +39,11 @@ extern "C" {
> #endif
>
> #ifdef __cplusplus
> +#ifdef __GNUC__
> +#define NULL __null
> +#else
> #define NULL 0L
> +#endif
> #else
> #define NULL ((void*)0)
> #endif
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
