[libc] refactor printf file writing

[michaelrj]

Summary

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.

[sivachandra]

Nit: You should probably keep the addition of vfprintf_internal in the other patch.

libc/src/stdio/printf_core/char_converter.h

22

Nit: If you make this macro take an argument, you can use like this:

RETURN_ON_ERROR(writer->write(...))

libc/src/stdio/printf_core/writer.cpp

21

Is this required now?

libc/src/stdio/printf_core/writer.h

32

Do we need this to be a signed integer now?

[sivachandra]

libc/src/stdio/printf_core/file_writer.h

24

Something I realized while reviewing the other patch: why should this internal class take a FILE* arg only to cast to the internal file data structure?

[michaelrj]

address comments and move macro to a shared location

libc/src/stdio/printf_core/file_writer.h

24

we have to cast it somewhere, and doing it inside vfprintf_internal means that we don't have to cast it in both fprintf and printf (and potentially also the v variants of those functions if those are added).

libc/src/stdio/printf_core/writer.h

32

It doesn't need to be an int, but it does make sense. Printf always returns an int, and that int is supposed to be the number of characters written, so storing the number of characters written in anything bigger doesn't really help.

[sivachandra]

libc/src/stdio/printf_core/core_structs.h

84 ↗

(On Diff #437252)

Nit: Put func in parenthesis: (func).

[michaelrj]

fix nit before land

libc/src/stdio/CMakeLists.txt

 
 add_entrypoint_object(
   fprintf
   SRCS
     fprintf.cpp
   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
 )

libc/src/stdio/fprintf.cpp

 //===-- Implementation of fprintf -------------------------------*- 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/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 {
 
 LLVM_LIBC_FUNCTION(int, fprintf,
                    (::FILE *__restrict stream, const char *__restrict format,
                     ...)) {
   va_list vlist;
   va_start(vlist, format);
   internal::ArgList args(vlist); // This holder class allows for easier copying
                                  // 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;
 }
 
 } // namespace __llvm_libc

libc/src/stdio/printf_core/CMakeLists.txt

HDRS
     printf_main.h
   DEPENDS
     .parser
     .converter
     .writer
     .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
+)

libc/src/stdio/printf_core/char_converter.h

 #define LLVM_LIBC_SRC_STDIO_PRINTF_CORE_CHAR_CONVERTER_H
 
 #include "src/stdio/printf_core/core_structs.h"
 #include "src/stdio/printf_core/writer.h"
 
 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;
 
+  int result = 0;
+#define RET_IF_RESULT_NEGATIVE()                                               \
+  if (result < 0)                                                              \
+    return result;
+
   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);
+      result = writer->write(&c, 1);
+      RET_IF_RESULT_NEGATIVE();
+      result = writer->write_chars(' ', to_conv.min_width - 1);
     } else {
-      writer->write_chars(' ', to_conv.min_width - 1);
-      writer->write(&c, 1);
+      result = writer->write_chars(' ', to_conv.min_width - 1);
+      RET_IF_RESULT_NEGATIVE();
+      result = writer->write(&c, 1);
     }
   } else {
-    writer->write(&c, 1);
+    result = writer->write(&c, 1);
   }
+  return result;
 }
 
 } // namespace printf_core
 } // namespace __llvm_libc
 
 #endif // LLVM_LIBC_SRC_STDIO_PRINTF_CORE_CHAR_CONVERTER_H

libc/src/stdio/printf_core/converter.h

 #include <stddef.h>
 
 namespace __llvm_libc {
 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
 
 #endif // LLVM_LIBC_SRC_STDIO_PRINTF_CORE_CONVERTER_H

libc/src/stdio/printf_core/converter.cpp

 #include LLVM_LIBC_PRINTF_CONV_ATLAS
 #endif
 
 #include <stddef.h>
 
 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
 } // namespace __llvm_libc

libc/src/stdio/printf_core/file_writer.h

 //===-- 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/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,
 
 } // namespace printf_core
 } // namespace __llvm_libc
 
 #endif // LLVM_LIBC_SRC_STDIO_PRINTF_CORE_FILE_WRITER_H

