[libc-commits] [libc] b9f6c20 - [libc] Move printf writer to new design

Michael Jones via libc-commits libc-commits at lists.llvm.org
Thu Jul 20 11:08:26 PDT 2023


Author: Michael Jones
Date: 2023-07-20T11:08:20-07:00
New Revision: b9f6c20876845abbe2b816d0d800b99412b5eab9

URL: https://github.com/llvm/llvm-project/commit/b9f6c20876845abbe2b816d0d800b99412b5eab9
DIFF: https://github.com/llvm/llvm-project/commit/b9f6c20876845abbe2b816d0d800b99412b5eab9.diff

LOG: [libc] Move printf writer to new design

The new printf writer design focuses on optimizing the fast path. It
inlines any write to a buffer or string, and by handling buffering
itself can more effectively work with both internal and external file
implementations. The overflow hook should allow for expansion to
asprintf with minimal extra code.

Reviewed By: sivachandra

Differential Revision: https://reviews.llvm.org/D153999

Added: 
    libc/test/src/stdio/printf_core/writer_test.cpp

Modified: 
    libc/src/stdio/CMakeLists.txt
    libc/src/stdio/fprintf.cpp
    libc/src/stdio/printf.cpp
    libc/src/stdio/printf_core/CMakeLists.txt
    libc/src/stdio/printf_core/vfprintf_internal.h
    libc/src/stdio/printf_core/writer.cpp
    libc/src/stdio/printf_core/writer.h
    libc/src/stdio/snprintf.cpp
    libc/src/stdio/sprintf.cpp
    libc/test/src/stdio/CMakeLists.txt
    libc/test/src/stdio/printf_core/CMakeLists.txt
    libc/test/src/stdio/printf_core/converter_test.cpp
    libc/test/src/stdio/snprintf_test.cpp
    utils/bazel/llvm-project-overlay/libc/BUILD.bazel
    utils/bazel/llvm-project-overlay/libc/test/src/stdio/BUILD.bazel

Removed: 
    libc/src/stdio/printf_core/file_writer.h
    libc/src/stdio/printf_core/string_writer.cpp
    libc/src/stdio/printf_core/string_writer.h
    libc/test/src/stdio/printf_core/string_writer_test.cpp


################################################################################
diff  --git a/libc/src/stdio/CMakeLists.txt b/libc/src/stdio/CMakeLists.txt
index 4747b5daee9e6b..df0933fcce510b 100644
--- a/libc/src/stdio/CMakeLists.txt
+++ b/libc/src/stdio/CMakeLists.txt
@@ -490,7 +490,6 @@ add_entrypoint_object(
     sprintf.h
   DEPENDS
     libc.src.stdio.printf_core.printf_main
-    libc.src.stdio.printf_core.string_writer
     libc.src.stdio.printf_core.writer
 )
 
@@ -502,18 +501,17 @@ add_entrypoint_object(
     snprintf.h
   DEPENDS
     libc.src.stdio.printf_core.printf_main
-    libc.src.stdio.printf_core.string_writer
     libc.src.stdio.printf_core.writer
 )
 
