Skip to content

Conversation

frobtech
Copy link
Contributor

@frobtech frobtech commented Sep 5, 2025

A namespace like LIBC_NAMESPACE::internal should only ever be
defined if it's providing global symbols declared in headers.
These StringUtil implementations were defining global namespaced
symbols for their file-local helper code, which they should not.

A namespace like LIBC_NAMESPACE::internal should only ever be
defined if it's providing global symbols declared in headers.
These StringUtil implementations were defining global namespaced
symbols for their file-local helper code, which they should not.
@frobtech frobtech marked this pull request as ready for review September 5, 2025 23:44
@llvmbot llvmbot added the libc label Sep 5, 2025
@llvmbot
Copy link
Member

llvmbot commented Sep 5, 2025

@llvm/pr-subscribers-libc

Author: Roland McGrath (frobtech)

Changes

A namespace like LIBC_NAMESPACE::internal should only ever be
defined if it's providing global symbols declared in headers.
These StringUtil implementations were defining global namespaced
symbols for their file-local helper code, which they should not.


Full diff: https://github.com/llvm/llvm-project/pull/157202.diff

2 Files Affected:

  • (modified) libc/src/__support/StringUtil/error_to_string.cpp (+9-10)
  • (modified) libc/src/__support/StringUtil/signal_to_string.cpp (+9-10)
diff --git a/libc/src/__support/StringUtil/error_to_string.cpp b/libc/src/__support/StringUtil/error_to_string.cpp
index 3b22021706bba..38916af83795d 100644
--- a/libc/src/__support/StringUtil/error_to_string.cpp
+++ b/libc/src/__support/StringUtil/error_to_string.cpp
@@ -7,8 +7,10 @@
 //===----------------------------------------------------------------------===//
 
 #include "error_to_string.h"
-#include "platform_errors.h"
 
+#include <stddef.h>
+
+#include "platform_errors.h"
 #include "src/__support/CPP/span.h"
 #include "src/__support/CPP/string_view.h"
 #include "src/__support/CPP/stringstream.h"
@@ -17,10 +19,8 @@
 #include "src/__support/macros/attributes.h"
 #include "src/__support/macros/config.h"
 
-#include <stddef.h>
-
 namespace LIBC_NAMESPACE_DECL {
-namespace internal {
+namespace {
 
 constexpr size_t max_buff_size() {
   constexpr size_t unknown_str_len = sizeof("Unknown error");
@@ -63,23 +63,22 @@ cpp::string_view build_error_string(int err_num, cpp::span<char> buffer) {
   return buffer_stream.str();
 }
 
-} // namespace internal
+} // namespace
 
 cpp::string_view get_error_string(int err_num) {
-  return get_error_string(err_num,
-                          {internal::error_buffer, internal::ERR_BUFFER_SIZE});
+  return get_error_string(err_num, {error_buffer, ERR_BUFFER_SIZE});
 }
 
 cpp::string_view get_error_string(int err_num, cpp::span<char> buffer) {
-  auto opt_str = internal::ERROR_MAPPER.get_str(err_num);
+  auto opt_str = ERROR_MAPPER.get_str(err_num);
   if (opt_str)
     return *opt_str;
   else
-    return internal::build_error_string(err_num, buffer);
+    return build_error_string(err_num, buffer);
 }
 
 cpp::optional<cpp::string_view> try_get_errno_name(int err_num) {
-  return internal::ERRNO_NAME_MAPPER.get_str(err_num);
+  return ERRNO_NAME_MAPPER.get_str(err_num);
 }
 
 } // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/__support/StringUtil/signal_to_string.cpp b/libc/src/__support/StringUtil/signal_to_string.cpp
index b67d28814816c..e5863ff1e8987 100644
--- a/libc/src/__support/StringUtil/signal_to_string.cpp
+++ b/libc/src/__support/StringUtil/signal_to_string.cpp
@@ -7,8 +7,11 @@
 //===----------------------------------------------------------------------===//
 
 #include "signal_to_string.h"
-#include "platform_signals.h"
 
+#include <signal.h>
+#include <stddef.h>
+
+#include "platform_signals.h"
 #include "src/__support/CPP/span.h"
 #include "src/__support/CPP/string_view.h"
 #include "src/__support/CPP/stringstream.h"
@@ -17,11 +20,8 @@
 #include "src/__support/macros/attributes.h"
 #include "src/__support/macros/config.h"
 
-#include <signal.h>
-#include <stddef.h>
-
 namespace LIBC_NAMESPACE_DECL {
-namespace internal {
+namespace {
 
 constexpr size_t max_buff_size() {
   constexpr size_t base_str_len = sizeof("Real-time signal");
@@ -63,19 +63,18 @@ cpp::string_view build_signal_string(int sig_num, cpp::span<char> buffer) {
   return buffer_stream.str();
 }
 
-} // namespace internal
+} // namespace
 
 cpp::string_view get_signal_string(int sig_num) {
-  return get_signal_string(
-      sig_num, {internal::signal_buffer, internal::SIG_BUFFER_SIZE});
+  return get_signal_string(sig_num, {signal_buffer, SIG_BUFFER_SIZE});
 }
 
 cpp::string_view get_signal_string(int sig_num, cpp::span<char> buffer) {
-  auto opt_str = internal::signal_mapper.get_str(sig_num);
+  auto opt_str = signal_mapper.get_str(sig_num);
   if (opt_str)
     return *opt_str;
   else
-    return internal::build_signal_string(sig_num, buffer);
+    return build_signal_string(sig_num, buffer);
 }
 
 } // namespace LIBC_NAMESPACE_DECL

@frobtech frobtech merged commit 4d0b816 into llvm:main Sep 6, 2025
23 checks passed
@frobtech frobtech deleted the p/libc-stringutil-decl branch September 6, 2025 00:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants