Skip to content

Conversation

@mleleszi
Copy link
Contributor

@mleleszi mleleszi commented Nov 7, 2025

Now that #166517 has landed and Writer has been refactored to track bytes written as size_t, strftime can be refactored as well to handle size_t return values.

Can't think of a proper way to test this without creating a 2GB+ string, but existing tests cover most cases.

@mleleszi mleleszi marked this pull request as ready for review November 7, 2025 07:06
@llvmbot llvmbot added the libc label Nov 7, 2025
@mleleszi
Copy link
Contributor Author

mleleszi commented Nov 7, 2025

@michaelrj-google @lntue

@llvmbot
Copy link
Member

llvmbot commented Nov 7, 2025

@llvm/pr-subscribers-libc

Author: Marcell Leleszi (mleleszi)

Changes

Now that #166517 has landed and Writer has been refactored to track bytes written as size_t, strftime can be refactored as well to handle size_t return values.

Can't think of a proper way to test this without creating a 2GB+ string, but existing tests cover most cases.


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

4 Files Affected:

  • (modified) libc/src/time/strftime.cpp (+2-2)
  • (modified) libc/src/time/strftime_core/CMakeLists.txt (+1)
  • (modified) libc/src/time/strftime_core/strftime_main.h (+5-5)
  • (modified) libc/src/time/strftime_l.cpp (+2-2)
diff --git a/libc/src/time/strftime.cpp b/libc/src/time/strftime.cpp
index 89b7d9bb7c1b9..ff8c05a0b07da 100644
--- a/libc/src/time/strftime.cpp
+++ b/libc/src/time/strftime.cpp
@@ -23,10 +23,10 @@ LLVM_LIBC_FUNCTION(size_t, strftime,
       printf_core::WriteMode::FILL_BUFF_AND_DROP_OVERFLOW>::value>
       wb(buffer, (buffsz > 0 ? buffsz - 1 : 0));
   printf_core::Writer writer(wb);
-  int ret = strftime_core::strftime_main(&writer, format, timeptr);
+  auto ret = strftime_core::strftime_main(&writer, format, timeptr);
   if (buffsz > 0) // if the buffsz is 0 the buffer may be a null pointer.
     wb.buff[wb.buff_cur] = '\0';
-  return (ret < 0 || static_cast<size_t>(ret) >= buffsz) ? 0 : ret;
+  return (!ret.has_value() || ret.value() >= buffsz) ? 0 : ret.value();
 }
 
 } // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/time/strftime_core/CMakeLists.txt b/libc/src/time/strftime_core/CMakeLists.txt
index 3ffd283ead7fe..a9aa573cc9a63 100644
--- a/libc/src/time/strftime_core/CMakeLists.txt
+++ b/libc/src/time/strftime_core/CMakeLists.txt
@@ -43,6 +43,7 @@ add_header_library(
     .core_structs
     .parser
     .converter
+    libc.src.__support.error_or
     libc.src.stdio.printf_core.writer
     libc.hdr.types.struct_tm
 )
diff --git a/libc/src/time/strftime_core/strftime_main.h b/libc/src/time/strftime_core/strftime_main.h
index 2b136d83234cd..855a44107914c 100644
--- a/libc/src/time/strftime_core/strftime_main.h
+++ b/libc/src/time/strftime_core/strftime_main.h
@@ -10,6 +10,7 @@
 #define LLVM_LIBC_SRC_STDIO_STRFTIME_CORE_STRFTIME_MAIN_H
 
 #include "hdr/types/struct_tm.h"
+#include "src/__support/error_or.h"
 #include "src/__support/macros/config.h"
 #include "src/stdio/printf_core/writer.h"
 #include "src/time/strftime_core/converter.h"
@@ -20,8 +21,8 @@ namespace LIBC_NAMESPACE_DECL {
 namespace strftime_core {
 
 template <printf_core::WriteMode write_mode>
-int strftime_main(printf_core::Writer<write_mode> *writer,
-                  const char *__restrict str, const tm *timeptr) {
+ErrorOr<size_t> strftime_main(printf_core::Writer<write_mode> *writer,
+                              const char *__restrict str, const tm *timeptr) {
   Parser parser(str);
   int result = 0;
   for (strftime_core::FormatSection cur_section = parser.get_next_section();
@@ -33,11 +34,10 @@ int strftime_main(printf_core::Writer<write_mode> *writer,
       result = writer->write(cur_section.raw_string);
 
     if (result < 0)
-      return result;
+      return Error(-result);
   }
 
-  // TODO: Use ErrorOr<size_t>
-  return static_cast<int>(writer->get_chars_written());
+  return writer->get_chars_written();
 }
 
 } // namespace strftime_core
diff --git a/libc/src/time/strftime_l.cpp b/libc/src/time/strftime_l.cpp
index 409f8683b7289..2ec90634ea347 100644
--- a/libc/src/time/strftime_l.cpp
+++ b/libc/src/time/strftime_l.cpp
@@ -26,10 +26,10 @@ LLVM_LIBC_FUNCTION(size_t, strftime_l,
       printf_core::WriteMode::FILL_BUFF_AND_DROP_OVERFLOW>::value>
       wb(buffer, (buffsz > 0 ? buffsz - 1 : 0));
   printf_core::Writer writer(wb);
-  int ret = strftime_core::strftime_main(&writer, format, timeptr);
+  auto ret = strftime_core::strftime_main(&writer, format, timeptr);
   if (buffsz > 0) // if the buffsz is 0 the buffer may be a null pointer.
     wb.buff[wb.buff_cur] = '\0';
-  return (ret < 0 || static_cast<size_t>(ret) >= buffsz) ? 0 : ret;
+  return (!ret.has_value() || ret.value() >= buffsz) ? 0 : ret.value();
 }
 
 } // namespace LIBC_NAMESPACE_DECL

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.

2 participants