Ensure proper NULL macro definition for system include files. #149176
Conversation
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-backend-x86 Author: Jamie Schmeiser (jamieschmeiser) ChangesThe C standard allows for at least 2 valid definitions of a null pointer constant and mandates that several standard headers files define the macro NULL to be a null pointer constant. Ensure that definitions of NULL are consistent across the various C header files. Patch is 20.22 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/149176.diff 14 Files Affected:
diff --git a/clang/lib/Headers/CMakeLists.txt b/clang/lib/Headers/CMakeLists.txt
index dd52498bbef4c..f1c10595ea38e 100644
--- a/clang/lib/Headers/CMakeLists.txt
+++ b/clang/lib/Headers/CMakeLists.txt
@@ -39,6 +39,25 @@ set(core_files
varargs.h
)
+set(aix_wrapper_files
+ dbm.h
+ stdio.h
+ stdlib.h
+ string.h
+ time.h
+ unistd.h
+ wchar.h
+ )
+set(aix_sys_subdir_wrapper_files
+ sys/dir.h
+ sys/param.h
+ sys/types.h
+ )
+set(aix_files
+ ${aix_wrapper_files}
+ ${aix_sys_subdir_wrapper_files}
+ )
+
set(arm_common_files
# Headers shared by Arm and AArch64
arm_acle.h
@@ -312,6 +331,7 @@ set(utility_files
set(files
${core_files}
+ ${aix_files}
${arm_common_files}
${arm_only_files}
${aarch64_only_files}
@@ -529,6 +549,7 @@ set_target_properties("clang-resource-headers" PROPERTIES
RUNTIME_OUTPUT_DIRECTORY "${output_dir}")
add_dependencies("clang-resource-headers"
"core-resource-headers"
+ "aix-resource-headers"
"arm-common-resource-headers"
"arm-resource-headers"
"aarch64-resource-headers"
@@ -557,6 +578,7 @@ add_header_target("core-resource-headers" ${core_files})
add_header_target("arm-common-resource-headers" "${arm_common_files};${arm_common_generated_files}")
# Architecture/platform specific targets
+add_header_target("aix-resource-headers" "${aix_files}")
add_header_target("arm-resource-headers" "${arm_only_files};${arm_only_generated_files}")
add_header_target("aarch64-resource-headers" "${aarch64_only_files};${aarch64_only_generated_files}")
add_header_target("cuda-resource-headers" "${cuda_files};${cuda_wrapper_files};${cuda_wrapper_bits_files};${cuda_wrapper_utility_files}")
@@ -644,6 +666,18 @@ install(
EXCLUDE_FROM_ALL
COMPONENT core-resource-headers)
+install(
+ FILES ${aix_wrapper_files}
+ DESTINATION ${header_install_dir}
+ EXCLUDE_FROM_ALL
+ COMPONENT aix-resource-headers)
+
+install(
+ FILES ${aix_sys_subdir_wrapper_files}
+ DESTINATION ${header_install_dir}/sys
+ EXCLUDE_FROM_ALL
+ COMPONENT aix-resource-headers)
+
install(
FILES ${arm_common_files} ${arm_common_generated_files}
DESTINATION ${header_install_dir}
@@ -837,6 +871,9 @@ if (NOT LLVM_ENABLE_IDE)
add_llvm_install_targets(install-core-resource-headers
DEPENDS core-resource-headers
COMPONENT core-resource-headers)
+ add_llvm_install_targets(install-aix-resource-headers
+ DEPENDS aix-resource-headers
+ COMPONENT aix-resource-headers)
add_llvm_install_targets(install-arm-common-resource-headers
DEPENDS arm-common-resource-headers
COMPONENT arm-common-resource-headers)
diff --git a/clang/lib/Headers/dbm.h b/clang/lib/Headers/dbm.h
new file mode 100644
index 0000000000000..8b7e2f51c5664
--- /dev/null
+++ b/clang/lib/Headers/dbm.h
@@ -0,0 +1,39 @@
+/*===---- dbm.h - BSD header for database management ----------------------===*\
+ *
+ * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+ * See https://llvm.org/LICENSE.txt for license information.
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ *
+\*===----------------------------------------------------------------------===*/
+
+// Although technically correct according to the standard, NULL defined as 0
+// may be problematic since it may result in a different size object than a
+// pointer (eg 64 bit mode on AIX). Therefore, re-define the macro to
+// ((void*)0) for consistency where needed.
+
+// Limit the effects to those platforms that have this as a system header.
+#if defined(_AIX)
+
+// Ensure that the definition of NULL (if present) is correct since it might
+// not be redefined if it is already defined. This ensures any use of NULL is
+// correct while processing the include_next.
+#define __need_NULL
+#include <stddef.h>
+
+#endif
+
+// Always include_next the file so that it will be included when requested.
+// This will trigger an error on platforms where it is not found unless
+// system include paths find it, which is the correct behaviour.
+
+#include_next <dbm.h>
+
+// Limit the effects to those platforms that have this as a system header.
+#if defined(_AIX) && defined(NULL)
+
+// Ensure that the definition of NULL (if present) is consistent with what
+// is expected, regardless of where it came from.
+#define __need_NULL
+#include <stddef.h>
+
+#endif
diff --git a/clang/lib/Headers/locale.h b/clang/lib/Headers/locale.h
new file mode 100644
index 0000000000000..6cb85738521cb
--- /dev/null
+++ b/clang/lib/Headers/locale.h
@@ -0,0 +1,25 @@
+/*===---- locale.h - Standard header for localization ---------------------===*\
+ *
+ * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+ * See https://llvm.org/LICENSE.txt for license information.
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ *
+\*===----------------------------------------------------------------------===*/
+
+// Although technically correct according to the standard, NULL defined as 0
+// may be problematic since it may result in a different size object than a
+// pointer (eg 64 bit mode on AIX). Therefore, re-define the macro to
+// ((void*)0) for consistency where needed.
+
+// The standard specifies that locale.h defines NULL so ensure that the
+// definition is correct since it might not be redefined if it is already
+// defined. This ensures any use of NULL is correct while processing the
+// include_next.
+#define __need_NULL
+#include <stddef.h>
+
+#include_next <locale.h>
+
+// Ensure that the definition of NULL is as expected (likely redundant).
+#define __need_NULL
+#include <stddef.h>
diff --git a/clang/lib/Headers/stdio.h b/clang/lib/Headers/stdio.h
new file mode 100644
index 0000000000000..db981f54ec193
--- /dev/null
+++ b/clang/lib/Headers/stdio.h
@@ -0,0 +1,25 @@
+/*===---- stdio.h - Standard header for input and output-------------------===*\
+ *
+ * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+ * See https://llvm.org/LICENSE.txt for license information.
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ *
+\*===----------------------------------------------------------------------===*/
+
+// Although technically correct according to the standard, NULL defined as 0
+// may be problematic since it may result in a different size object than a
+// pointer (eg 64 bit mode on AIX). Therefore, re-define the macro to
+// ((void*)0) for consistency where needed.
+
+// The standard specifies that stdio.h defines NULL so ensure that the
+// definition is correct since it might not be redefined if it is already
+// defined. This ensures any use of NULL is correct while processing the
+// include_next.
+#define __need_NULL
+#include <stddef.h>
+
+#include_next <stdio.h>
+
+// Ensure that the definition of NULL is as expected (likely redundant).
+#define __need_NULL
+#include <stddef.h>
diff --git a/clang/lib/Headers/stdlib.h b/clang/lib/Headers/stdlib.h
new file mode 100644
index 0000000000000..2bcf79e9622a5
--- /dev/null
+++ b/clang/lib/Headers/stdlib.h
@@ -0,0 +1,25 @@
+/*===---- stdlib.h - Standard header for general utilities ----------------===*\
+ *
+ * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+ * See https://llvm.org/LICENSE.txt for license information.
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ *
+\*===----------------------------------------------------------------------===*/
+
+// Although technically correct according to the standard, NULL defined as 0
+// may be problematic since it may result in a different size object than a
+// pointer (eg 64 bit mode on AIX). Therefore, re-define the macro to
+// ((void*)0) for consistency where needed.
+
+// The standard specifies that stdlib.h defines NULL so ensure that the
+// definition is correct since it might not be redefined if it is already
+// defined. This ensures any use of NULL is correct while processing the
+// include_next.
+#define __need_NULL
+#include <stddef.h>
+
+#include_next <stdlib.h>
+
+// Ensure that the definition of NULL is as expected (likely redundant).
+#define __need_NULL
+#include <stddef.h>
diff --git a/clang/lib/Headers/string.h b/clang/lib/Headers/string.h
new file mode 100644
index 0000000000000..a23d91e25743f
--- /dev/null
+++ b/clang/lib/Headers/string.h
@@ -0,0 +1,25 @@
+/*===---- string.h - Standard header for string handling ------------------===*\
+ *
+ * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+ * See https://llvm.org/LICENSE.txt for license information.
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ *
+\*===----------------------------------------------------------------------===*/
+
+// Although technically correct according to the standard, NULL defined as 0
+// may be problematic since it may result in a different size object than a
+// pointer (eg 64 bit mode on AIX). Therefore, re-define the macro to
+// ((void*)0) for consistency where needed.
+
+// The standard specifies that string.h defines NULL so ensure that the
+// definition is correct since it might not be redefined if it is already
+// defined. This ensures any use of NULL is correct while processing the
+// include_next.
+#define __need_NULL
+#include <stddef.h>
+
+#include_next <string.h>
+
+// Ensure that the definition of NULL is as expected (likely redundant).
+#define __need_NULL
+#include <stddef.h>
diff --git a/clang/lib/Headers/sys/dir.h b/clang/lib/Headers/sys/dir.h
new file mode 100644
index 0000000000000..8f7e2a40d1bd8
--- /dev/null
+++ b/clang/lib/Headers/sys/dir.h
@@ -0,0 +1,38 @@
+/*===---- sys/dir.h - BSD header for directory handling -------------------===*\
+ *
+ * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+ * See https://llvm.org/LICENSE.txt for license information.
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ *
+\*===----------------------------------------------------------------------===*/
+
+// Although technically correct according to the standard, NULL defined as 0
+// may be problematic since it may result in a different size object than a
+// pointer (eg 64 bit mode on AIX). Therefore, re-define the macro to
+// ((void*)0) for consistency where needed.
+
+// Limit the effects to those platforms that have this as a system header.
+#if defined(_AIX)
+
+// Ensure that the definition of NULL (if present) is correct since it might
+// not be redefined if it is already defined. This ensures any use of NULL is
+// correct while processing the include_next.
+#define __need_NULL
+#include <stddef.h>
+
+#endif
+
+// Always include_next the file so that it will be included when requested.
+// This will trigger an error on platforms where it is not found unless
+// system include paths find it, which is the correct behaviour.
+#include_next <sys/dir.h>
+
+// Limit the effects to those platforms that have this as a system header.
+#if defined(_AIX) && defined(NULL)
+
+// Ensure that the definition of NULL (if present) is consistent with what
+// is expected, regardless of where it came from.
+#define __need_NULL
+#include <stddef.h>
+
+#endif
diff --git a/clang/lib/Headers/sys/param.h b/clang/lib/Headers/sys/param.h
new file mode 100644
index 0000000000000..e1338854ad0b0
--- /dev/null
+++ b/clang/lib/Headers/sys/param.h
@@ -0,0 +1,38 @@
+/*===---- sys/param.h - BSD header ----------------------------------------===*\
+ *
+ * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+ * See https://llvm.org/LICENSE.txt for license information.
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ *
+\*===----------------------------------------------------------------------===*/
+
+// Although technically correct according to the standard, NULL defined as 0
+// may be problematic since it may result in a different size object than a
+// pointer (eg 64 bit mode on AIX). Therefore, re-define the macro to
+// ((void*)0) for consistency where needed.
+
+// Limit the effects to those platforms that have this as a system header.
+#if defined(_AIX)
+
+// Ensure that the definition of NULL (if present) is correct since it might
+// not be redefined if it is already defined. This ensures any use of NULL is
+// correct while processing the include_next.
+#define __need_NULL
+#include <stddef.h>
+
+#endif
+
+// Always include_next the file so that it will be included when requested.
+// This will trigger an error on platforms where it is not found unless
+// system include paths find it, which is the correct behaviour.
+#include_next <sys/param.h>
+
+// Limit the effects to those platforms that have this as a system header.
+#if defined(_AIX) && defined(NULL)
+
+// Ensure that the definition of NULL (if present) is consistent with what
+// is expected, regardless of where it came from.
+#define __need_NULL
+#include <stddef.h>
+
+#endif
diff --git a/clang/lib/Headers/sys/types.h b/clang/lib/Headers/sys/types.h
new file mode 100644
index 0000000000000..c4f6d75491d3a
--- /dev/null
+++ b/clang/lib/Headers/sys/types.h
@@ -0,0 +1,38 @@
+/*===---- sys/types.h - BSD header for types ------------------------------===*\
+ *
+ * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+ * See https://llvm.org/LICENSE.txt for license information.
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ *
+\*===----------------------------------------------------------------------===*/
+
+// Although technically correct according to the standard, NULL defined as 0
+// may be problematic since it may result in a different size object than a
+// pointer (eg 64 bit mode on AIX). Therefore, re-define the macro to
+// ((void*)0) for consistency where needed.
+
+// Limit the effects to those platforms that have this as a system header.
+#if defined(_AIX)
+
+// Ensure that the definition of NULL (if present) is correct since it might
+// not be redefined if it is already defined. This ensures any use of NULL is
+// correct while processing the include_next.
+#define __need_NULL
+#include <stddef.h>
+
+#endif
+
+// Always include_next the file so that it will be included when requested.
+// This will trigger an error on platforms where it is not found unless
+// system include paths find it, which is the correct behaviour.
+#include_next <sys/types.h>
+
+// Limit the effects to those platforms that have this as a system header.
+#if defined(_AIX) && defined(NULL)
+
+// Ensure that the definition of NULL (if present) is consistent with what
+// is expected, regardless of where it came from.
+#define __need_NULL
+#include <stddef.h>
+
+#endif
diff --git a/clang/lib/Headers/time.h b/clang/lib/Headers/time.h
new file mode 100644
index 0000000000000..3702d40caf8c5
--- /dev/null
+++ b/clang/lib/Headers/time.h
@@ -0,0 +1,25 @@
+/*===---- time.h - Standard header for date and time handling -------------===*\
+ *
+ * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+ * See https://llvm.org/LICENSE.txt for license information.
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ *
+\*===----------------------------------------------------------------------===*/
+
+// Although technically correct according to the standard, NULL defined as 0
+// may be problematic since it may result in a different size object than a
+// pointer (eg 64 bit mode on AIX). Therefore, re-define the macro to
+// ((void*)0) for consistency where needed.
+
+// The standard specifies that time.h defines NULL so ensure that the
+// definition is correct since it might not be redefined if it is already
+// defined. This ensures any use of NULL is correct while processing the
+// include_next.
+#define __need_NULL
+#include <stddef.h>
+
+#include_next <time.h>
+
+// Ensure that the definition of NULL is as expected (likely redundant).
+#define __need_NULL
+#include <stddef.h>
diff --git a/clang/lib/Headers/unistd.h b/clang/lib/Headers/unistd.h
new file mode 100644
index 0000000000000..05b8c936ba574
--- /dev/null
+++ b/clang/lib/Headers/unistd.h
@@ -0,0 +1,35 @@
+/*===---- unistd.h - Posix Standard header --------------------------------===*\
+ *
+ * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+ * See https://llvm.org/LICENSE.txt for license information.
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ *
+\*===----------------------------------------------------------------------===*/
+
+// Although technically correct according to the standard, NULL defined as 0
+// may be problematic since it may result in a different size object than a
+// pointer (eg 64 bit mode on AIX). Therefore, re-define the macro to
+// ((void*)0) for consistency where needed.
+
+// Limit the effects to those platforms that are POSIX compliant.
+#if defined(_AIX)
+
+// POSIX specifies that unistd.h defines NULL so ensure that the
+// definition is correct since it might not be redefined if it is already
+// defined. This ensures any use of NULL is correct while processing the
+// include_next.
+#define __need_NULL
+#include <stddef.h>
+
+#endif
+
+#include_next <unistd.h>
+
+// Limit the effects to those platforms that are POSIX compliant.
+#if defined(_AIX) && defined(NULL)
+
+// Ensure that the definition of NULL is as expected (likely redundant).
+#define __need_NULL
+#include <stddef.h>
+
+#endif
diff --git a/clang/lib/Headers/wchar.h b/clang/lib/Headers/wchar.h
new file mode 100644
index 0000000000000..ae7a1e779f5b0
--- /dev/null
+++ b/clang/lib/Headers/wchar.h
@@ -0,0 +1,25 @@
+/*===---- wchar.h - Standard header for string handling ------------------===*\
+ *
+ * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+ * See https://llvm.org/LICENSE.txt for license information.
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ *
+\*===----------------------------------------------------------------------===*/
+
+// Although technically correct according to the standard, NULL defined as 0
+// may be problematic since it may result in a different size object than a
+// pointer (eg 64 bit mode on AIX). Therefore, re-define the macro to
+// ((void*)0) for consistency where needed.
+
+// The standard specifies that wchar.h defines NULL so ensure that the
+// definition is correct since it might not be redefined if it is already
+// defined. This ensures any use of NULL is correct while processing the
+// include_next.
+#define __need_NULL
+#include <stddef.h>
+
+#include_next <wchar.h>
+
+// Ensure that the definition of NULL is as expected (likely redundant).
+#define __need_NULL
+#include <stddef.h>
diff --git a/clang/test/Headers/check-NULL-aix.c b/clang/test/Headers/check-NULL-aix.c
new file mode 100644
index 0000000000000..01bef5efd4ecd
--- /dev/null
+++ b/clang/test/Headers/check-NULL-aix.c
@@ -0,0 +1,16 @@
+// There are at least 2 valid C null-pointer constants as defined
+// by the C language standard.
+// Test that the macro NULL is defined consistently by those system headers
+// on AIX that have a macro definition for NULL.
+
+// REQUIRES: system-aix
+
+// RUN: %clang %s -Dheader="<dbm.h>" -E | tail -1 | FileCheck %s
+// RUN: %clang %s -Dheader="<sys/dir.h>" -E | tail -1 | FileCheck %s
+// RUN: %clang %s -Dheader="<sys/param.h>" -E | tail -1 | FileCheck %s
+// RUN: %clang %s -Dheader="<sys/types.h>" -E | tail -1 | FileCheck %s
+// RUN: %clang %s -Dheader="<unistd.h>" -E | tail -1 | FileCheck %s
+
+#include header
+void *p = NULL;
+// CHECK: ({{ *}}({{ *}}void{{ *}}*{{ *}}){{ *}}0{{ *}})
diff --git a/clang/test/Headers/check-NULL.c b/clang/test/Headers/check-NULL.c
new file mode 100644
index 0000000000000..10d40b941b648
--- /dev/null
+++ b/clang/test/Headers/check-NULL.c
@@ -0,0 +1,15 @@
+// There are at least 2 valid C null-pointer constants as defined
+// by the C language standard.
+// Test that the macro NULL is defined consistently for all platforms by
+// those headers that the C standard mandates a macro definition for NULL.
+
+// RUN: %clang %s -Dheader="<locale.h>" -E | tail -1 | FileCheck %s
+// RUN: %clang %s -Dheader="<stdio.h>" -E | tail -1 | FileCheck %s
+// RUN: %clang %s -Dheader="<stdlib.h>" -E | tail -1 | FileCheck %s
+// RUN: %clang %s -Dheader="<string.h>" -E | tail -1 | FileCheck %s
+// R...
[truncated]
|
|
|
|
This seems like a very complicated solution. If the AIX system headers want to define NULL, should we just let them? Just |
|
@AaronBallman, this is the patch I mentioned in today's Clang C/C++ Language Workgroup. |
If Also, the PR as is currently slightly off and fails to express the correct scope: In other words, part of the motivation of this PR is to address a general issue where Clang fails to cover the set of headers that it ought to. |
jamieschmeiser
force-pushed
the
users/jamieschmeiser/ensure_NULL_macro
branch
from
de50580 to
e50062b
Compare
Just doing an The standard mandates that certain headers define NULL but what if the |
There was a problem hiding this comment.
I guess... if you think this is what's best for AIX, I'm not going to argue very hard about it, but I don't really want to entangle every other operating system in this. The current situation is mostly stable.
What logic did you use to decide which headers should use #idfdef _AIX?
clang/test/Headers/check-NULL.c
Outdated
There was a problem hiding this comment.
This is assuming that the default search path for clang will find system headers. This isn't really a safe assumption if the default triple is a cross-compile triple.
There was a problem hiding this comment.
How can that happen? According to https://clang.llvm.org/docs/CrossCompilation.html: If you don’t specify the target, CPU names won’t match (since Clang assumes the host triple), and the compilation will go ahead, creating code for the host platform. There is no target specified so it will not be cross-compiling.
Is there something that you would suggest to alleviate your concerns?
There was a problem hiding this comment.
There is no target specified so it will not be cross-compiling.
It's entirely possible to create a cross-compiling build where the architecture is the same but the OS is different.
Using
%clang_cc1 -x c++ -internal-isystem %S/Inputs/include
instead of %clang seems like part of the solution.
There was a problem hiding this comment.
The statement "Clang assumes the host triple" is an approximation. clang's default target is specified by the LLVM_DEFAULT_TARGET_TRIPLE CMake flag. By default, that's the host triple, but you can override it.
If you need some specific set of "libc" headers, you can mock them in clang/test/Headers/Inputs/ .
@efriedma-quic, are you saying that you have a strong preference to not handle |
The headers that exist on AIX that define NULL but are not mandated by the C standard to define NULL are guarded with
|
Yes. You know exactly which AIX headers define NULL, and because IBM also ships the AIX headers, you can ensure no future version of the headers changes the status quo. But there are a lot of different libc variants, and I don't want to try to chase after every variant, and figure out exactly which headers define NULL, or how we accidentally broke something. Messing with core libc headers is fundamentally risky. Most libcs don't make the mistake of defining null to plain "0" on 64-bit systems, anyway. One more issue: do we need to update module.modulemap to mark these headers as "textual"? |
I had proposed this universally as a "public service" due to it being mandated by the C standard. Since you are opposed, I will alter the files involved to be guarded as Aix specific. Regarding module.modulemap, since this will now be Aix specific, we will deal with that aspect at some future point. |
The C standard allows for at least 2 valid definitions of a null pointer constant and mandates that several standard headers files define the macro NULL to be a null pointer constant. Ensure that definitions of NULL are consistent across the various C header files. respond to review: add in locale.h respond to review: make all changes AIX specific
|
Unless you're planning to hide the headers from other targets (like clang/lib/Headers/zos_wrappers/), the headers will exist on every target, so you need to deal with module.modulemap. |
jamieschmeiser
force-pushed
the
users/jamieschmeiser/ensure_NULL_macro
branch
from
e50062b to
7f3dccd
Compare
I think it makes sense for Clang to handle the C Standard Library headers which can introduce |
| #define __need_NULL | ||
| #include <stddef.h> | ||
|
|
||
| #include_next <dbm.h> | ||
|
|
||
| /* Ensure that the definition of NULL is as expected. */ | ||
| #define __need_NULL | ||
| #include <stddef.h> |
There was a problem hiding this comment.
Why is the pattern to define NULL, include_next the header the user asked for, and then define NULL again? Are the include_next headers defining NULL to something different? And if so, what are we potentially breaking by redefining it out from under them?
These other headers define NULL to 0 on AIX and are included through the system include paths. Note that they are guarded to only deal with NULL on AIX and just include_next on other platforms.
(The following relates to AIX, I do not know what happens on other platforms...) Yes, some of the include_next headers are defining NULL to 0, hence the pattern. The definitions are typically guarded such that NULL is not defined if it is already defined. Therefore, it is necessary to force the expected macro before the include_next to ensure that a potentially existing definition is not retained nor a definition of NULL to 0 introduced. The include_next file will now not define the macro since it is already defined but there are ways that the processing of the included files could be unexpected (eg, a header could be replaced) so NULL is forced to the expected definition again after the include_next is processed. Yes, this pattern can potentially break code if the user has defined NULL to something unexpected before including the header but unfortunately, NULL is not a special identifier. The proposed pattern attempts to ensure that the definition is at least consistent within the expected processing of system header files (and after) for those headers that define NULL. There really isn't much more the compiler can do to maintain consistency of the macro and the pattern should provide the correct macro definitions for the overwhelming majority of users. |
So basically those headers are broken on AIX and this is working around them being broken?
Thank you for the explanation. My concerns with this pattern are:
|
|
All, |
The C standard allows for at least 2 valid definitions of a null pointer constant and mandates that several standard headers files define the macro NULL to be a null pointer constant. Ensure that definitions of NULL are consistent across the various C header files.