[libc-commits] [libc] [libc] Refactor strftime internals to handle size_t return values (PR #166901)

[via libc-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libc

Author: Marcell Leleszi (mleleszi)

<details>
<summary>Changes</summary>

Now that https://github.com/llvm/llvm-project/pull/166517 has landed and [Writer](https://github.com/llvm/llvm-project/blob/main/libc/src/stdio/printf_core/writer.h#L130) 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
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

``````````

</details>


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