-list(APPEND printf_deps 
-      libc.src.__support.arg_list 
+list(APPEND printf_deps
+      libc.src.__support.arg_list
       libc.src.stdio.printf_core.vfprintf_internal
 )
 if(LLVM_LIBC_FULL_BUILD)
- list(APPEND printf_deps  
-      libc.src.__support.File.file 
-      libc.src.__support.File.platform_file 
+ list(APPEND printf_deps
+      libc.src.__support.File.file
+      libc.src.__support.File.platform_file
   )
 else()
  set(printf_copts "-DLIBC_COPT_PRINTF_USE_SYSTEM_FILE")

diff  --git a/libc/src/stdio/fprintf.cpp b/libc/src/stdio/fprintf.cpp
index 900e18f72167fd..e13ed4d075ac85 100644
--- a/libc/src/stdio/fprintf.cpp
+++ b/libc/src/stdio/fprintf.cpp
@@ -8,6 +8,7 @@
 
 #include "src/stdio/fprintf.h"
 
+#include "src/__support/File/file.h"
 #include "src/__support/arg_list.h"
 #include "src/stdio/printf_core/vfprintf_internal.h"
 
@@ -16,13 +17,6 @@
 
 namespace __llvm_libc {
 
-#ifndef LIBC_COPT_PRINTF_USE_SYSTEM_FILE
-#include "src/__support/File/file.h"
-using FileT = __llvm_libc::File;
-#else  // defined(LIBC_COPT_PRINTF_USE_SYSTEM_FILE)
-using FileT = ::FILE;
-#endif // LIBC_COPT_PRINTF_USE_SYSTEM_FILE
-
 LLVM_LIBC_FUNCTION(int, fprintf,
                    (::FILE *__restrict stream, const char *__restrict format,
                     ...)) {
@@ -32,8 +26,7 @@ LLVM_LIBC_FUNCTION(int, fprintf,
                                  // and pointer semantics, as well as handling
                                  // destruction automatically.
   va_end(vlist);
-  int ret_val = printf_core::vfprintf_internal(
-      reinterpret_cast<FileT *>(stream), format, args);
+  int ret_val = printf_core::vfprintf_internal(stream, format, args);
   return ret_val;
 }
 

diff  --git a/libc/src/stdio/printf.cpp b/libc/src/stdio/printf.cpp
index ca6f61ed630331..3aaab697793497 100644
--- a/libc/src/stdio/printf.cpp
+++ b/libc/src/stdio/printf.cpp
@@ -8,6 +8,7 @@
 
 #include "src/stdio/printf.h"
 
+#include "src/__support/File/file.h"
 #include "src/__support/arg_list.h"
 #include "src/stdio/printf_core/vfprintf_internal.h"
 
@@ -15,7 +16,6 @@
 #include <stdio.h>
 
 #ifndef LIBC_COPT_PRINTF_USE_SYSTEM_FILE
-#include "src/__support/File/file.h"
 #define PRINTF_STDOUT __llvm_libc::stdout
 #else // LIBC_COPT_PRINTF_USE_SYSTEM_FILE
 #define PRINTF_STDOUT ::stdout
@@ -30,7 +30,8 @@ LLVM_LIBC_FUNCTION(int, printf, (const char *__restrict format, ...)) {
                                  // and pointer semantics, as well as handling
                                  // destruction automatically.
   va_end(vlist);
-  int ret_val = printf_core::vfprintf_internal(PRINTF_STDOUT, format, args);
+  int ret_val = printf_core::vfprintf_internal(
+      reinterpret_cast<::FILE *>(PRINTF_STDOUT), format, args);
   return ret_val;
 }
 

diff  --git a/libc/src/stdio/printf_core/CMakeLists.txt b/libc/src/stdio/printf_core/CMakeLists.txt
index e41ef20a733b6a..a2227fbf77c7ac 100644
--- a/libc/src/stdio/printf_core/CMakeLists.txt
+++ b/libc/src/stdio/printf_core/CMakeLists.txt
@@ -46,19 +46,6 @@ add_object_library(
     -DLIBC_COPT_MOCK_ARG_LIST
 )
 
-add_object_library(
-  string_writer
-  SRCS
-    string_writer.cpp
-  HDRS
-    string_writer.h
-  DEPENDS
-    .core_structs
-    libc.src.__support.CPP.string_view
-    libc.src.string.memory_utils.inline_memcpy
-    libc.src.string.memory_utils.inline_memset
-)
-
 add_object_library(
   writer
   SRCS
@@ -67,6 +54,10 @@ add_object_library(
     writer.h
   DEPENDS
     libc.src.__support.CPP.string_view
+    libc.src.__support.macros.optimization
+    libc.src.string.memory_utils.inline_memcpy
+    libc.src.string.memory_utils.inline_memset
+    .core_structs
 )
 
 add_object_library(
@@ -125,18 +116,6 @@ if(NOT (TARGET libc.src.__support.File.file) AND LLVM_LIBC_FULL_BUILD)
   return()
 endif()
 
-add_header_library(
-  file_writer
-  HDRS
-    file_writer.h
-  DEPENDS
-    .core_structs
-    libc.include.stdio
-    libc.src.__support.CPP.string_view
-    libc.src.__support.File.file
-    libc.src.string.memory_utils.inline_memset
-)
-
 add_header_library(
   vfprintf_internal
   HDRS
@@ -146,6 +125,5 @@ add_header_library(
     libc.src.__support.File.file
     libc.src.__support.arg_list
     libc.src.stdio.printf_core.printf_main
-    libc.src.stdio.printf_core.file_writer
     libc.src.stdio.printf_core.writer
 )

diff  --git a/libc/src/stdio/printf_core/file_writer.h b/libc/src/stdio/printf_core/file_writer.h
deleted file mode 100644
index 0fd6d115ddd8b8..00000000000000
--- a/libc/src/stdio/printf_core/file_writer.h
+++ /dev/null
@@ -1,101 +0,0 @@
-//===-- FILE Writer definition for printf -----------------------*- C++ -*-===//
-//
-// 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
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLVM_LIBC_SRC_STDIO_PRINTF_CORE_FILE_WRITER_H
-#define LLVM_LIBC_SRC_STDIO_PRINTF_CORE_FILE_WRITER_H
-
-#include "src/__support/CPP/string_view.h"
-#include "src/__support/File/file.h"
-#include "src/__support/macros/attributes.h" // For LIBC_INLINE
-#include "src/stdio/printf_core/core_structs.h"
-
-#include <stddef.h>
-#include <stdio.h>
-
-namespace __llvm_libc {
-namespace printf_core {
-
-template <typename file_t> class FileWriter {
-  file_t *file;
-
-public:
-  LIBC_INLINE FileWriter(file_t *init_file);
-
-  LIBC_INLINE ~FileWriter();
-
-  LIBC_INLINE int write(const char *__restrict to_write, size_t len);
-
-  // These write functions take a FileWriter as a void* in raw_pointer, and
-  // call the appropriate write function on it.
-  static int write_str(void *raw_pointer, cpp::string_view new_string) {
-    FileWriter *file_writer = reinterpret_cast<FileWriter *>(raw_pointer);
-    return file_writer->write(new_string.data(), new_string.size());
-  }
-  static int write_chars(void *raw_pointer, char new_char, size_t len) {
-    FileWriter *file_writer = reinterpret_cast<FileWriter *>(raw_pointer);
-    constexpr size_t BUFF_SIZE = 8;
-    char buff[BUFF_SIZE] = {new_char};
-    int result;
-    while (len > BUFF_SIZE) {
-      result = file_writer->write(buff, BUFF_SIZE);
-      if (result < 0)
-        return result;
-      len -= BUFF_SIZE;
-    }
-    return file_writer->write(buff, len);
-  }
-  static int write_char(void *raw_pointer, char new_char) {
-    FileWriter *file_writer = reinterpret_cast<FileWriter *>(raw_pointer);
-    return file_writer->write(&new_char, 1);
-  }
-};
-
-// The interface for using our internal file implementation.
-template <>
-LIBC_INLINE
-FileWriter<__llvm_libc::File>::FileWriter(__llvm_libc::File *init_file) {
-  file = init_file;
-  file->lock();
-}
-template <> LIBC_INLINE FileWriter<__llvm_libc::File>::~FileWriter() {
-  file->unlock();
-}
-template <>
-LIBC_INLINE int
-FileWriter<__llvm_libc::File>::write(const char *__restrict to_write,
-                                     size_t len) {
-  auto result = file->write_unlocked(to_write, len);
-  size_t written = result.value;
-  if (written != len || result.has_error())
-    written = FILE_WRITE_ERROR;
-  if (file->error_unlocked())
-    written = FILE_STATUS_ERROR;
-  return written;
-}
-
-// The interface for using the system's file implementation.
-template <> LIBC_INLINE FileWriter<::FILE>::FileWriter(::FILE *init_file) {
-  file = init_file;
-  ::flockfile(file);
-}
-template <> LIBC_INLINE FileWriter<::FILE>::~FileWriter() {
-  ::funlockfile(file);
-}
-template <>
-LIBC_INLINE int FileWriter<::FILE>::write(const char *__restrict to_write,
-                                          size_t len) {
-  size_t written = ::fwrite_unlocked(to_write, 1, len, file);
-  if (written != len || ::ferror_unlocked(file))
-    written = FILE_WRITE_ERROR;
-  return written;
-}
-
-} // namespace printf_core
-} // namespace __llvm_libc
-
-#endif // LLVM_LIBC_SRC_STDIO_PRINTF_CORE_FILE_WRITER_H

diff  --git a/libc/src/stdio/printf_core/string_writer.cpp b/libc/src/stdio/printf_core/string_writer.cpp
deleted file mode 100644
index f89e9d11b00eab..00000000000000
--- a/libc/src/stdio/printf_core/string_writer.cpp
+++ /dev/null
@@ -1,70 +0,0 @@
-//===-- String Writer implementation for printf -----------------*- C++ -*-===//
-//
-// 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
-//
-//===----------------------------------------------------------------------===//
-
-#include "src/stdio/printf_core/string_writer.h"
-#include "src/__support/CPP/string_view.h"
-#include "src/stdio/printf_core/core_structs.h"
-#include "src/string/memory_utils/inline_memcpy.h"
-#include "src/string/memory_utils/inline_memset.h"
-#include <stddef.h>
-
-namespace __llvm_libc {
-namespace printf_core {
-
-void StringWriter::write(cpp::string_view new_string) {
-  size_t len = new_string.size();
-  if (len > available_capacity)
-    len = available_capacity;
-
-  if (len > 0) {
-    inline_memcpy(cur_buffer, new_string.data(), len);
-    cur_buffer += len;
-    available_capacity -= len;
-  }
-}
-
-void StringWriter::write(char new_char, size_t len) {
-  if (len > available_capacity)
-    len = available_capacity;
-
-  if (len > 0) {
-    inline_memset(cur_buffer, static_cast<uint8_t>(new_char), len);
-    cur_buffer += len;
-    available_capacity -= len;
-  }
-}
-
-void StringWriter::write(char new_char) {
-  if (1 > available_capacity)
-    return;
-
-  cur_buffer[0] = new_char;
-  ++cur_buffer;
-  available_capacity -= 1;
-}
-
-int StringWriter::write_str(void *raw_pointer, cpp::string_view new_string) {
-  StringWriter *string_writer = reinterpret_cast<StringWriter *>(raw_pointer);
-  string_writer->write(new_string);
-  return WRITE_OK;
-}
-
-int StringWriter::write_chars(void *raw_pointer, char new_char, size_t len) {
-  StringWriter *string_writer = reinterpret_cast<StringWriter *>(raw_pointer);
-  string_writer->write(new_char, len);
-  return WRITE_OK;
-}
-
-int StringWriter::write_char(void *raw_pointer, char new_char) {
-  StringWriter *string_writer = reinterpret_cast<StringWriter *>(raw_pointer);
-  string_writer->write(new_char);
-  return WRITE_OK;
-}
-
-} // namespace printf_core
-} // namespace __llvm_libc

diff  --git a/libc/src/stdio/printf_core/string_writer.h b/libc/src/stdio/printf_core/string_writer.h
deleted file mode 100644
index 8c5ccabee510d2..00000000000000
--- a/libc/src/stdio/printf_core/string_writer.h
+++ /dev/null
@@ -1,48 +0,0 @@
-//===-- String Writer definition for printf ---------------------*- C++ -*-===//
-//
-// 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
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLVM_LIBC_SRC_STDIO_PRINTF_CORE_STRING_WRITER_H
-#define LLVM_LIBC_SRC_STDIO_PRINTF_CORE_STRING_WRITER_H
-
-#include "src/__support/CPP/string_view.h"
-#include <stddef.h>
-
-namespace __llvm_libc {
-namespace printf_core {
-
-class StringWriter {
-  char *__restrict cur_buffer;
-  size_t available_capacity;
-
-public:
-  // StringWriter is intended to take a copy of the cur_buffer pointer, as well
-  // as the maximum length of the string. This maximum length should not include
-  // the null terminator, since that's written separately.
-  StringWriter(char *__restrict buffer, size_t max_len = ~size_t(0))
-      : cur_buffer(buffer), available_capacity(max_len) {}
-
-  void write(cpp::string_view new_string);
-  void write(char new_char, size_t len);
-  void write(char new_char);
-
-  // Terminate should only be called if the original max length passed to
-  // snprintf was greater than 0. It writes a null byte to the end of the
-  // cur_buffer, regardless of available_capacity.
-  void terminate() { *cur_buffer = '\0'; }
-
-  // These write functions take a StringWriter as a void* in raw_pointer, and
-  // call the appropriate write function on it.
-  static int write_str(void *raw_pointer, cpp::string_view new_string);
-  static int write_chars(void *raw_pointer, char new_char, size_t len);
-  static int write_char(void *raw_pointer, char new_char);
-};
-
-} // namespace printf_core
-} // namespace __llvm_libc
-
-#endif // LLVM_LIBC_SRC_STDIO_PRINTF_CORE_STRING_WRITER_H

diff  --git a/libc/src/stdio/printf_core/vfprintf_internal.h b/libc/src/stdio/printf_core/vfprintf_internal.h
index 762018f0b04c41..198213c18c816e 100644
--- a/libc/src/stdio/printf_core/vfprintf_internal.h
+++ b/libc/src/stdio/printf_core/vfprintf_internal.h
@@ -12,24 +12,74 @@
 #include "src/__support/File/file.h"
 #include "src/__support/arg_list.h"
 #include "src/__support/macros/attributes.h" // For LIBC_INLINE
-#include "src/stdio/printf_core/file_writer.h"
+#include "src/stdio/printf_core/core_structs.h"
 #include "src/stdio/printf_core/printf_main.h"
 #include "src/stdio/printf_core/writer.h"
 
 #include <stdio.h>
 
 namespace __llvm_libc {
+
+namespace internal {
+#ifndef LIBC_COPT_PRINTF_USE_SYSTEM_FILE
+LIBC_INLINE int ferror_unlocked(FILE *f) {
+  return reinterpret_cast<__llvm_libc::File *>(f)->error_unlocked();
+}
+
+LIBC_INLINE void flockfile(FILE *f) {
+  reinterpret_cast<__llvm_libc::File *>(f)->lock();
+}
+
+LIBC_INLINE void funlockfile(FILE *f) {
+  reinterpret_cast<__llvm_libc::File *>(f)->unlock();
+}
+
+LIBC_INLINE int fwrite_unlocked(const void *ptr, size_t size, size_t nmemb,
+                                FILE *f) {
+  return reinterpret_cast<__llvm_libc::File *>(f)->write_unlocked(ptr,
+                                                                  size * nmemb);
+}
+#else  // defined(LIBC_COPT_PRINTF_USE_SYSTEM_FILE)
+LIBC_INLINE int ferror_unlocked(::FILE *f) { return ::ferror_unlocked(f); }
+
+LIBC_INLINE void flockfile(::FILE *f) { ::flockfile(f); }
+
+LIBC_INLINE void funlockfile(::FILE *f) { ::funlockfile(f); }
+
+LIBC_INLINE int fwrite_unlocked(const void *ptr, size_t size, size_t nmemb,
+                                ::FILE *f) {
+  return ::fwrite_unlocked(ptr, size, nmemb, f);
+}
+#endif // LIBC_COPT_PRINTF_USE_SYSTEM_FILE
+} // namespace internal
+
 namespace printf_core {
 
-template <typename file_t>
-LIBC_INLINE int vfprintf_internal(file_t *__restrict stream,
-                                  const char *__restrict format,
-                                  internal::ArgList &args) {
-  FileWriter<file_t> file_writer(stream);
-  Writer writer(reinterpret_cast<void *>(&file_writer),
-                FileWriter<file_t>::write_str, FileWriter<file_t>::write_chars,
-                FileWriter<file_t>::write_char);
-  return printf_main(&writer, format, args);
+int file_write_hook(cpp::string_view new_str, void *fp) {
+  ::FILE *target_file = reinterpret_cast<::FILE *>(fp);
+  // Write new_str to the target file. The logic preventing a zero-length write
+  // is in the writer, so we don't check here.
+  size_t written = internal::fwrite_unlocked(new_str.data(), sizeof(char),
+                                             new_str.size(), target_file);
+  if (written != new_str.size() || internal::ferror_unlocked(target_file))
+    return FILE_WRITE_ERROR;
+  return WRITE_OK;
+}
+
+int vfprintf_internal(::FILE *__restrict stream, const char *__restrict format,
+                      internal::ArgList &args) {
+  constexpr size_t BUFF_SIZE = 1024;
+  char buffer[BUFF_SIZE];
+  printf_core::WriteBuffer wb(buffer, BUFF_SIZE, &file_write_hook,
+                              reinterpret_cast<void *>(stream));
+  Writer writer(&wb);
+  internal::flockfile(stream);
+  int retval = printf_main(&writer, format, args);
+  int flushval = wb.overflow_write("");
+  if (flushval != WRITE_OK)
+    retval = flushval;
+  internal::funlockfile(stream);
+  return retval;
 }
 
 } // namespace printf_core

diff  --git a/libc/src/stdio/printf_core/writer.cpp b/libc/src/stdio/printf_core/writer.cpp
index 825697103492cf..5f8091aec58a28 100644
--- a/libc/src/stdio/printf_core/writer.cpp
+++ b/libc/src/stdio/printf_core/writer.cpp
@@ -8,24 +8,39 @@
 
 #include "writer.h"
 #include "src/__support/CPP/string_view.h"
+#include "src/__support/macros/optimization.h"
+#include "src/stdio/printf_core/core_structs.h"
+#include "src/string/memory_utils/inline_memcpy.h"
+#include "src/string/memory_utils/inline_memset.h"
 #include <stddef.h>
 
 namespace __llvm_libc {
 namespace printf_core {
 
-int Writer::write(cpp::string_view new_string) {
-  chars_written += new_string.size();
-  return str_write(output, new_string);
-}
-
-int Writer::write(char new_char, size_t length) {
-  chars_written += length;
-  return chars_write(output, new_char, length);
-}
+int Writer::pad(char new_char, size_t length) {
+  // First, fill as much of the buffer as possible with the padding char.
+  size_t written = 0;
+  const size_t buff_space = wb->buff_len - wb->buff_cur;
+  // ASSERT: length > buff_space
+  if (buff_space > 0) {
+    inline_memset(wb->buff + wb->buff_cur, new_char, buff_space);
+    wb->buff_cur += buff_space;
+    written = buff_space;
+  }
 
-int Writer::write(char new_char) {
-  chars_written += 1;
-  return char_write(output, new_char);
+  // Next, overflow write the rest of length using the mini_buff.
+  constexpr size_t MINI_BUFF_SIZE = 64;
+  char mini_buff[MINI_BUFF_SIZE];
+  inline_memset(mini_buff, new_char, MINI_BUFF_SIZE);
+  cpp::string_view mb_string_view(mini_buff, MINI_BUFF_SIZE);
+  while (written + MINI_BUFF_SIZE < length) {
+    int result = wb->overflow_write(mb_string_view);
+    if (result != WRITE_OK)
+      return result;
+    written += MINI_BUFF_SIZE;
+  }
+  cpp::string_view mb_substr = mb_string_view.substr(0, length - written);
+  return wb->overflow_write(mb_substr);
 }
 
 } // namespace printf_core

diff  --git a/libc/src/stdio/printf_core/writer.h b/libc/src/stdio/printf_core/writer.h
index b5c36bf38e8141..1cf05de5a49d1b 100644
--- a/libc/src/stdio/printf_core/writer.h
+++ b/libc/src/stdio/printf_core/writer.h
@@ -10,51 +10,127 @@
 #define LLVM_LIBC_SRC_STDIO_PRINTF_CORE_WRITER_H
 
 #include "src/__support/CPP/string_view.h"
+#include "src/__support/macros/optimization.h"
+#include "src/stdio/printf_core/core_structs.h"
+#include "src/string/memory_utils/inline_memcpy.h"
+#include "src/string/memory_utils/inline_memset.h"
+
 #include <stddef.h>
 
 namespace __llvm_libc {
 namespace printf_core {
 
-using WriteStrFunc = int (*)(void *, cpp::string_view);
-using WriteCharsFunc = int (*)(void *, char, size_t);
-using WriteCharFunc = int (*)(void *, char);
+struct WriteBuffer {
+  using StreamWriter = int (*)(cpp::string_view, void *);
+  char *buff;
+  const size_t buff_len;
+  size_t buff_cur = 0;
 
-class Writer final {
-  // output is a pointer to the string or file that the writer is meant to write
-  // to.
-  void *output;
-
-  // raw_write is a function that, when called on output with a char* and
-  // length, will copy the number of bytes equal to the length from the char*
-  // onto the end of output. It should return a positive number or zero on
-  // success, or a negative number on failure.
-  WriteStrFunc str_write;
-  WriteCharsFunc chars_write;
-  WriteCharFunc char_write;
+  // The stream writer will be called when the buffer is full. It will be passed
+  // string_views to write to the stream.
+  StreamWriter stream_writer;
+  void *output_target;
+
+  LIBC_INLINE WriteBuffer(char *Buff, size_t Buff_len, StreamWriter hook,
+                          void *target)
+      : buff(Buff), buff_len(Buff_len), stream_writer(hook),
+        output_target(target) {}
+
+  LIBC_INLINE WriteBuffer(char *Buff, size_t Buff_len)
+      : buff(Buff), buff_len(Buff_len), stream_writer(nullptr),
+        output_target(nullptr) {}
 
+  // The overflow_write method is intended to be called to write the contents of
+  // the buffer and new_str to the stream_writer if it exists, else it will
+  // write as much of new_str to the buffer as it can. The current position in
+  // the buffer will be reset iff stream_writer is called. Calling this with an
+  // empty string will flush the buffer if relevant.
+  int overflow_write(cpp::string_view new_str) {
+    // If there is a stream_writer, write the contents of the buffer, then
+    // new_str, then clear the buffer.
+    if (stream_writer != nullptr) {
+      if (buff_cur > 0) {
+        int retval = stream_writer({buff, buff_cur}, output_target);
+        if (retval < 0) {
+          return retval;
+        }
+      }
+      if (new_str.size() > 0) {
+        int retval = stream_writer(new_str, output_target);
+        if (retval < 0) {
+          return retval;
+        }
+      }
+      buff_cur = 0;
+      return WRITE_OK;
+    } else {
+      // We can't flush to the stream, so fill the rest of the buffer, then drop
+      // the overflow.
+      if (buff_cur < buff_len) {
+        size_t bytes_to_write = buff_len - buff_cur;
+        if (bytes_to_write > new_str.size()) {
+          bytes_to_write = new_str.size();
+        }
+        inline_memcpy(buff + buff_cur, new_str.data(), bytes_to_write);
+        buff_cur += bytes_to_write;
+      }
+      return WRITE_OK;
+    }
+  }
+};
+
+class Writer final {
+  WriteBuffer *wb;
   int chars_written = 0;
 
+  // This is a separate, non-inlined function so that the inlined part of the
+  // write function is shorter.
+  int pad(char new_char, size_t length);
+
 public:
-  Writer(void *init_output, WriteStrFunc init_str_write,
-         WriteCharsFunc init_chars_write, WriteCharFunc init_char_write)
-      : output(init_output), str_write(init_str_write),
-        chars_write(init_chars_write), char_write(init_char_write) {}
-
-  // write will copy new_string into output using str_write. It increments
-  // chars_written by the length of new_string. It returns the result of
-  // str_write.
-  int write(cpp::string_view new_string);
-
-  // this version of write will copy length copies of new_char into output using
-  // chars_write. This is primarily used for padding.  It returns the result of
-  // chars_write.
-  int write(char new_char, size_t len);
-
-  // this version of write will copy just new_char into output. This is often
-  // used for negative signs. It returns the result of chars_write.
-  int write(char new_char);
-
-  int get_chars_written() { return chars_written; }
+  LIBC_INLINE Writer(WriteBuffer *WB) : wb(WB) {}
+
+  // Takes a string, copies it into the buffer if there is space, else passes it
+  // to the overflow mechanism to be handled separately.
+  LIBC_INLINE int write(cpp::string_view new_string) {
+    chars_written += new_string.size();
+    if (LIBC_LIKELY(wb->buff_cur + new_string.size() <= wb->buff_len)) {
+      inline_memcpy(wb->buff + wb->buff_cur, new_string.data(),
+                    new_string.size());
+      wb->buff_cur += new_string.size();
+      return WRITE_OK;
+    }
+    return wb->overflow_write(new_string);
+  }
+
+  // Takes a char and a length, memsets the next length characters of the buffer
+  // if there is space, else calls pad which will loop and call the overflow
+  // mechanism on a secondary buffer.
+  LIBC_INLINE int write(char new_char, size_t length) {
+    chars_written += length;
+
+    if (LIBC_LIKELY(wb->buff_cur + length <= wb->buff_len)) {
+      inline_memset(wb->buff + wb->buff_cur, new_char, length);
+      wb->buff_cur += length;
+      return WRITE_OK;
+    }
+    return pad(new_char, length);
+  }
+
+  // Takes a char, copies it into the buffer if there is space, else passes it
+  // to the overflow mechanism to be handled separately.
+  LIBC_INLINE int write(char new_char) {
+    chars_written += 1;
+    if (LIBC_LIKELY(wb->buff_cur + 1 <= wb->buff_len)) {
+      wb->buff[wb->buff_cur] = new_char;
+      wb->buff_cur += 1;
+      return WRITE_OK;
+    }
+    cpp::string_view char_string_view(&new_char, 1);
+    return wb->overflow_write(char_string_view);
+  }
+
+  LIBC_INLINE int get_chars_written() { return chars_written; }
 };
 
 } // namespace printf_core

diff  --git a/libc/src/stdio/snprintf.cpp b/libc/src/stdio/snprintf.cpp
index 571a547ed8f2dc..4e1973fe9be3d1 100644
--- a/libc/src/stdio/snprintf.cpp
+++ b/libc/src/stdio/snprintf.cpp
@@ -10,7 +10,6 @@
 
 #include "src/__support/arg_list.h"
 #include "src/stdio/printf_core/printf_main.h"
-#include "src/stdio/printf_core/string_writer.h"
 #include "src/stdio/printf_core/writer.h"
 
 #include <stdarg.h>
@@ -27,15 +26,12 @@ LLVM_LIBC_FUNCTION(int, snprintf,
                                  // and pointer semantics, as well as handling
                                  // destruction automatically.
   va_end(vlist);
-  printf_core::StringWriter str_writer(buffer, (buffsz > 0 ? buffsz - 1 : 0));
-  printf_core::Writer writer(reinterpret_cast<void *>(&str_writer),
-                             printf_core::StringWriter::write_str,
-                             printf_core::StringWriter::write_chars,
-                             printf_core::StringWriter::write_char);
+  printf_core::WriteBuffer wb(buffer, (buffsz > 0 ? buffsz - 1 : 0));
+  printf_core::Writer writer(&wb);
 
   int ret_val = printf_core::printf_main(&writer, format, args);
   if (buffsz > 0) // if the buffsz is 0 the buffer may be a null pointer.
-    str_writer.terminate();
+    wb.buff[wb.buff_cur] = '\0';
   return ret_val;
 }
 

diff  --git a/libc/src/stdio/sprintf.cpp b/libc/src/stdio/sprintf.cpp
index a97b83e805d1e6..3e4206be0aa698 100644
--- a/libc/src/stdio/sprintf.cpp
+++ b/libc/src/stdio/sprintf.cpp
@@ -8,9 +8,9 @@
 
 #include "src/stdio/sprintf.h"
 
+#include "src/__support/CPP/limits.h"
 #include "src/__support/arg_list.h"
 #include "src/stdio/printf_core/printf_main.h"
-#include "src/stdio/printf_core/string_writer.h"
 #include "src/stdio/printf_core/writer.h"
 
 #include <stdarg.h>
@@ -26,14 +26,12 @@ LLVM_LIBC_FUNCTION(int, sprintf,
                                  // and pointer semantics, as well as handling
                                  // destruction automatically.
   va_end(vlist);
-  printf_core::StringWriter str_writer(buffer);
-  printf_core::Writer writer(reinterpret_cast<void *>(&str_writer),
-                             printf_core::StringWriter::write_str,
-                             printf_core::StringWriter::write_chars,
-                             printf_core::StringWriter::write_char);
+
+  printf_core::WriteBuffer wb(buffer, cpp::numeric_limits<size_t>::max());
+  printf_core::Writer writer(&wb);
 
   int ret_val = printf_core::printf_main(&writer, format, args);
-  str_writer.terminate();
+  wb.buff[wb.buff_cur] = '\0';
   return ret_val;
 }
 

diff  --git a/libc/test/src/stdio/CMakeLists.txt b/libc/test/src/stdio/CMakeLists.txt
index 7b28a9c8a9b166..8ad48545080729 100644
--- a/libc/test/src/stdio/CMakeLists.txt
+++ b/libc/test/src/stdio/CMakeLists.txt
@@ -135,23 +135,22 @@ add_libc_unittest(
     libc.src.stdio.snprintf
 )
 
-# In fullbuild mode, fprintf's tests use the internal FILE for other functions.
-if(LLVM_LIBC_FULL_BUILD)
-add_libc_unittest(
-  fprintf_test
-  SUITE
-    libc_stdio_unittests
-  SRCS
-    fprintf_test.cpp
-  DEPENDS
-    libc.src.stdio.fprintf
-    libc.src.stdio.fclose
-    libc.src.stdio.ferror
-    libc.src.stdio.fopen
-    libc.src.stdio.fread
+list(APPEND fprintf_test_deps
+      libc.src.stdio.fprintf
 )
+if(LLVM_LIBC_FULL_BUILD)
+# In fullbuild mode, fprintf's tests use the internal FILE for other functions.
+ list(APPEND fprintf_test_deps
+      libc.src.stdio.fclose
+      libc.src.stdio.ferror
+      libc.src.stdio.fopen
+      libc.src.stdio.fread
+  )
 else()
 # Else in overlay mode they use the system's FILE.
+ set(fprintf_test_copts "-DLIBC_COPT_PRINTF_USE_SYSTEM_FILE")
+endif()
+
 add_libc_unittest(
   fprintf_test
   SUITE
@@ -159,11 +158,10 @@ add_libc_unittest(
   SRCS
     fprintf_test.cpp
   DEPENDS
-    libc.src.stdio.fprintf
+    ${fprintf_test_deps}
   COMPILE_OPTIONS
-    -DLIBC_COPT_PRINTF_USE_SYSTEM_FILE
+    ${fprintf_test_copts}
 )
-endif()
 
 add_libc_unittest(
   printf_test

diff  --git a/libc/test/src/stdio/printf_core/CMakeLists.txt b/libc/test/src/stdio/printf_core/CMakeLists.txt
index 7636eec138d388..ff7ebbc4f5fd0b 100644
--- a/libc/test/src/stdio/printf_core/CMakeLists.txt
+++ b/libc/test/src/stdio/printf_core/CMakeLists.txt
@@ -14,14 +14,14 @@ add_libc_unittest(
 )
 
 add_libc_unittest(
-  string_writer_test
+  writer_test
   SUITE
     libc_stdio_unittests
   SRCS
-    string_writer_test.cpp
+    writer_test.cpp
   DEPENDS
     libc.src.stdio.printf_core.writer
-    libc.src.stdio.printf_core.string_writer
+    libc.src.string.memory_utils.inline_memcpy
     libc.src.__support.CPP.string_view
 )
 
@@ -34,6 +34,5 @@ add_libc_unittest(
   DEPENDS
     libc.src.stdio.printf_core.converter
     libc.src.stdio.printf_core.writer
-    libc.src.stdio.printf_core.string_writer
     libc.src.stdio.printf_core.core_structs
 )

diff  --git a/libc/test/src/stdio/printf_core/converter_test.cpp b/libc/test/src/stdio/printf_core/converter_test.cpp
index 55b77794720d2e..69860d627f800d 100644
--- a/libc/test/src/stdio/printf_core/converter_test.cpp
+++ b/libc/test/src/stdio/printf_core/converter_test.cpp
@@ -8,7 +8,6 @@
 
 #include "src/stdio/printf_core/converter.h"
 #include "src/stdio/printf_core/core_structs.h"
-#include "src/stdio/printf_core/string_writer.h"
 #include "src/stdio/printf_core/writer.h"
 
 #include "test/UnitTest/Test.h"
@@ -19,13 +18,10 @@ class LlvmLibcPrintfConverterTest : public __llvm_libc::testing::Test {
   // void TearDown() override {}
 
   char str[60];
-  __llvm_libc::printf_core::StringWriter str_writer =
-      __llvm_libc::printf_core::StringWriter(str);
-  __llvm_libc::printf_core::Writer writer = __llvm_libc::printf_core::Writer(
-      reinterpret_cast<void *>(&str_writer),
-      __llvm_libc::printf_core::StringWriter::write_str,
-      __llvm_libc::printf_core::StringWriter::write_chars,
-      __llvm_libc::printf_core::StringWriter::write_char);
+  __llvm_libc::printf_core::WriteBuffer wb =
+      __llvm_libc::printf_core::WriteBuffer(str, sizeof(str) - 1);
+  __llvm_libc::printf_core::Writer writer =
+      __llvm_libc::printf_core::Writer(&wb);
 };
 
 TEST_F(LlvmLibcPrintfConverterTest, SimpleRawConversion) {
@@ -35,7 +31,7 @@ TEST_F(LlvmLibcPrintfConverterTest, SimpleRawConversion) {
 
   __llvm_libc::printf_core::convert(&writer, raw_section);
 
-  str_writer.terminate();
+  wb.buff[wb.buff_cur] = '\0';
 
   ASSERT_STREQ(str, "abc");
   ASSERT_EQ(writer.get_chars_written(), 3);
@@ -49,7 +45,7 @@ TEST_F(LlvmLibcPrintfConverterTest, PercentConversion) {
 
   __llvm_libc::printf_core::convert(&writer, simple_conv);
 
-  str[1] = '\0';
+  wb.buff[wb.buff_cur] = '\0';
 
   ASSERT_STREQ(str, "%");
   ASSERT_EQ(writer.get_chars_written(), 1);
@@ -67,7 +63,7 @@ TEST_F(LlvmLibcPrintfConverterTest, CharConversionSimple) {
 
   __llvm_libc::printf_core::convert(&writer, simple_conv);
 
-  str_writer.terminate();
+  wb.buff[wb.buff_cur] = '\0';
 
   ASSERT_STREQ(str, "D");
   ASSERT_EQ(writer.get_chars_written(), 1);
@@ -82,7 +78,7 @@ TEST_F(LlvmLibcPrintfConverterTest, CharConversionRightJustified) {
   right_justified_conv.conv_val_raw = 'E';
   __llvm_libc::printf_core::convert(&writer, right_justified_conv);
 
-  str_writer.terminate();
+  wb.buff[wb.buff_cur] = '\0';
 
   ASSERT_STREQ(str, "   E");
   ASSERT_EQ(writer.get_chars_written(), 4);
@@ -99,7 +95,7 @@ TEST_F(LlvmLibcPrintfConverterTest, CharConversionLeftJustified) {
   left_justified_conv.conv_val_raw = 'F';
   __llvm_libc::printf_core::convert(&writer, left_justified_conv);
 
-  str_writer.terminate();
+  wb.buff[wb.buff_cur] = '\0';
 
   ASSERT_STREQ(str, "F   ");
   ASSERT_EQ(writer.get_chars_written(), 4);
@@ -115,7 +111,7 @@ TEST_F(LlvmLibcPrintfConverterTest, StringConversionSimple) {
 
   __llvm_libc::printf_core::convert(&writer, simple_conv);
 
-  str_writer.terminate();
+  wb.buff[wb.buff_cur] = '\0';
 
   ASSERT_STREQ(str, "DEF");
   ASSERT_EQ(writer.get_chars_written(), 3);
@@ -130,7 +126,7 @@ TEST_F(LlvmLibcPrintfConverterTest, StringConversionPrecisionHigh) {
   high_precision_conv.conv_val_ptr = const_cast<char *>("456");
   __llvm_libc::printf_core::convert(&writer, high_precision_conv);
 
-  str_writer.terminate();
+  wb.buff[wb.buff_cur] = '\0';
 
   ASSERT_STREQ(str, "456");
   ASSERT_EQ(writer.get_chars_written(), 3);
@@ -145,7 +141,7 @@ TEST_F(LlvmLibcPrintfConverterTest, StringConversionPrecisionLow) {
   low_precision_conv.conv_val_ptr = const_cast<char *>("xyz");
   __llvm_libc::printf_core::convert(&writer, low_precision_conv);
 
-  str_writer.terminate();
+  wb.buff[wb.buff_cur] = '\0';
 
   ASSERT_STREQ(str, "xy");
   ASSERT_EQ(writer.get_chars_written(), 2);
@@ -160,7 +156,7 @@ TEST_F(LlvmLibcPrintfConverterTest, StringConversionRightJustified) {
   right_justified_conv.conv_val_ptr = const_cast<char *>("789");
   __llvm_libc::printf_core::convert(&writer, right_justified_conv);
 
-  str_writer.terminate();
+  wb.buff[wb.buff_cur] = '\0';
 
   ASSERT_STREQ(str, " 789");
   ASSERT_EQ(writer.get_chars_written(), 4);
@@ -177,7 +173,7 @@ TEST_F(LlvmLibcPrintfConverterTest, StringConversionLeftJustified) {
   left_justified_conv.conv_val_ptr = const_cast<char *>("ghi");
   __llvm_libc::printf_core::convert(&writer, left_justified_conv);
 
-  str_writer.terminate();
+  wb.buff[wb.buff_cur] = '\0';
 
   ASSERT_STREQ(str, "ghi ");
   ASSERT_EQ(writer.get_chars_written(), 4);
@@ -191,7 +187,7 @@ TEST_F(LlvmLibcPrintfConverterTest, IntConversionSimple) {
   section.conv_val_raw = 12345;
   __llvm_libc::printf_core::convert(&writer, section);
 
-  str_writer.terminate();
+  wb.buff[wb.buff_cur] = '\0';
 
   ASSERT_STREQ(str, "12345");
   ASSERT_EQ(writer.get_chars_written(), 5);
@@ -209,7 +205,7 @@ TEST_F(LlvmLibcPrintfConverterTest, HexConversion) {
   section.conv_val_raw = 0x123456ab;
   __llvm_libc::printf_core::convert(&writer, section);
 
-  str_writer.terminate();
+  wb.buff[wb.buff_cur] = '\0';
   ASSERT_STREQ(str, "0x00000000123456ab");
   ASSERT_EQ(writer.get_chars_written(), 18);
 }
@@ -223,7 +219,7 @@ TEST_F(LlvmLibcPrintfConverterTest, PointerConversion) {
   section.conv_val_ptr = (void *)(0x123456ab);
   __llvm_libc::printf_core::convert(&writer, section);
 
-  str_writer.terminate();
+  wb.buff[wb.buff_cur] = '\0';
   ASSERT_STREQ(str, "0x123456ab");
   ASSERT_EQ(writer.get_chars_written(), 10);
 }
@@ -237,7 +233,7 @@ TEST_F(LlvmLibcPrintfConverterTest, OctConversion) {
   section.conv_val_raw = 01234;
   __llvm_libc::printf_core::convert(&writer, section);
 
-  str_writer.terminate();
+  wb.buff[wb.buff_cur] = '\0';
   ASSERT_STREQ(str, "1234");
   ASSERT_EQ(writer.get_chars_written(), 4);
 }

diff  --git a/libc/test/src/stdio/printf_core/string_writer_test.cpp b/libc/test/src/stdio/printf_core/string_writer_test.cpp
deleted file mode 100644
index 7e67137584326f..00000000000000
--- a/libc/test/src/stdio/printf_core/string_writer_test.cpp
+++ /dev/null
@@ -1,205 +0,0 @@
-//===-- Unittests for the printf String Writer ----------------------------===//
-//
-// 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
-//
-//===----------------------------------------------------------------------===//
-
-#include "src/__support/CPP/string_view.h"
-#include "src/stdio/printf_core/string_writer.h"
-#include "src/stdio/printf_core/writer.h"
-
-#include "test/UnitTest/Test.h"
-
-using __llvm_libc::cpp::string_view;
-
-__llvm_libc::printf_core::Writer get_writer(void *str_writer) {
-  return __llvm_libc::printf_core::Writer(
-      str_writer, __llvm_libc::printf_core::StringWriter::write_str,
-      __llvm_libc::printf_core::StringWriter::write_chars,
-      __llvm_libc::printf_core::StringWriter::write_char);
-}
-
-TEST(LlvmLibcPrintfStringWriterTest, Constructor) {
-  char str[10];
-  __llvm_libc::printf_core::StringWriter str_writer(str);
-  __llvm_libc::printf_core::Writer writer =
-      get_writer(reinterpret_cast<void *>(&str_writer));
-  (void)writer;
-}
-
-TEST(LlvmLibcPrintfStringWriterTest, Write) {
-  char str[4] = {'D', 'E', 'F', 'G'};
-  __llvm_libc::printf_core::StringWriter str_writer(str);
-  __llvm_libc::printf_core::Writer writer =
-      get_writer(reinterpret_cast<void *>(&str_writer));
-  writer.write({"abc", 3});
-
-  EXPECT_EQ(str[3], 'G');
-  // This null terminates the string. The writer has no indication when the
-  // string is done, so it relies on the user to tell it when to null terminate
-  // the string. Importantly, it can't tell the 
diff erence between an intended
-  // max length of 0 (write nothing) or 1 (write just a null byte), and so it
-  // relies on the caller to do that bounds check.
-  str_writer.terminate();
-
-  ASSERT_STREQ("abc", str);
-  ASSERT_EQ(writer.get_chars_written(), 3);
-}
-
-TEST(LlvmLibcPrintfStringWriterTest, WriteMultipleTimes) {
-  char str[10];
-  __llvm_libc::printf_core::StringWriter str_writer(str);
-  __llvm_libc::printf_core::Writer writer =
-      get_writer(reinterpret_cast<void *>(&str_writer));
-  writer.write({"abc", 3});
-  writer.write({"DEF", 3});
-  writer.write({"1234", 3});
-
-  str_writer.terminate();
-
-  ASSERT_STREQ("abcDEF123", str);
-  ASSERT_EQ(writer.get_chars_written(), 9);
-}
-
-TEST(LlvmLibcPrintfStringWriterTest, WriteChars) {
-  char str[4] = {'D', 'E', 'F', 'G'};
-  __llvm_libc::printf_core::StringWriter str_writer(str);
-  __llvm_libc::printf_core::Writer writer =
-      get_writer(reinterpret_cast<void *>(&str_writer));
-  writer.write('a', 3);
-
-  EXPECT_EQ(str[3], 'G');
-  str_writer.terminate();
-
-  ASSERT_STREQ("aaa", str);
-  ASSERT_EQ(writer.get_chars_written(), 3);
-}
-
-TEST(LlvmLibcPrintfStringWriterTest, WriteCharsMultipleTimes) {
-  char str[10];
-  __llvm_libc::printf_core::StringWriter str_writer(str);
-  __llvm_libc::printf_core::Writer writer =
-      get_writer(reinterpret_cast<void *>(&str_writer));
-  writer.write('a', 3);
-  writer.write('D', 3);
-  writer.write('1', 3);
-
-  str_writer.terminate();
-
-  ASSERT_STREQ("aaaDDD111", str);
-  ASSERT_EQ(writer.get_chars_written(), 9);
-}
-
-TEST(LlvmLibcPrintfStringWriterTest, WriteManyChars) {
-  char str[100];
-  __llvm_libc::printf_core::StringWriter str_writer(str);
-  __llvm_libc::printf_core::Writer writer =
-      get_writer(reinterpret_cast<void *>(&str_writer));
-  writer.write('Z', 99);
-
-  str_writer.terminate();
-
-  ASSERT_STREQ("ZZZZZZZZZZ"
-               "ZZZZZZZZZZ"
-               "ZZZZZZZZZZ"
-               "ZZZZZZZZZZ"
-               "ZZZZZZZZZZ"
-               "ZZZZZZZZZZ"
-               "ZZZZZZZZZZ"
-               "ZZZZZZZZZZ"
-               "ZZZZZZZZZZ"
-               "ZZZZZZZZZ",
-               str);
-  ASSERT_EQ(writer.get_chars_written(), 99);
-}
-
-TEST(LlvmLibcPrintfStringWriterTest, MixedWrites) {
-  char str[13];
-  __llvm_libc::printf_core::StringWriter str_writer(str);
-  __llvm_libc::printf_core::Writer writer =
-      get_writer(reinterpret_cast<void *>(&str_writer));
-  writer.write('a', 3);
-  writer.write({"DEF", 3});
-  writer.write('1', 3);
-  writer.write({"456", 3});
-
-  str_writer.terminate();
-
-  ASSERT_STREQ("aaaDEF111456", str);
-  ASSERT_EQ(writer.get_chars_written(), 12);
-}
-
-TEST(LlvmLibcPrintfStringWriterTest, WriteWithMaxLength) {
-  char str[11];
-  __llvm_libc::printf_core::StringWriter str_writer(str, 10);
-  __llvm_libc::printf_core::Writer writer =
-      get_writer(reinterpret_cast<void *>(&str_writer));
-  writer.write({"abcDEF123456", 12});
-
-  str_writer.terminate();
-
-  ASSERT_STREQ("abcDEF1234", str);
-  ASSERT_EQ(writer.get_chars_written(), 12);
-}
-
-TEST(LlvmLibcPrintfStringWriterTest, WriteCharsWithMaxLength) {
-  char str[11];
-  __llvm_libc::printf_core::StringWriter str_writer(str, 10);
-  __llvm_libc::printf_core::Writer writer =
-      get_writer(reinterpret_cast<void *>(&str_writer));
-
-  writer.write('1', 15);
-
-  str_writer.terminate();
-
-  ASSERT_STREQ("1111111111", str);
-  ASSERT_EQ(writer.get_chars_written(), 15);
-}
-
-TEST(LlvmLibcPrintfStringWriterTest, MixedWriteWithMaxLength) {
-  char str[11];
-  __llvm_libc::printf_core::StringWriter str_writer(str, 10);
-  __llvm_libc::printf_core::Writer writer =
-      get_writer(reinterpret_cast<void *>(&str_writer));
-  writer.write('a', 3);
-  writer.write({"DEF", 3});
-  writer.write('1', 3);
-  writer.write({"456", 3});
-
-  str_writer.terminate();
-
-  ASSERT_STREQ("aaaDEF1114", str);
-  ASSERT_EQ(writer.get_chars_written(), 12);
-}
-
-TEST(LlvmLibcPrintfStringWriterTest, StringWithMaxLengthOne) {
-  char str[1];
-  __llvm_libc::printf_core::StringWriter str_writer(str, 0);
-  __llvm_libc::printf_core::Writer writer =
-      get_writer(reinterpret_cast<void *>(&str_writer));
-  // This is because the max length should be at most 1 less than the size of
-  // the buffer it's writing to.
-  writer.write('a', 3);
-  writer.write({"DEF", 3});
-  writer.write('1', 3);
-  writer.write({"456", 3});
-
-  str_writer.terminate();
-
-  ASSERT_STREQ("", str);
-  ASSERT_EQ(writer.get_chars_written(), 12);
-}
-
-TEST(LlvmLibcPrintfStringWriterTest, NullStringWithZeroMaxLength) {
-  __llvm_libc::printf_core::StringWriter str_writer(nullptr, 0);
-  __llvm_libc::printf_core::Writer writer =
-      get_writer(reinterpret_cast<void *>(&str_writer));
-  writer.write('a', 3);
-  writer.write({"DEF", 3});
-  writer.write('1', 3);
-  writer.write({"456", 3});
-
-  ASSERT_EQ(writer.get_chars_written(), 12);
-}

diff  --git a/libc/test/src/stdio/printf_core/writer_test.cpp b/libc/test/src/stdio/printf_core/writer_test.cpp
new file mode 100644
index 00000000000000..f31ff91d64960e
--- /dev/null
+++ b/libc/test/src/stdio/printf_core/writer_test.cpp
@@ -0,0 +1,310 @@
+//===-- Unittests for the printf String Writer ----------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/__support/CPP/string_view.h"
+#include "src/stdio/printf_core/writer.h"
+
+#include "src/string/memory_utils/inline_memcpy.h"
+
+#include "test/UnitTest/Test.h"
+
+using __llvm_libc::cpp::string_view;
+using __llvm_libc::printf_core::WriteBuffer;
+using __llvm_libc::printf_core::Writer;
+
+TEST(LlvmLibcPrintfWriterTest, Constructor) {
+  char str[10];
+  WriteBuffer wb(str, sizeof(str) - 1);
+  Writer writer(&wb);
+  (void)writer;
+}
+
+TEST(LlvmLibcPrintfWriterTest, Write) {
+  char str[4] = {'D', 'E', 'F', 'G'};
+  WriteBuffer wb(str, sizeof(str) - 1);
+  Writer writer(&wb);
+  writer.write({"abc", 3});
+
+  EXPECT_EQ(str[3], 'G');
+
+  // The string must be null terminated manually since the writer cannot tell
+  // when it's done.
+  wb.buff[wb.buff_cur] = '\0';
+
+  ASSERT_STREQ("abc", str);
+  ASSERT_EQ(writer.get_chars_written(), 3);
+}
+
+TEST(LlvmLibcPrintfWriterTest, WriteMultipleTimes) {
+  char str[10];
+  WriteBuffer wb(str, sizeof(str) - 1);
+  Writer writer(&wb);
+  writer.write({"abc", 3});
+  writer.write({"DEF", 3});
+  writer.write({"1234", 3});
+
+  wb.buff[wb.buff_cur] = '\0';
+
+  ASSERT_STREQ("abcDEF123", str);
+  ASSERT_EQ(writer.get_chars_written(), 9);
+}
+
+TEST(LlvmLibcPrintfWriterTest, WriteChars) {
+  char str[4] = {'D', 'E', 'F', 'G'};
+  WriteBuffer wb(str, sizeof(str) - 1);
+  Writer writer(&wb);
+  writer.write('a', 3);
+
+  EXPECT_EQ(str[3], 'G');
+  wb.buff[wb.buff_cur] = '\0';
+
+  ASSERT_STREQ("aaa", str);
+  ASSERT_EQ(writer.get_chars_written(), 3);
+}
+
+TEST(LlvmLibcPrintfWriterTest, WriteCharsMultipleTimes) {
+  char str[10];
+  WriteBuffer wb(str, sizeof(str) - 1);
+  Writer writer(&wb);
+  writer.write('a', 3);
+  writer.write('D', 3);
+  writer.write('1', 3);
+
+  wb.buff[wb.buff_cur] = '\0';
+
+  ASSERT_STREQ("aaaDDD111", str);
+  ASSERT_EQ(writer.get_chars_written(), 9);
+}
+
+TEST(LlvmLibcPrintfWriterTest, WriteManyChars) {
+  char str[100];
+  WriteBuffer wb(str, sizeof(str) - 1);
+  Writer writer(&wb);
+  writer.write('Z', 99);
+
+  wb.buff[wb.buff_cur] = '\0';
+
+  ASSERT_STREQ("ZZZZZZZZZZ"
+               "ZZZZZZZZZZ"
+               "ZZZZZZZZZZ"
+               "ZZZZZZZZZZ"
+               "ZZZZZZZZZZ"
+               "ZZZZZZZZZZ"
+               "ZZZZZZZZZZ"
+               "ZZZZZZZZZZ"
+               "ZZZZZZZZZZ"
+               "ZZZZZZZZZ",
+               str);
+  ASSERT_EQ(writer.get_chars_written(), 99);
+}
+
+TEST(LlvmLibcPrintfWriterTest, MixedWrites) {
+  char str[13];
+  WriteBuffer wb(str, sizeof(str) - 1);
+  Writer writer(&wb);
+  writer.write('a', 3);
+  writer.write({"DEF", 3});
+  writer.write('1', 3);
+  writer.write({"456", 3});
+
+  wb.buff[wb.buff_cur] = '\0';
+
+  ASSERT_STREQ("aaaDEF111456", str);
+  ASSERT_EQ(writer.get_chars_written(), 12);
+}
+
+TEST(LlvmLibcPrintfWriterTest, WriteWithMaxLength) {
+  char str[11];
+  WriteBuffer wb(str, sizeof(str) - 1);
+  Writer writer(&wb);
+  writer.write({"abcDEF123456", 12});
+
+  wb.buff[wb.buff_cur] = '\0';
+
+  ASSERT_STREQ("abcDEF1234", str);
+  ASSERT_EQ(writer.get_chars_written(), 12);
+}
+
+TEST(LlvmLibcPrintfWriterTest, WriteCharsWithMaxLength) {
+  char str[11];
+  WriteBuffer wb(str, sizeof(str) - 1);
+  Writer writer(&wb);
+  writer.write('1', 15);
+
+  wb.buff[wb.buff_cur] = '\0';
+
+  ASSERT_STREQ("1111111111", str);
+  ASSERT_EQ(writer.get_chars_written(), 15);
+}
+
+TEST(LlvmLibcPrintfWriterTest, MixedWriteWithMaxLength) {
+  char str[11];
+  WriteBuffer wb(str, sizeof(str) - 1);
+
+  Writer writer(&wb);
+  writer.write('a', 3);
+  writer.write({"DEF", 3});
+  writer.write('1', 3);
+  writer.write({"456", 3});
+
+  wb.buff[wb.buff_cur] = '\0';
+
+  ASSERT_STREQ("aaaDEF1114", str);
+  ASSERT_EQ(writer.get_chars_written(), 12);
+}
+
+TEST(LlvmLibcPrintfWriterTest, StringWithMaxLengthOne) {
+  char str[1];
+  // This is because the max length should be at most 1 less than the size of
+  // the buffer it's writing to.
+  WriteBuffer wb(str, 0);
+
+  Writer writer(&wb);
+  writer.write('a', 3);
+  writer.write({"DEF", 3});
+  writer.write('1', 3);
+  writer.write({"456", 3});
+
+  wb.buff[wb.buff_cur] = '\0';
+
+  ASSERT_STREQ("", str);
+  ASSERT_EQ(writer.get_chars_written(), 12);
+}
+
+TEST(LlvmLibcPrintfWriterTest, NullStringWithZeroMaxLength) {
+  WriteBuffer wb(nullptr, 0);
+
+  Writer writer(&wb);
+  writer.write('a', 3);
+  writer.write({"DEF", 3});
+  writer.write('1', 3);
+  writer.write({"456", 3});
+
+  ASSERT_EQ(writer.get_chars_written(), 12);
+}
+
+struct OutBuff {
+  char *out_str;
+  size_t cur_pos = 0;
+};
+
+int copy_to_out(string_view new_str, void *raw_out_buff) {
+  if (new_str.size() == 0) {
+    return 0;
+  }
+
+  OutBuff *out_buff = reinterpret_cast<OutBuff *>(raw_out_buff);
+
+  __llvm_libc::inline_memcpy(out_buff->out_str + out_buff->cur_pos,
+                             new_str.data(), new_str.size());
+
+  out_buff->cur_pos += new_str.size();
+  return 0;
+}
+
+TEST(LlvmLibcPrintfWriterTest, WriteWithMaxLengthWithCallback) {
+  char str[16];
+
+  OutBuff out_buff = {str, 0};
+
+  char wb_buff[8];
+  WriteBuffer wb(wb_buff, sizeof(wb_buff), &copy_to_out,
+                 reinterpret_cast<void *>(&out_buff));
+  Writer writer(&wb);
+  writer.write({"abcDEF123456", 12});
+
+  // Flush the buffer
+  wb.overflow_write("");
+  str[out_buff.cur_pos] = '\0';
+
+  ASSERT_STREQ("abcDEF123456", str);
+  ASSERT_EQ(writer.get_chars_written(), 12);
+}
+
+TEST(LlvmLibcPrintfWriterTest, WriteCharsWithMaxLengthWithCallback) {
+  char str[16];
+
+  OutBuff out_buff = {str, 0};
+
+  char wb_buff[8];
+  WriteBuffer wb(wb_buff, sizeof(wb_buff), &copy_to_out,
+                 reinterpret_cast<void *>(&out_buff));
+  Writer writer(&wb);
+  writer.write('1', 15);
+
+  // Flush the buffer
+  wb.overflow_write("");
+  str[out_buff.cur_pos] = '\0';
+
+  ASSERT_STREQ("111111111111111", str);
+  ASSERT_EQ(writer.get_chars_written(), 15);
+}
+
+TEST(LlvmLibcPrintfWriterTest, MixedWriteWithMaxLengthWithCallback) {
+  char str[16];
+
+  OutBuff out_buff = {str, 0};
+
+  char wb_buff[8];
+  WriteBuffer wb(wb_buff, sizeof(wb_buff), &copy_to_out,
+                 reinterpret_cast<void *>(&out_buff));
+  Writer writer(&wb);
+  writer.write('a', 3);
+  writer.write({"DEF", 3});
+  writer.write('1', 3);
+  writer.write({"456", 3});
+
+  // Flush the buffer
+  wb.overflow_write("");
+  str[out_buff.cur_pos] = '\0';
+
+  ASSERT_STREQ("aaaDEF111456", str);
+  ASSERT_EQ(writer.get_chars_written(), 12);
+}
+
+TEST(LlvmLibcPrintfWriterTest, ZeroLengthBufferWithCallback) {
+  char str[16];
+
+  OutBuff out_buff = {str, 0};
+
+  char wb_buff[1];
+  WriteBuffer wb(wb_buff, 0, &copy_to_out, reinterpret_cast<void *>(&out_buff));
+
+  Writer writer(&wb);
+  writer.write('a', 3);
+  writer.write({"DEF", 3});
+  writer.write('1', 3);
+  writer.write({"456", 3});
+
+  // Flush the buffer
+  wb.overflow_write("");
+  str[out_buff.cur_pos] = '\0';
+
+  ASSERT_STREQ("aaaDEF111456", str);
+  ASSERT_EQ(writer.get_chars_written(), 12);
+}
+
+TEST(LlvmLibcPrintfWriterTest, NullStringWithZeroMaxLengthWithCallback) {
+  char str[16];
+
+  OutBuff out_buff = {str, 0};
+
+  WriteBuffer wb(nullptr, 0, &copy_to_out, reinterpret_cast<void *>(&out_buff));
+
+  Writer writer(&wb);
+  writer.write('a', 3);
+  writer.write({"DEF", 3});
+  writer.write('1', 3);
+  writer.write({"456", 3});
+
+  wb.overflow_write("");
+  str[out_buff.cur_pos] = '\0';
+
+  ASSERT_EQ(writer.get_chars_written(), 12);
+  ASSERT_STREQ("aaaDEF111456", str);
+}

diff  --git a/libc/test/src/stdio/snprintf_test.cpp b/libc/test/src/stdio/snprintf_test.cpp
index 124264b5a1e8a7..764be9a9ba48b4 100644
--- a/libc/test/src/stdio/snprintf_test.cpp
+++ b/libc/test/src/stdio/snprintf_test.cpp
@@ -14,7 +14,7 @@
 // these tests will focus on snprintf exclusive features.
 
 TEST(LlvmLibcSNPrintfTest, CutOff) {
-  char buff[64];
+  char buff[100];
   int written;
 
   written =
@@ -26,6 +26,18 @@ TEST(LlvmLibcSNPrintfTest, CutOff) {
   EXPECT_EQ(written, 10);
   ASSERT_STREQ(buff, "1234");
 
+  written = __llvm_libc::snprintf(buff, 67, "%-101c", 'a');
+  EXPECT_EQ(written, 101);
+  ASSERT_STREQ(buff, "a "
+                     "        " // Each of these is 8 spaces, and there are 8.
+                     "        " // In total there are 65 spaces
+                     "        " // 'a' + 65 spaces + '\0' = 67
+                     "        "
+                     "        "
+                     "        "
+                     "        "
+                     "        ");
+
   // passing null as the output pointer is allowed as long as buffsz is 0.
   written = __llvm_libc::snprintf(nullptr, 0, "%s and more", "1234567890");
   EXPECT_EQ(written, 19);

diff  --git a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
index 52f4d42044c129..0ee3395260b5b6 100644
--- a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
@@ -2576,31 +2576,6 @@ libc_support_library(
     ],
 )
 
-libc_support_library(
-    name = "printf_string_writer",
-    srcs = ["src/stdio/printf_core/string_writer.cpp"],
-    hdrs = ["src/stdio/printf_core/string_writer.h"],
-    defines = PRINTF_COPTS,
-    deps = [
-        ":__support_cpp_string_view",
-        ":libc_root",
-        ":printf_core_structs",
-        ":string_memory_utils",
-    ],
-)
-
-libc_support_library(
-    name = "printf_file_writer",
-    hdrs = ["src/stdio/printf_core/file_writer.h"],
-    defines = PRINTF_COPTS,
-    deps = [
-        ":__support_cpp_string_view",
-        ":__support_file_file",
-        ":libc_root",
-        ":printf_core_structs",
-    ],
-)
-
 libc_support_library(
     name = "printf_writer",
     srcs = ["src/stdio/printf_core/writer.cpp"],
@@ -2608,7 +2583,10 @@ libc_support_library(
     defines = PRINTF_COPTS,
     deps = [
         ":__support_cpp_string_view",
+        ":__support_macros_optimization",
         ":libc_root",
+        ":printf_core_structs",
+        ":string_memory_utils",
     ],
 )
 
@@ -2671,9 +2649,9 @@ libc_function(
     defines = PRINTF_COPTS,
     deps = [
         ":__support_arg_list",
+        ":__support_cpp_limits",
         ":errno",
         ":printf_main",
-        ":printf_string_writer",
         ":printf_writer",
     ],
 )
@@ -2687,7 +2665,6 @@ libc_function(
         ":__support_arg_list",
         ":errno",
         ":printf_main",
-        ":printf_string_writer",
         ":printf_writer",
     ],
 )
@@ -2700,7 +2677,6 @@ libc_support_library(
         ":__support_arg_list",
         ":__support_file_file",
         ":__support_macros_attributes",
-        ":printf_file_writer",
         ":printf_main",
         ":printf_writer",
     ],
@@ -2713,6 +2689,7 @@ libc_function(
     defines = PRINTF_COPTS,
     deps = [
         ":__support_arg_list",
+        ":__support_file_file",
         ":errno",
         ":vfprintf_internal",
     ],
@@ -2725,6 +2702,7 @@ libc_function(
     defines = PRINTF_COPTS,
     deps = [
         ":__support_arg_list",
+        ":__support_file_file",
         ":errno",
         ":vfprintf_internal",
     ],

diff  --git a/utils/bazel/llvm-project-overlay/libc/test/src/stdio/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/test/src/stdio/BUILD.bazel
index 9277d6a31a4f39..46bef299013a2b 100644
--- a/utils/bazel/llvm-project-overlay/libc/test/src/stdio/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/libc/test/src/stdio/BUILD.bazel
@@ -26,16 +26,16 @@ libc_test(
 )
 
 libc_test(
-    name = "printf_string_writer_test",
-    srcs = ["printf_core/string_writer_test.cpp"],
+    name = "printf_writer_test",
+    srcs = ["printf_core/writer_test.cpp"],
     libc_function_deps = [
     ],
     deps = [
         "//libc:__support_arg_list",
         "//libc:__support_cpp_string_view",
         "//libc:printf_core_structs",
-        "//libc:printf_string_writer",
         "//libc:printf_writer",
+        "//libc:string_memory_utils",
     ],
 )
 
@@ -49,7 +49,6 @@ libc_test(
         "//libc:__support_cpp_string_view",
         "//libc:printf_converter",
         "//libc:printf_core_structs",
-        "//libc:printf_string_writer",
         "//libc:printf_writer",
     ],
 )


        


More information about the libc-commits mailing list