libc/src/stdio/printf_core/file_writer.cpp

 //===-- FILE 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/file_writer.h"
 #include "src/__support/File/file.h"
 #include <stddef.h>
 
 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,
+  FileWriter *file_writer = reinterpret_cast<FileWriter *>(raw_pointer);
+  return file_writer->write(to_write, len);
 }
 
 } // namespace printf_core
 } // namespace __llvm_libc

libc/src/stdio/printf_core/int_converter.h

 #include "src/stdio/printf_core/writer.h"
 
 #include <inttypes.h>
 #include <stddef.h>
 
 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;
 
   // This approximates the number of digits it takes to represent an integer of
   // a certain number of bits. The calculation is floor((bits * 5) / 16)
   // 32 -> 10 (actually needs 10)
   // 64 -> 20 (actually needs 20)
   // 128 -> 40 (actually needs 39)
if (to_conv.precision < 0) {
     zeroes = to_conv.precision - digits_written; // a negative value means 0
     if (zeroes < 0)
       zeroes = 0;
     spaces = to_conv.min_width - zeroes - digits_written - sign_char_len;
   }
   if (spaces < 0)
     spaces = 0;
 
+  int result = 0;
+#define RET_IF_RESULT_NEGATIVE()                                               \
+  if (result < 0)                                                              \
+    return result;
+
   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);
-    if (zeroes > 0)
-      writer->write_chars('0', zeroes);
-    if (digits_written > 0)
-      writer->write(buffer + buff_cur, digits_written);
-    if (spaces > 0)
-      writer->write_chars(' ', spaces);
+    if (sign_char != 0) {
+      result = writer->write(&sign_char, 1);
+      RET_IF_RESULT_NEGATIVE();
+    }
+    if (zeroes > 0) {
+      result = writer->write_chars('0', zeroes);
+      RET_IF_RESULT_NEGATIVE();
+    }
+    if (digits_written > 0) {
+      result = writer->write(buffer + buff_cur, digits_written);
+      RET_IF_RESULT_NEGATIVE();
+    }
+    if (spaces > 0) {
+      result = writer->write_chars(' ', spaces);
+    }
+
   } else {
     // Else it goes spaces sign zeroes digits
-    if (spaces > 0)
-      writer->write_chars(' ', spaces);
-    if (sign_char != 0)
-      writer->write(&sign_char, 1);
-    if (zeroes > 0)
-      writer->write_chars('0', zeroes);
-    if (digits_written > 0)
-      writer->write(buffer + buff_cur, digits_written);
+    if (spaces > 0) {
+      result = writer->write_chars(' ', spaces);
+      RET_IF_RESULT_NEGATIVE();
+    }
+    if (sign_char != 0) {
+      result = writer->write(&sign_char, 1);
+      RET_IF_RESULT_NEGATIVE();
+    }
+    if (zeroes > 0) {
+      result = writer->write_chars('0', zeroes);
+      RET_IF_RESULT_NEGATIVE();
+    }
+    if (digits_written > 0) {
+      result = writer->write(buffer + buff_cur, digits_written);
+    }
   }
+  return result;
 }
 
 } // namespace printf_core
 } // namespace __llvm_libc
 
 #endif // LLVM_LIBC_SRC_STDIO_PRINTF_CORE_INT_CONVERTER_H

libc/src/stdio/printf_core/printf_main.cpp

 #include <stddef.h>
 
 namespace __llvm_libc {
 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();
 }
 
 } // namespace printf_core
 } // namespace __llvm_libc

libc/src/stdio/printf_core/string_converter.h

 #include "src/stdio/printf_core/core_structs.h"
 #include "src/stdio/printf_core/writer.h"
 
 #include <stddef.h>
 
 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);
        cur_str[string_len]; ++string_len) {
     ;
   }
 
   if (to_conv.precision >= 0 && to_conv.precision < string_len)
     string_len = to_conv.precision;
 
+  int result = 0;
+#define RET_IF_RESULT_NEGATIVE()                                               \
+  if (result < 0)                                                              \
+    return result;
   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);
