[libc-commits] [libc] 2e6eccf - [libc] refactor printf file writing

Michael Jones via libc-commits libc-commits at lists.llvm.org
Wed Jun 15 11:45:43 PDT 2022


Author: Michael Jones
Date: 2022-06-15T11:45:36-07:00
New Revision: 2e6eccfe34c1ff005352ca78f87bbcc4884f7371

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

LOG: [libc] refactor printf file writing

Add return values to converter functions to allow for better error
handling when writing files. Also move the file writing code around to
be easier to read.

Reviewed By: sivachandra

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

Added: 
    libc/src/stdio/printf_core/vfprintf_internal.cpp
    libc/src/stdio/printf_core/vfprintf_internal.h

Modified: 
    libc/src/stdio/CMakeLists.txt
    libc/src/stdio/fprintf.cpp
    libc/src/stdio/printf_core/CMakeLists.txt
    libc/src/stdio/printf_core/char_converter.h
    libc/src/stdio/printf_core/converter.cpp
    libc/src/stdio/printf_core/converter.h
    libc/src/stdio/printf_core/core_structs.h
    libc/src/stdio/printf_core/file_writer.cpp
    libc/src/stdio/printf_core/file_writer.h
    libc/src/stdio/printf_core/int_converter.h
    libc/src/stdio/printf_core/printf_main.cpp
    libc/src/stdio/printf_core/string_converter.h
    libc/src/stdio/printf_core/string_writer.cpp
    libc/src/stdio/printf_core/string_writer.h
    libc/src/stdio/printf_core/writer.cpp
    libc/src/stdio/printf_core/writer.h
    libc/test/src/stdio/fprintf_test.cpp
    libc/test/src/stdio/printf_core/converter_test.cpp
    libc/test/src/stdio/printf_core/string_writer_test.cpp

Removed: 
    


################################################################################
diff  --git a/libc/src/stdio/CMakeLists.txt b/libc/src/stdio/CMakeLists.txt
index 16fb58fb15b88..7432e0e3b1531 100644
--- a/libc/src/stdio/CMakeLists.txt
+++ b/libc/src/stdio/CMakeLists.txt
@@ -258,8 +258,6 @@ add_entrypoint_object(
   HDRS
     fprintf.h
   DEPENDS
-    libc.include.stdio
-    libc.src.stdio.printf_core.printf_main
-    libc.src.stdio.printf_core.file_writer
-    libc.src.stdio.printf_core.writer
+    libc.src.__support.arg_list
+    libc.src.stdio.printf_core.vfprintf_internal
 )

diff  --git a/libc/src/stdio/fprintf.cpp b/libc/src/stdio/fprintf.cpp
index 21451f5d5f5b0..796d5b5c47095 100644
--- a/libc/src/stdio/fprintf.cpp
+++ b/libc/src/stdio/fprintf.cpp
@@ -8,14 +8,11 @@
 
 #include "src/stdio/fprintf.h"
 
+#include "src/__support/File/file.h"
 #include "src/__support/arg_list.h"
-#include "src/stdio/ferror.h"
-#include "src/stdio/printf_core/file_writer.h"
-#include "src/stdio/printf_core/printf_main.h"
-#include "src/stdio/printf_core/writer.h"
+#include "src/stdio/printf_core/vfprintf_internal.h"
 
 #include <stdarg.h>
