[libc] Make 'printf' converter output "(null)" instead of "null" #85845
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-libc Author: Joseph Huber (jhuber6) ChangesSummary: Full diff: https://github.com/llvm/llvm-project/pull/85845.diff 2 Files Affected:
diff --git a/libc/src/stdio/printf_core/string_converter.h b/libc/src/stdio/printf_core/string_converter.h
index 04dc5a06da2226..9e05591079faa4 100644
--- a/libc/src/stdio/printf_core/string_converter.h
+++ b/libc/src/stdio/printf_core/string_converter.h
@@ -25,7 +25,7 @@ LIBC_INLINE int convert_string(Writer *writer, const FormatSection &to_conv) {
#ifndef LIBC_COPT_PRINTF_NO_NULLPTR_CHECKS
if (str_ptr == nullptr) {
- str_ptr = "null";
+ str_ptr = "(null)";
}
#endif // LIBC_COPT_PRINTF_NO_NULLPTR_CHECKS
diff --git a/libc/test/src/stdio/sprintf_test.cpp b/libc/test/src/stdio/sprintf_test.cpp
index ab8bdb23b1187c..8dde95d02a96dc 100644
--- a/libc/test/src/stdio/sprintf_test.cpp
+++ b/libc/test/src/stdio/sprintf_test.cpp
@@ -121,8 +121,8 @@ TEST(LlvmLibcSPrintfTest, StringConv) {
#ifndef LIBC_COPT_PRINTF_NO_NULLPTR_CHECKS
written = LIBC_NAMESPACE::sprintf(buff, "%s", nullptr);
- EXPECT_EQ(written, 4);
- ASSERT_STREQ(buff, "null");
+ EXPECT_EQ(written, 6);
+ ASSERT_STREQ(buff, "(null)");
#endif // LIBC_COPT_PRINTF_NO_NULLPTR_CHECKS
}
|
|
Please add design decisions to the undefined behavior documentation. https://github.com/llvm/llvm-project/blob/main/libc/docs/dev/undefined_behavior.rst |
Summary: Currently we print `null` for the null pointer in a `%s` expression. Although it's not defined by the standard, other implementations choose to use `(null)` to indicate this. We also currently print `(nullptr)` so I think it's more consistent to use parens in both cases.
|
This patch LGTM, thank you. |
chencha3
pushed a commit
to chencha3/llvm-project
that referenced
this pull request
Mar 23, 2024
…m#85845) Summary: Currently we print `null` for the null pointer in a `%s` expression. Although it's not defined by the standard, other implementations choose to use `(null)` to indicate this. We also currently print `(nullptr)` so I think it's more consistent to use parens in both cases.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
Currently we print
nullfor the null pointer in a%sexpression.Although it's not defined by the standard, other implementations choose
to use
(null)to indicate this. We also currently print(nullptr)soI think it's more consistent to use parens in both cases.