+      result = writer->write(
+          reinterpret_cast<const char *>(to_conv.conv_val_ptr), string_len);
+      RET_IF_RESULT_NEGATIVE();
+      result = 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);
+      result = writer->write_chars(' ', to_conv.min_width - string_len);
+      RET_IF_RESULT_NEGATIVE();
+      result = 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),
+    result = writer->write(reinterpret_cast<const char *>(to_conv.conv_val_ptr),
                            string_len);
   }
+  return result;
 }
 
 } // namespace printf_core
 } // namespace __llvm_libc
 
 #endif // LLVM_LIBC_SRC_STDIO_PRINTF_CORE_STRING_CONVERTER_H

libc/src/stdio/printf_core/string_writer.h

public:
   // 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'; }
 };
 
 // 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,
 
 } // namespace printf_core
 } // namespace __llvm_libc
 
 #endif // LLVM_LIBC_SRC_STDIO_PRINTF_CORE_STRING_WRITER_H

libc/src/stdio/printf_core/string_writer.cpp

void StringWriter::write(const char *__restrict to_write, size_t len) {
 
   if (len > 0) {
     inline_memcpy(cur_buffer, to_write, len);
     cur_buffer += len;
     available_capacity -= 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,
   StringWriter *string_writer = reinterpret_cast<StringWriter *>(raw_pointer);
   string_writer->write(to_write, len);
+  return 0;
 }
 
 } // namespace printf_core
 } // namespace __llvm_libc

libc/src/stdio/printf_core/vfprintf_internal.h

+//===-- 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

libc/src/stdio/printf_core/vfprintf_internal.cpp

+//===-- 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

libc/src/stdio/printf_core/writer.h

 //===-- 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_WRITER_H
 #define LLVM_LIBC_SRC_STDIO_PRINTF_CORE_WRITER_H
 
 #include <stddef.h>
 
 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
   // 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.
+  // onto the end of output. It should return a positive number on success, or a
+  // negative number on failure. Once a negative number is returned the value of
+  // chars_written will be set to it and it will not be updated unless another
+  // negative number is passed.
   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);
-
-  // 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);
+  // raw_write. It increments chars_written by length unless raw_write returns a
+  // negative value. 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 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
 } // namespace __llvm_libc
 
 #endif // LLVM_LIBC_SRC_STDIO_PRINTF_CORE_WRITER_H