-#include <stdio.h>
 
 namespace __llvm_libc {
 
@@ -28,12 +25,7 @@ LLVM_LIBC_FUNCTION(int, fprintf,
                                  // and pointer semantics, as well as handling
                                  // destruction automatically.
   va_end(vlist);
-  printf_core::Writer writer(reinterpret_cast<void *>(stream),
-                             printf_core::write_to_file);
-
-  int ret_val = printf_core::printf_main(&writer, format, args);
-  if (__llvm_libc::ferror(stream))
-    return -1;
+  int ret_val = printf_core::vfprintf_internal(stream, format, args);
   return ret_val;
 }
 

diff  --git a/libc/src/stdio/printf_core/CMakeLists.txt b/libc/src/stdio/printf_core/CMakeLists.txt
index 2b814f072f1d9..71f7cbbb467da 100644
--- a/libc/src/stdio/printf_core/CMakeLists.txt
+++ b/libc/src/stdio/printf_core/CMakeLists.txt
@@ -79,3 +79,19 @@ add_object_library(
     .core_structs
     libc.src.__support.arg_list
 )
+
+add_object_library(
+  vfprintf_internal
+  SRCS
+    vfprintf_internal.cpp
+  HDRS
+    vfprintf_internal.h
+  DEPENDS
+    libc.include.stdio
+    libc.src.__support.File.file
+    libc.src.__support.File.platform_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/char_converter.h b/libc/src/stdio/printf_core/char_converter.h
index d294be937cb8a..bc74156b14491 100644
--- a/libc/src/stdio/printf_core/char_converter.h
+++ b/libc/src/stdio/printf_core/char_converter.h
@@ -15,21 +15,22 @@
 namespace __llvm_libc {
 namespace printf_core {
 
-void inline convert_char(Writer *writer, const FormatSection &to_conv) {
+int inline convert_char(Writer *writer, const FormatSection &to_conv) {
   char c = to_conv.conv_val_raw;
 
   if (to_conv.min_width > 1) {
     if ((to_conv.flags & FormatFlags::LEFT_JUSTIFIED) ==
         FormatFlags::LEFT_JUSTIFIED) {
-      writer->write(&c, 1);
-      writer->write_chars(' ', to_conv.min_width - 1);
+      RET_IF_RESULT_NEGATIVE(writer->write(&c, 1));
+      RET_IF_RESULT_NEGATIVE(writer->write_chars(' ', to_conv.min_width - 1));
     } else {
-      writer->write_chars(' ', to_conv.min_width - 1);
-      writer->write(&c, 1);
+      RET_IF_RESULT_NEGATIVE(writer->write_chars(' ', to_conv.min_width - 1));
+      RET_IF_RESULT_NEGATIVE(writer->write(&c, 1));
     }
   } else {
-    writer->write(&c, 1);
+    RET_IF_RESULT_NEGATIVE(writer->write(&c, 1));
   }
+  return 0;
 }
 
 } // namespace printf_core

diff  --git a/libc/src/stdio/printf_core/converter.cpp b/libc/src/stdio/printf_core/converter.cpp
index 65f0bfb39c9e7..90390424e826f 100644
--- a/libc/src/stdio/printf_core/converter.cpp
+++ b/libc/src/stdio/printf_core/converter.cpp
@@ -24,61 +24,48 @@
 namespace __llvm_libc {
 namespace printf_core {
 
-void convert(Writer *writer, const FormatSection &to_conv) {
-  if (!to_conv.has_conv) {
-    writer->write(to_conv.raw_string, to_conv.raw_len);
-    return;
-  }
+int convert(Writer *writer, const FormatSection &to_conv) {
+  if (!to_conv.has_conv)
+    return writer->write(to_conv.raw_string, to_conv.raw_len);
+
   switch (to_conv.conv_name) {
   case '%':
-    writer->write("%", 1);
-    return;
+    return writer->write("%", 1);
   case 'c':
-    convert_char(writer, to_conv);
-    return;
+    return convert_char(writer, to_conv);
   case 's':
-    convert_string(writer, to_conv);
-    return;
+    return convert_string(writer, to_conv);
   case 'd':
   case 'i':
   case 'u':
-    convert_int(writer, to_conv);
-    return;
+    return convert_int(writer, to_conv);
   case 'o':
-    // convert_oct(writer, to_conv);
-    return;
+    // return convert_oct(writer, to_conv);
   case 'x':
   case 'X':
-    // convert_hex(writer, to_conv);
-    return;
+    // return convert_hex(writer, to_conv);
   // TODO(michaelrj): add a flag to disable float point values here
   case 'f':
   case 'F':
-    // convert_float_decimal(writer, to_conv);
-    return;
+    // return convert_float_decimal(writer, to_conv);
   case 'e':
   case 'E':
-    // convert_float_dec_exp(writer, to_conv);
-    return;
+    // return convert_float_dec_exp(writer, to_conv);
   case 'a':
   case 'A':
-    // convert_float_hex_exp(writer, to_conv);
-    return;
+    // return convert_float_hex_exp(writer, to_conv);
   case 'g':
   case 'G':
-    // convert_float_mixed(writer, to_conv);
-    return;
+    // return convert_float_mixed(writer, to_conv);
   // TODO(michaelrj): add a flag to disable writing an int here
   case 'n':
-    // convert_write_int(writer, to_conv);
-    return;
+    // return convert_write_int(writer, to_conv);
   case 'p':
-    // convert_pointer(writer, to_conv);
-    return;
+    // return convert_pointer(writer, to_conv);
   default:
-    writer->write(to_conv.raw_string, to_conv.raw_len);
-    return;
+    return writer->write(to_conv.raw_string, to_conv.raw_len);
   }
+  return -1;
 }
 
 } // namespace printf_core

diff  --git a/libc/src/stdio/printf_core/converter.h b/libc/src/stdio/printf_core/converter.h
index 52f506e340d32..ae281ba99ec07 100644
--- a/libc/src/stdio/printf_core/converter.h
+++ b/libc/src/stdio/printf_core/converter.h
@@ -20,7 +20,7 @@ namespace printf_core {
 // convert will call a conversion function to convert the FormatSection into
 // its string representation, and then that will write the result to the
 // writer.
-void convert(Writer *writer, const FormatSection &to_conv);
+int convert(Writer *writer, const FormatSection &to_conv);
 
 } // namespace printf_core
 } // namespace __llvm_libc

diff  --git a/libc/src/stdio/printf_core/core_structs.h b/libc/src/stdio/printf_core/core_structs.h
index 80f187102bc10..93c92f1fcf3d7 100644
--- a/libc/src/stdio/printf_core/core_structs.h
+++ b/libc/src/stdio/printf_core/core_structs.h
@@ -79,6 +79,13 @@ struct FormatSection {
   }
 };
 
+#define RET_IF_RESULT_NEGATIVE(func)                                           \
+  {                                                                            \
+    int result = (func);                                                       \
+    if (result < 0)                                                            \
+      return result;                                                           \
+  }
+
 } // namespace printf_core
 } // namespace __llvm_libc
 

diff  --git a/libc/src/stdio/printf_core/file_writer.cpp b/libc/src/stdio/printf_core/file_writer.cpp
index 465e4cba57fbb..3f84dddaf275e 100644
--- a/libc/src/stdio/printf_core/file_writer.cpp
+++ b/libc/src/stdio/printf_core/file_writer.cpp
@@ -13,10 +13,19 @@
 namespace __llvm_libc {
 namespace printf_core {
 
-void write_to_file(void *raw_pointer, const char *__restrict to_write,
-                   size_t len) {
-  __llvm_libc::File *file = reinterpret_cast<__llvm_libc::File *>(raw_pointer);
-  file->write(to_write, len);
+int FileWriter::write(const char *__restrict to_write, size_t len) {
+  int written = file->write_unlocked(to_write, len);
+  if (written != len)
+    written = -1;
+  if (file->error_unlocked())
+    written = -2;
+  return written;
+}
+
+int write_to_file(void *raw_pointer, const char *__restrict to_write,
+                  size_t len) {
+  FileWriter *file_writer = reinterpret_cast<FileWriter *>(raw_pointer);
+  return file_writer->write(to_write, len);
 }
 
 } // namespace printf_core

diff  --git a/libc/src/stdio/printf_core/file_writer.h b/libc/src/stdio/printf_core/file_writer.h
index 755ed32a1b422..9e4eb331687f2 100644
--- a/libc/src/stdio/printf_core/file_writer.h
+++ b/libc/src/stdio/printf_core/file_writer.h
@@ -10,15 +10,31 @@
 #define LLVM_LIBC_SRC_STDIO_PRINTF_CORE_FILE_WRITER_H
 
 #include "src/__support/File/file.h"
+
 #include <stddef.h>
+#include <stdio.h>
 
 namespace __llvm_libc {
 namespace printf_core {
 
+class FileWriter {
+  __llvm_libc::File *file;
+
+public:
+  FileWriter(::FILE *init_file) {
+    file = reinterpret_cast<__llvm_libc::File *>(init_file);
+    file->lock();
+  }
+
+  ~FileWriter() { file->unlock(); }
+
+  int write(const char *__restrict to_write, size_t len);
+};
+
 // write_to_file treats raw_pointer as a File and calls its write
 // function.
-void write_to_file(void *raw_pointer, const char *__restrict to_write,
-                   size_t len);
+int write_to_file(void *raw_pointer, const char *__restrict to_write,
+                  size_t len);
 
 } // namespace printf_core
 } // namespace __llvm_libc

diff  --git a/libc/src/stdio/printf_core/int_converter.h b/libc/src/stdio/printf_core/int_converter.h
index 4b40d474cddec..ad2dad6b7284c 100644
--- a/libc/src/stdio/printf_core/int_converter.h
+++ b/libc/src/stdio/printf_core/int_converter.h
@@ -19,7 +19,7 @@
 namespace __llvm_libc {
 namespace printf_core {
 
-void inline convert_int(Writer *writer, const FormatSection &to_conv) {
+int inline convert_int(Writer *writer, const FormatSection &to_conv) {
   static constexpr size_t BITS_IN_BYTE = 8;
   static constexpr size_t BITS_IN_NUM = sizeof(uintmax_t) * BITS_IN_BYTE;
 
@@ -148,24 +148,25 @@ void inline convert_int(Writer *writer, const FormatSection &to_conv) {
   if ((flags & FormatFlags::LEFT_JUSTIFIED) == FormatFlags::LEFT_JUSTIFIED) {
     // If left justified it goes sign zeroes digits spaces
     if (sign_char != 0)
-      writer->write(&sign_char, 1);
+      RET_IF_RESULT_NEGATIVE(writer->write(&sign_char, 1));
     if (zeroes > 0)
-      writer->write_chars('0', zeroes);
+      RET_IF_RESULT_NEGATIVE(writer->write_chars('0', zeroes));
     if (digits_written > 0)
-      writer->write(buffer + buff_cur, digits_written);
+      RET_IF_RESULT_NEGATIVE(writer->write(buffer + buff_cur, digits_written));
     if (spaces > 0)
-      writer->write_chars(' ', spaces);
+      RET_IF_RESULT_NEGATIVE(writer->write_chars(' ', spaces));
   } else {
     // Else it goes spaces sign zeroes digits
     if (spaces > 0)
-      writer->write_chars(' ', spaces);
+      RET_IF_RESULT_NEGATIVE(writer->write_chars(' ', spaces));
     if (sign_char != 0)
-      writer->write(&sign_char, 1);
+      RET_IF_RESULT_NEGATIVE(writer->write(&sign_char, 1));
     if (zeroes > 0)
-      writer->write_chars('0', zeroes);
+      RET_IF_RESULT_NEGATIVE(writer->write_chars('0', zeroes));
     if (digits_written > 0)
-      writer->write(buffer + buff_cur, digits_written);
+      RET_IF_RESULT_NEGATIVE(writer->write(buffer + buff_cur, digits_written));
   }
+  return 0;
 }
 
 } // namespace printf_core

diff  --git a/libc/src/stdio/printf_core/printf_main.cpp b/libc/src/stdio/printf_core/printf_main.cpp
index 7ec046f458be4..fb46da46e074d 100644
--- a/libc/src/stdio/printf_core/printf_main.cpp
+++ b/libc/src/stdio/printf_core/printf_main.cpp
@@ -22,13 +22,16 @@ namespace printf_core {
 int printf_main(Writer *writer, const char *__restrict str,
                 internal::ArgList &args) {
   Parser parser(str, args);
-
+  int result = 0;
   for (FormatSection cur_section = parser.get_next_section();
        cur_section.raw_len > 0; cur_section = parser.get_next_section()) {
     if (cur_section.has_conv)
-      convert(writer, cur_section);
+      result = convert(writer, cur_section);
     else
-      writer->write(cur_section.raw_string, cur_section.raw_len);
+      result = writer->write(cur_section.raw_string, cur_section.raw_len);
+
+    if (result < 0)
+      return result;
   }
 
   return writer->get_chars_written();

diff  --git a/libc/src/stdio/printf_core/string_converter.h b/libc/src/stdio/printf_core/string_converter.h
index 3ca13ceb3e41d..888854d44853e 100644
--- a/libc/src/stdio/printf_core/string_converter.h
+++ b/libc/src/stdio/printf_core/string_converter.h
@@ -17,7 +17,7 @@
 namespace __llvm_libc {
 namespace printf_core {
 
-void inline convert_string(Writer *writer, const FormatSection &to_conv) {
+int inline convert_string(Writer *writer, const FormatSection &to_conv) {
   int string_len = 0;
 
   for (char *cur_str = reinterpret_cast<char *>(to_conv.conv_val_ptr);
@@ -31,18 +31,22 @@ void inline convert_string(Writer *writer, const FormatSection &to_conv) {
   if (to_conv.min_width > string_len) {
     if ((to_conv.flags & FormatFlags::LEFT_JUSTIFIED) ==
         FormatFlags::LEFT_JUSTIFIED) {
-      writer->write(reinterpret_cast<const char *>(to_conv.conv_val_ptr),
-                    string_len);
-      writer->write_chars(' ', to_conv.min_width - string_len);
+      RET_IF_RESULT_NEGATIVE(writer->write(
+          reinterpret_cast<const char *>(to_conv.conv_val_ptr), string_len));
+      RET_IF_RESULT_NEGATIVE(
+          writer->write_chars(' ', to_conv.min_width - string_len));
+
     } else {
-      writer->write_chars(' ', to_conv.min_width - string_len);
-      writer->write(reinterpret_cast<const char *>(to_conv.conv_val_ptr),
-                    string_len);
+      RET_IF_RESULT_NEGATIVE(
+          writer->write_chars(' ', to_conv.min_width - string_len));
+      RET_IF_RESULT_NEGATIVE(writer->write(
+          reinterpret_cast<const char *>(to_conv.conv_val_ptr), string_len));
     }
   } else {
-    writer->write(reinterpret_cast<const char *>(to_conv.conv_val_ptr),
-                  string_len);
+    RET_IF_RESULT_NEGATIVE(writer->write(
+        reinterpret_cast<const char *>(to_conv.conv_val_ptr), string_len));
   }
+  return 0;
 }
 
 } // namespace printf_core

diff  --git a/libc/src/stdio/printf_core/string_writer.cpp b/libc/src/stdio/printf_core/string_writer.cpp
index b53c2cc4a93eb..44160d821b8fc 100644
--- a/libc/src/stdio/printf_core/string_writer.cpp
+++ b/libc/src/stdio/printf_core/string_writer.cpp
@@ -24,10 +24,11 @@ void StringWriter::write(const char *__restrict to_write, size_t len) {
   }
 }
 
-void write_to_string(void *raw_pointer, const char *__restrict to_write,
-                     size_t len) {
+int write_to_string(void *raw_pointer, const char *__restrict to_write,
+                    size_t len) {
   StringWriter *string_writer = reinterpret_cast<StringWriter *>(raw_pointer);
   string_writer->write(to_write, len);
+  return 0;
 }
 
 } // namespace printf_core

diff  --git a/libc/src/stdio/printf_core/string_writer.h b/libc/src/stdio/printf_core/string_writer.h
index 93b31371f3bae..bcf040b9b16f5 100644
--- a/libc/src/stdio/printf_core/string_writer.h
+++ b/libc/src/stdio/printf_core/string_writer.h
@@ -36,8 +36,8 @@ class StringWriter {
 
 // write_to_string treats raw_pointer as a StringWriter and calls its write
 // function.
-void write_to_string(void *raw_pointer, const char *__restrict to_write,
-                     size_t len);
+int write_to_string(void *raw_pointer, const char *__restrict to_write,
+                    size_t len);
 
 } // namespace printf_core
 } // namespace __llvm_libc

diff  --git a/libc/src/stdio/printf_core/vfprintf_internal.cpp b/libc/src/stdio/printf_core/vfprintf_internal.cpp
new file mode 100644
index 0000000000000..479bf1acc6260
--- /dev/null
+++ b/libc/src/stdio/printf_core/vfprintf_internal.cpp
@@ -0,0 +1,30 @@
+//===-- Internal implementation of vfprintf ---------------------*- 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/vfprintf_internal.h"
+
+#include "src/__support/arg_list.h"
+#include "src/stdio/printf_core/file_writer.h"
+#include "src/stdio/printf_core/printf_main.h"
+#include "src/stdio/printf_core/writer.h"
+
+#include <stdio.h>
+
+namespace __llvm_libc {
+namespace printf_core {
+
+int vfprintf_internal(::FILE *__restrict stream, const char *__restrict format,
+                      internal::ArgList &args) {
+  FileWriter file_writer(stream);
+  printf_core::Writer writer(reinterpret_cast<void *>(&file_writer),
+                             printf_core::write_to_file);
+  return printf_core::printf_main(&writer, format, args);
+}
+
+} // namespace printf_core
+} // namespace __llvm_libc

diff  --git a/libc/src/stdio/printf_core/vfprintf_internal.h b/libc/src/stdio/printf_core/vfprintf_internal.h
new file mode 100644
index 0000000000000..b837ebba182b4
--- /dev/null
+++ b/libc/src/stdio/printf_core/vfprintf_internal.h
@@ -0,0 +1,24 @@
+//===-- Internal implementation header of vfprintf --------------*- 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_VFPRINTF_INTERNAL_H
+#define LLVM_LIBC_SRC_STDIO_PRINTF_CORE_VFPRINTF_INTERNAL_H
+
+#include "src/__support/arg_list.h"
+
+#include <stdio.h>
+
+namespace __llvm_libc {
+namespace printf_core {
+
+int vfprintf_internal(::FILE *__restrict stream, const char *__restrict format,
+                      internal::ArgList &args);
+} // namespace printf_core
+} // namespace __llvm_libc
+
+#endif // LLVM_LIBC_SRC_STDIO_PRINTF_CORE_VFPRINTF_INTERNAL_H

diff  --git a/libc/src/stdio/printf_core/writer.cpp b/libc/src/stdio/printf_core/writer.cpp
index 8f5e5167142e3..fe48414837189 100644
--- a/libc/src/stdio/printf_core/writer.cpp
+++ b/libc/src/stdio/printf_core/writer.cpp
@@ -13,24 +13,23 @@
 namespace __llvm_libc {
 namespace printf_core {
 
-void Writer::write(const char *new_string, size_t length) {
-
-  raw_write(output, new_string, length);
-
-  // chars_written tracks the number of chars that would have been written
-  // regardless of what the raw_write call does.
+int Writer::write(const char *new_string, size_t length) {
   chars_written += length;
+  return raw_write(output, new_string, length);
 }
 
-void Writer::write_chars(char new_char, size_t length) {
+int Writer::write_chars(char new_char, size_t length) {
   constexpr size_t BUFF_SIZE = 8;
   char buff[BUFF_SIZE];
+  int result;
   inline_memset(buff, new_char, BUFF_SIZE);
   while (length > BUFF_SIZE) {
-    write(buff, BUFF_SIZE);
+    result = write(buff, BUFF_SIZE);
+    if (result < 0)
+      return result;
     length -= BUFF_SIZE;
   }
-  write(buff, length);
+  return write(buff, length);
 }
 
 } // namespace printf_core

diff  --git a/libc/src/stdio/printf_core/writer.h b/libc/src/stdio/printf_core/writer.h
index 0ad37e492eb28..c8a9f7d7e8786 100644
--- a/libc/src/stdio/printf_core/writer.h
+++ b/libc/src/stdio/printf_core/writer.h
@@ -14,7 +14,7 @@
 namespace __llvm_libc {
 namespace printf_core {
 
-using WriteFunc = void (*)(void *, const char *__restrict, size_t);
+using WriteFunc = int (*)(void *, const char *__restrict, size_t);
 
 class Writer final {
   // output is a pointer to the string or file that the writer is meant to write
@@ -23,26 +23,27 @@ class Writer final {
 
   // 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.
+  // onto the end of output. It should return a positive number or zero on
+  // success, or a negative number on failure.
   WriteFunc raw_write;
 
-  unsigned long long chars_written = 0;
+  int chars_written = 0;
 
 public:
   Writer(void *init_output, WriteFunc init_raw_write)
       : output(init_output), raw_write(init_raw_write) {}
 
   // write will copy length bytes from new_string into output using
-  // raw_write, unless that would cause more bytes than max_length to be
-  // written. It always increments chars_written by length.
-  void write(const char *new_string, size_t length);
+  // raw_write. It increments chars_written by length always. It returns the
+  // result of raw_write.
+  int write(const char *new_string, size_t length);
 
-  // write_chars will copy length copies of new_char into output using raw_write
-  // unless that would cause more bytes than max_length to be written. It always
-  // increments chars_written by length.
-  void write_chars(char new_char, size_t length);
+  // write_chars will copy length copies of new_char into output using the write
+  // function and a statically sized buffer. This is primarily used for padding.
+  // If write returns a negative value, this will return early with that value.
+  int write_chars(char new_char, size_t length);
 
-  unsigned long long get_chars_written() { return chars_written; }
+  int get_chars_written() { return chars_written; }
 };
 
 } // namespace printf_core

diff  --git a/libc/test/src/stdio/fprintf_test.cpp b/libc/test/src/stdio/fprintf_test.cpp
index c7bf2cc1525d2..84f1c316af822 100644
--- a/libc/test/src/stdio/fprintf_test.cpp
+++ b/libc/test/src/stdio/fprintf_test.cpp
@@ -62,7 +62,7 @@ TEST(LlvmLibcFPrintfTest, WriteToFile) {
 
   written =
       __llvm_libc::fprintf(file, "Writing to a read only file should fail.");
-  EXPECT_EQ(written, -1);
+  EXPECT_LT(written, 0);
 
   ASSERT_EQ(__llvm_libc::fclose(file), 0);
 }

diff  --git a/libc/test/src/stdio/printf_core/converter_test.cpp b/libc/test/src/stdio/printf_core/converter_test.cpp
index b3bc5e53817b7..2e95d60d71a9e 100644
--- a/libc/test/src/stdio/printf_core/converter_test.cpp
+++ b/libc/test/src/stdio/printf_core/converter_test.cpp
@@ -37,7 +37,7 @@ TEST_F(LlvmLibcPrintfConverterTest, SimpleRawConversion) {
   str_writer.terminate();
 
   ASSERT_STREQ(str, "abc");
-  ASSERT_EQ(writer.get_chars_written(), 3ull);
+  ASSERT_EQ(writer.get_chars_written(), 3);
 }
 
 TEST_F(LlvmLibcPrintfConverterTest, PercentConversion) {
@@ -51,7 +51,7 @@ TEST_F(LlvmLibcPrintfConverterTest, PercentConversion) {
   str[1] = '\0';
 
   ASSERT_STREQ(str, "%");
-  ASSERT_EQ(writer.get_chars_written(), 1ull);
+  ASSERT_EQ(writer.get_chars_written(), 1);
 }
 
 TEST_F(LlvmLibcPrintfConverterTest, CharConversionSimple) {
@@ -69,7 +69,7 @@ TEST_F(LlvmLibcPrintfConverterTest, CharConversionSimple) {
   str_writer.terminate();
 
   ASSERT_STREQ(str, "D");
-  ASSERT_EQ(writer.get_chars_written(), 1ull);
+  ASSERT_EQ(writer.get_chars_written(), 1);
 }
 
 TEST_F(LlvmLibcPrintfConverterTest, CharConversionRightJustified) {
@@ -84,7 +84,7 @@ TEST_F(LlvmLibcPrintfConverterTest, CharConversionRightJustified) {
   str_writer.terminate();
 
   ASSERT_STREQ(str, "   E");
-  ASSERT_EQ(writer.get_chars_written(), 4ull);
+  ASSERT_EQ(writer.get_chars_written(), 4);
 }
 
 TEST_F(LlvmLibcPrintfConverterTest, CharConversionLeftJustified) {
@@ -101,7 +101,7 @@ TEST_F(LlvmLibcPrintfConverterTest, CharConversionLeftJustified) {
   str_writer.terminate();
 
   ASSERT_STREQ(str, "F   ");
-  ASSERT_EQ(writer.get_chars_written(), 4ull);
+  ASSERT_EQ(writer.get_chars_written(), 4);
 }
 
 TEST_F(LlvmLibcPrintfConverterTest, StringConversionSimple) {
@@ -117,7 +117,7 @@ TEST_F(LlvmLibcPrintfConverterTest, StringConversionSimple) {
   str_writer.terminate();
 
   ASSERT_STREQ(str, "DEF");
-  ASSERT_EQ(writer.get_chars_written(), 3ull);
+  ASSERT_EQ(writer.get_chars_written(), 3);
 }
 
 TEST_F(LlvmLibcPrintfConverterTest, StringConversionPrecisionHigh) {
@@ -132,7 +132,7 @@ TEST_F(LlvmLibcPrintfConverterTest, StringConversionPrecisionHigh) {
   str_writer.terminate();
 
   ASSERT_STREQ(str, "456");
-  ASSERT_EQ(writer.get_chars_written(), 3ull);
+  ASSERT_EQ(writer.get_chars_written(), 3);
 }
 
 TEST_F(LlvmLibcPrintfConverterTest, StringConversionPrecisionLow) {
@@ -147,7 +147,7 @@ TEST_F(LlvmLibcPrintfConverterTest, StringConversionPrecisionLow) {
   str_writer.terminate();
 
   ASSERT_STREQ(str, "xy");
-  ASSERT_EQ(writer.get_chars_written(), 2ull);
+  ASSERT_EQ(writer.get_chars_written(), 2);
 }
 
 TEST_F(LlvmLibcPrintfConverterTest, StringConversionRightJustified) {
@@ -162,7 +162,7 @@ TEST_F(LlvmLibcPrintfConverterTest, StringConversionRightJustified) {
   str_writer.terminate();
 
   ASSERT_STREQ(str, " 789");
-  ASSERT_EQ(writer.get_chars_written(), 4ull);
+  ASSERT_EQ(writer.get_chars_written(), 4);
 }
 
 TEST_F(LlvmLibcPrintfConverterTest, StringConversionLeftJustified) {
@@ -179,7 +179,7 @@ TEST_F(LlvmLibcPrintfConverterTest, StringConversionLeftJustified) {
   str_writer.terminate();
 
   ASSERT_STREQ(str, "ghi ");
-  ASSERT_EQ(writer.get_chars_written(), 4ull);
+  ASSERT_EQ(writer.get_chars_written(), 4);
 }
 
 TEST_F(LlvmLibcPrintfConverterTest, IntConversionSimple) {
@@ -193,5 +193,5 @@ TEST_F(LlvmLibcPrintfConverterTest, IntConversionSimple) {
   str_writer.terminate();
 
   ASSERT_STREQ(str, "12345");
-  ASSERT_EQ(writer.get_chars_written(), 5ull);
+  ASSERT_EQ(writer.get_chars_written(), 5);
 }

diff  --git a/libc/test/src/stdio/printf_core/string_writer_test.cpp b/libc/test/src/stdio/printf_core/string_writer_test.cpp
index 6701a23b3f374..12a6e93705b9f 100644
--- a/libc/test/src/stdio/printf_core/string_writer_test.cpp
+++ b/libc/test/src/stdio/printf_core/string_writer_test.cpp
@@ -36,7 +36,7 @@ TEST(LlvmLibcPrintfStringWriterTest, Write) {
   str_writer.terminate();
 
   ASSERT_STREQ("abc", str);
-  ASSERT_EQ(writer.get_chars_written(), 3ull);
+  ASSERT_EQ(writer.get_chars_written(), 3);
 }
 
 TEST(LlvmLibcPrintfStringWriterTest, WriteMultipleTimes) {
@@ -52,7 +52,7 @@ TEST(LlvmLibcPrintfStringWriterTest, WriteMultipleTimes) {
   str_writer.terminate();
 
   ASSERT_STREQ("abcDEF123", str);
-  ASSERT_EQ(writer.get_chars_written(), 9ull);
+  ASSERT_EQ(writer.get_chars_written(), 9);
 }
 
 TEST(LlvmLibcPrintfStringWriterTest, WriteChars) {
@@ -67,7 +67,7 @@ TEST(LlvmLibcPrintfStringWriterTest, WriteChars) {
   str_writer.terminate();
 
   ASSERT_STREQ("aaa", str);
-  ASSERT_EQ(writer.get_chars_written(), 3ull);
+  ASSERT_EQ(writer.get_chars_written(), 3);
 }
 
 TEST(LlvmLibcPrintfStringWriterTest, WriteCharsMultipleTimes) {
@@ -83,7 +83,7 @@ TEST(LlvmLibcPrintfStringWriterTest, WriteCharsMultipleTimes) {
   str_writer.terminate();
 
   ASSERT_STREQ("aaaDDD111", str);
-  ASSERT_EQ(writer.get_chars_written(), 9ull);
+  ASSERT_EQ(writer.get_chars_written(), 9);
 }
 
 TEST(LlvmLibcPrintfStringWriterTest, WriteManyChars) {
@@ -107,7 +107,7 @@ TEST(LlvmLibcPrintfStringWriterTest, WriteManyChars) {
                "ZZZZZZZZZZ"
                "ZZZZZZZZZ",
                str);
-  ASSERT_EQ(writer.get_chars_written(), 99ull);
+  ASSERT_EQ(writer.get_chars_written(), 99);
 }
 
 TEST(LlvmLibcPrintfStringWriterTest, MixedWrites) {
@@ -124,7 +124,7 @@ TEST(LlvmLibcPrintfStringWriterTest, MixedWrites) {
   str_writer.terminate();
 
   ASSERT_STREQ("aaaDEF111456", str);
-  ASSERT_EQ(writer.get_chars_written(), 12ull);
+  ASSERT_EQ(writer.get_chars_written(), 12);
 }
 
 TEST(LlvmLibcPrintfStringWriterTest, WriteWithMaxLength) {
@@ -138,7 +138,7 @@ TEST(LlvmLibcPrintfStringWriterTest, WriteWithMaxLength) {
   str_writer.terminate();
 
   ASSERT_STREQ("abcDEF1234", str);
-  ASSERT_EQ(writer.get_chars_written(), 12ull);
+  ASSERT_EQ(writer.get_chars_written(), 12);
 }
 
 TEST(LlvmLibcPrintfStringWriterTest, WriteCharsWithMaxLength) {
@@ -153,7 +153,7 @@ TEST(LlvmLibcPrintfStringWriterTest, WriteCharsWithMaxLength) {
   str_writer.terminate();
 
   ASSERT_STREQ("1111111111", str);
-  ASSERT_EQ(writer.get_chars_written(), 15ull);
+  ASSERT_EQ(writer.get_chars_written(), 15);
 }
 
 TEST(LlvmLibcPrintfStringWriterTest, MixedWriteWithMaxLength) {
@@ -170,7 +170,7 @@ TEST(LlvmLibcPrintfStringWriterTest, MixedWriteWithMaxLength) {
   str_writer.terminate();
 
   ASSERT_STREQ("aaaDEF1114", str);
-  ASSERT_EQ(writer.get_chars_written(), 12ull);
+  ASSERT_EQ(writer.get_chars_written(), 12);
 }
 
 TEST(LlvmLibcPrintfStringWriterTest, StringWithMaxLengthOne) {
@@ -189,7 +189,7 @@ TEST(LlvmLibcPrintfStringWriterTest, StringWithMaxLengthOne) {
   str_writer.terminate();
 
   ASSERT_STREQ("", str);
-  ASSERT_EQ(writer.get_chars_written(), 12ull);
+  ASSERT_EQ(writer.get_chars_written(), 12);
 }
 
 TEST(LlvmLibcPrintfStringWriterTest, NullStringWithZeroMaxLength) {
@@ -202,5 +202,5 @@ TEST(LlvmLibcPrintfStringWriterTest, NullStringWithZeroMaxLength) {
   writer.write_chars('1', 3);
   writer.write("456", 3);
 
-  ASSERT_EQ(writer.get_chars_written(), 12ull);
+  ASSERT_EQ(writer.get_chars_written(), 12);
 }


        


More information about the libc-commits mailing list