libc/src/stdio/printf_core/writer.cpp

 //===-- 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
 //
 //===----------------------------------------------------------------------===//
 
 #include "writer.h"
 #include "src/string/memory_utils/memset_implementations.h"
 #include <stddef.h>
 
 namespace __llvm_libc {
 namespace printf_core {
 
-void Writer::write(const char *new_string, size_t length) {
+int Writer::write(const char *new_string, size_t length) {
 
-  raw_write(output, new_string, length);
+  int result = 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.
+  if (result < 0) {
+    chars_written = result;
+  } else if (chars_written >= 0) {
     chars_written += length;
   }
+  return result;
+}
 
-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
 } // namespace __llvm_libc

libc/test/src/stdio/fprintf_test.cpp

ASSERT_EQ(__llvm_libc::fread(
             sizeof(format_more) + sizeof(short_numbers) - 4);
   data[sizeof(format_more) + sizeof(short_numbers) - 4] = '\0';
   ASSERT_STREQ(data, "1234 and more\n");
 
   ASSERT_EQ(__llvm_libc::ferror(file), 0);
 
   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);
 }

libc/test/src/stdio/printf_core/converter_test.cpp

TEST_F(LlvmLibcPrintfConverterTest, SimpleRawConversion) {
   raw_section.raw_string = "abc";
   raw_section.raw_len = 3;
 
   __llvm_libc::printf_core::convert(&writer, raw_section);
 
   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) {
   __llvm_libc::printf_core::FormatSection simple_conv;
   simple_conv.has_conv = true;
   simple_conv.raw_string = "%%";
   simple_conv.conv_name = '%';
 
   __llvm_libc::printf_core::convert(&writer, simple_conv);
 
   str[1] = '\0';
 
   ASSERT_STREQ(str, "%");
-  ASSERT_EQ(writer.get_chars_written(), 1ull);
+  ASSERT_EQ(writer.get_chars_written(), 1);
 }
 
 TEST_F(LlvmLibcPrintfConverterTest, CharConversionSimple) {
   __llvm_libc::printf_core::FormatSection simple_conv;
   simple_conv.has_conv = true;
   // If has_conv is true, the raw string is ignored. They are not being parsed
   // and match the actual conversion taking place so that you can compare these
   // tests with other implmentations. The raw strings are completely optional.
   simple_conv.raw_string = "%c";
   simple_conv.conv_name = 'c';
   simple_conv.conv_val_raw = 'D';
 
   __llvm_libc::printf_core::convert(&writer, simple_conv);
 
   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) {
   __llvm_libc::printf_core::FormatSection right_justified_conv;
   right_justified_conv.has_conv = true;
   right_justified_conv.raw_string = "%4c";
   right_justified_conv.conv_name = 'c';
   right_justified_conv.min_width = 4;
   right_justified_conv.conv_val_raw = 'E';
   __llvm_libc::printf_core::convert(&writer, right_justified_conv);
 
   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) {
   __llvm_libc::printf_core::FormatSection left_justified_conv;
   left_justified_conv.has_conv = true;
   left_justified_conv.raw_string = "%-4c";
   left_justified_conv.conv_name = 'c';
   left_justified_conv.flags =
       __llvm_libc::printf_core::FormatFlags::LEFT_JUSTIFIED;
   left_justified_conv.min_width = 4;
   left_justified_conv.conv_val_raw = 'F';
   __llvm_libc::printf_core::convert(&writer, left_justified_conv);
 
   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) {
 
   __llvm_libc::printf_core::FormatSection simple_conv;
   simple_conv.has_conv = true;
   simple_conv.raw_string = "%s";
   simple_conv.conv_name = 's';
   simple_conv.conv_val_ptr = const_cast<char *>("DEF");
 
   __llvm_libc::printf_core::convert(&writer, simple_conv);
 
   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) {
   __llvm_libc::printf_core::FormatSection high_precision_conv;
   high_precision_conv.has_conv = true;
   high_precision_conv.raw_string = "%4s";
   high_precision_conv.conv_name = 's';
   high_precision_conv.precision = 4;
   high_precision_conv.conv_val_ptr = const_cast<char *>("456");
   __llvm_libc::printf_core::convert(&writer, high_precision_conv);
 
   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) {
   __llvm_libc::printf_core::FormatSection low_precision_conv;
   low_precision_conv.has_conv = true;
   low_precision_conv.raw_string = "%.2s";
   low_precision_conv.conv_name = 's';
   low_precision_conv.precision = 2;
   low_precision_conv.conv_val_ptr = const_cast<char *>("xyz");
   __llvm_libc::printf_core::convert(&writer, low_precision_conv);
 
   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) {
   __llvm_libc::printf_core::FormatSection right_justified_conv;
   right_justified_conv.has_conv = true;
   right_justified_conv.raw_string = "%4s";
   right_justified_conv.conv_name = 's';
   right_justified_conv.min_width = 4;
   right_justified_conv.conv_val_ptr = const_cast<char *>("789");
   __llvm_libc::printf_core::convert(&writer, right_justified_conv);
 
   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) {
   __llvm_libc::printf_core::FormatSection left_justified_conv;
   left_justified_conv.has_conv = true;
   left_justified_conv.raw_string = "%-4s";
   left_justified_conv.conv_name = 's';
   left_justified_conv.flags =
       __llvm_libc::printf_core::FormatFlags::LEFT_JUSTIFIED;
   left_justified_conv.min_width = 4;
   left_justified_conv.conv_val_ptr = const_cast<char *>("ghi");
   __llvm_libc::printf_core::convert(&writer, left_justified_conv);
 
   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) {
   __llvm_libc::printf_core::FormatSection section;
   section.has_conv = true;
   section.raw_string = "%d";
   section.conv_name = 'd';
   section.conv_val_raw = 12345;
   __llvm_libc::printf_core::convert(&writer, section);
 
   str_writer.terminate();
 
   ASSERT_STREQ(str, "12345");
-  ASSERT_EQ(writer.get_chars_written(), 5ull);
+  ASSERT_EQ(writer.get_chars_written(), 5);
 }

libc/test/src/stdio/printf_core/string_writer_test.cpp

TEST(LlvmLibcPrintfStringWriterTest, Write) {
   // 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 difference 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(), 3ull);
+  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(
       reinterpret_cast<void *>(&str_writer),
       __llvm_libc::printf_core::write_to_string);
   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(), 9ull);
+  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(
       reinterpret_cast<void *>(&str_writer),
       __llvm_libc::printf_core::write_to_string);
   writer.write_chars('a', 3);
 
   EXPECT_EQ(str[3], 'G');
   str_writer.terminate();
 
   ASSERT_STREQ("aaa", str);
-  ASSERT_EQ(writer.get_chars_written(), 3ull);
+  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(
       reinterpret_cast<void *>(&str_writer),
       __llvm_libc::printf_core::write_to_string);
   writer.write_chars('a', 3);
   writer.write_chars('D', 3);
   writer.write_chars('1', 3);
 
   str_writer.terminate();
 
   ASSERT_STREQ("aaaDDD111", str);
-  ASSERT_EQ(writer.get_chars_written(), 9ull);
+  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(
       reinterpret_cast<void *>(&str_writer),
       __llvm_libc::printf_core::write_to_string);
   writer.write_chars('Z', 99);
 
   str_writer.terminate();
 
   ASSERT_STREQ("ZZZZZZZZZZ"
                "ZZZZZZZZZZ"
                "ZZZZZZZZZZ"
                "ZZZZZZZZZZ"
                "ZZZZZZZZZZ"
                "ZZZZZZZZZZ"
                "ZZZZZZZZZZ"
                "ZZZZZZZZZZ"
                "ZZZZZZZZZZ"
                "ZZZZZZZZZ",
                str);
-  ASSERT_EQ(writer.get_chars_written(), 99ull);
+  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(
       reinterpret_cast<void *>(&str_writer),
       __llvm_libc::printf_core::write_to_string);
   writer.write_chars('a', 3);
   writer.write("DEF", 3);
   writer.write_chars('1', 3);
   writer.write("456", 3);
 
   str_writer.terminate();
 
   ASSERT_STREQ("aaaDEF111456", str);
-  ASSERT_EQ(writer.get_chars_written(), 12ull);
+  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(
       reinterpret_cast<void *>(&str_writer),
       __llvm_libc::printf_core::write_to_string);
   writer.write("abcDEF123456", 12);
 
   str_writer.terminate();
 
   ASSERT_STREQ("abcDEF1234", str);
-  ASSERT_EQ(writer.get_chars_written(), 12ull);
+  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(
       reinterpret_cast<void *>(&str_writer),
       __llvm_libc::printf_core::write_to_string);
 
   writer.write_chars('1', 15);
 
   str_writer.terminate();
 
   ASSERT_STREQ("1111111111", str);
-  ASSERT_EQ(writer.get_chars_written(), 15ull);
+  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(
       reinterpret_cast<void *>(&str_writer),
       __llvm_libc::printf_core::write_to_string);
   writer.write_chars('a', 3);
   writer.write("DEF", 3);
   writer.write_chars('1', 3);
   writer.write("456", 3);
 
   str_writer.terminate();
 
   ASSERT_STREQ("aaaDEF1114", str);
-  ASSERT_EQ(writer.get_chars_written(), 12ull);
+  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(
       reinterpret_cast<void *>(&str_writer),
       __llvm_libc::printf_core::write_to_string);
   // This is because the max length should be at most 1 less than the size of
   // the buffer it's writing to.
   writer.write_chars('a', 3);
   writer.write("DEF", 3);
   writer.write_chars('1', 3);
   writer.write("456", 3);
 
   str_writer.terminate();
 
   ASSERT_STREQ("", str);
-  ASSERT_EQ(writer.get_chars_written(), 12ull);
+  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(
       reinterpret_cast<void *>(&str_writer),
       __llvm_libc::printf_core::write_to_string);
   writer.write_chars('a', 3);
   writer.write("DEF", 3);
   writer.write_chars('1', 3);
   writer.write("456", 3);
 
-  ASSERT_EQ(writer.get_chars_written(), 12ull);
+  ASSERT_EQ(writer.get_chars_written(), 12);
 }