[libc] add buffering to FILE writes

[michaelrj]

Summary

Previously all FILE objects were fully buffered, this patch adds line
buffering and unbuffered output, as well as applying them to stdout and
stderr.

[sivachandra]

libc/src/__support/File/file.cpp

34

These additional conditionals are making the code even more complicated. Instead, we should do this: rename the current write_unlocked method to write_unlocked_fully_buffered and make it private. Next, add two new methods like this:

public:
size_t write_unlocked(const void *data, size_t len) {
  if (bufmode == _IOLBF) {
    return write_unlocked_line_buffered(data, len);
  } else if (bufmode == _IOFBF) {
    return write_unlocked_fully_buffered(data, len);
  } else {
    size_t written_size =platform_write(this, data, len);
    // Error handling if platform_write fails.

    platform_flush(this);

    // Error handling if platform_flush fails

    return len;
  }
}

private:
size_t write_unlocked_line_buffered(const void *data, size_t len) {
  // Instead of looking for the first new line char, look for the last new line char.
  char *last = last_newline(data, len);
  if (last == nullptr) {
   // If no new line char is found, just do normal buffering.
    return write_unlocked_fully_buffered(data, len);
  }

  size_t written_size;
  if (pos == 0) {
    // If there is no content in the buffer, write out the new data.
    written_size = platform_write(data, last - data /* size including the new line char */);
    // Perform error handling
  } else {
    // First write out the existing content, and then write out the new data.
    platform_write(buf, pos);
    // Perform error handling
    pos = 0;

    written_size = platform_write(data, last - data);
    // Perform error handling.
  }


  platform_flush();

  // Perform error handling

  if (written_size == len)
    return len;

  // Remaining data should be buffered.
  written_size = write_unlocked_fully_buffered(data + written_size, len - written_size);

  // Perform error handling

  return len;
}

libc/src/__support/File/file.h

62

We shouldn't need this, and may be even not do this. Add the macros _IOFBF etc. to stdio.h and use them directly.

libc/src/__support/File/linux_file.cpp

162–163

Shouldn't the default buffering mode be _IOFBF?

172–174

We don't need these anymore - just make the size arg 0 and buffer arg nullptr in the constructor call below.

libc/test/src/__support/File/file_test.cpp

110

Why did you change this to calloc?

[michaelrj]

rewrite to be closer to the original

libc/src/__support/File/file.cpp

34

I don't think splitting line buffered and fully buffered writes is the correct design. The reason being that their logic is almost identical, only varying in the spot where they flush the buffer. With this idea in mind, I've rewritten the original function to support both types of buffering in a manner closer to your original design. I've also kept unbuffered writes in this function, since it seems odd to split just that function off on its own if all the other write modes are together.

[sivachandra]

libc/src/__support/File/file.cpp

34

The concept of a flush_point is promising but in the current state of this patch, I am not yet convinced.

109

While the concept of flush_point seems promising, I think there is a mix up of a xfer_point and a flush_point in the code below. You want to transfer and clear the libc buffer when it is full. But, under _IOLBF, you want to actually perform the flush operation. May be I am missing it, but I don't see any call to platform_flush after this line.

117

It took me some time to understand why this return is OK. Which probably means we need to refactor/rename/comment.

118

If the if has braces, else should also have braces.

166

Should this be:

size_t remaining = len - write_size;

170

Should remaining be updated inside this if block? Couple of points:

1. It seems like the condition is true only for a specific case of _IOLBF.
2. This condition can be true even if there is no newline char in the input?

176

Is an else missing here?

177

This condition can be true if flush_point - write_size is less than remaining?

[michaelrj]

address comments

libc/src/__support/File/file.cpp

109

I've added a call to flush at the end.

166

in this case remaining represents the number of characters left after the flush point. This is useful, since we want to know if the characters after the flush point will fit into the buffer, and if they will we want to write everything to output directly.

170

no, since it's only tracking the number of characters after the flush point, and from that the number of characters written should be logical after each step.

This isn't only true in a special case of line buffered mode, but it is related to that. This handles the case where the flush point is after the end of the buffer, as well as the cases where the remainder after the flush point doesn't fit into the buffer. The flush point can only be after the end of the buffer if there's a newline, because otherwise the flush point is equal to the end of the buffer.

It should be able to be true if there's no newline in the input, that's a good catch. I've added an extra condition.

176

no, but there used to be one here. If this were only fully buffered then writing remaining to the buffer and writing from data to the output directly would never happen together, but in line buffered mode in the case where the newline is after the end of the buffer, but the characters after the newline fit into the buffer, we might need to do both.

[sivachandra]

While this is probably OK logically, it is way harder to reason than before. I am not yet convinced that this idea of flush_point and mixing line buffering and fully buffering is better.

I gave it one pass and will need at least one more to actually understand everything. I will do it once more and get back to you.

[michaelrj]

change to a simpler algorithm

[sivachandra]

The idea of "split point" makes it almost equivalent to the split approach I proposed. Taking a step back and looking at it objectively, consider that once a file is in a buffering mode, it is in that mode for its life time. Likewise, FBF is a mode by itself but LBF is FBF plus something else. To reflect these conceptual notions in code, we should use the approach of building LBF over FBF. That will keep FBF and LBF implementations simple and focused. Quantitatively also, the current state of this patch demonstrates that mixing makes it more complicated than it was previously.

I was wrong in expecting that concepts like flush point would make the implementation simple so I apologize for the misleading feedback I have given earlier. To move forward, we will do this:

1. Use the split approach of building LBF over FBF as I have suggested earlier.
2. Fix any bugs in FBF implementation separately.
3. If and when some optimizations are required, we will optimize based on the data about what exactly the problem is.

[michaelrj]

split the buffering modes into different functions, as well as cleaning up each implementation to be simpler in its specific case.

[sivachandra]

Good to go after the comments addressed.

libc/src/__support/File/file.cpp

183

We will keep the implementation simple for now like this:

fbf(primary.data(), primary.size());
// Error check
flush_unlocked(); // A call to flush and not to platform_flush()
fbf(remaining.data(), remaining.size());
// Error check

It should incur the same number of platform_* calls but might lead to a few additional copies. Iff required, we can optimize FBF to eliminate the unecessary copies. Infact, what we really want is to replace the first FBF operation by an NBF operation. We can do all that iff required.

I don't think we have flush_unlocked, so you will have to add it.

libc/test/src/__support/File/file_test.cpp

155

It will be cool to actually create an FBF file also and show the difference in pos comparisons (like those on line 170 and 174).

426

_IOFBF?

440

Use some buffering mode. Or, test for all modes.

[michaelrj]

address comments

libc/src/__support/File/file.cpp

183

I did add NBF as well as flush_unlocked, and now we're down to three platform writes maximum (one for the buffer, one for the section up to the newline, and one for the remainder).

[sivachandra]

LGTM modulo one comment about the NBF operation.

libc/src/__support/File/file.cpp

54

You should have an else which does platfrom_flush()?

169

You shouldn't need this as the NBF operation does platform flush anyway. My previous comment suggested this assuming FBF on line 163.

[michaelrj]

libc/src/__support/File/file.cpp

54

I have the flush outside of the unbuffered write so that in the fully buffered function (line 63) we can use the unbuffered write when the data is too large for the buffer. This means that in the base write_unlocked function and in write_unlocked_lbf we have to flush separately, but I think that's okay.

[sivachandra]

libc/src/__support/File/file.cpp

54

Ah, I missed that!

libc/config/linux/api.td

let Macros = [
     NullMacro,
   ];
 }
 
 def StdIOAPI : PublicAPI<"stdio.h"> {
   let Macros = [
     SimpleMacroDef<"stderr", "stderr">,
     SimpleMacroDef<"stdout", "stdout">,
+    SimpleMacroDef<"_IOFBF", "0">,
+    SimpleMacroDef<"_IOLBF", "1">,
+    SimpleMacroDef<"_IONBF", "2">,
   ];
   let Types = ["size_t", "FILE", "cookie_io_functions_t"];
 }
 
 def StdlibAPI : PublicAPI<"stdlib.h"> {
   let Types = [
     "div_t",
     "ldiv_t",

libc/spec/stdc.td

HeaderSpec Math = HeaderSpec<
       ]
   >;
 
   HeaderSpec StdIO = HeaderSpec<
       "stdio.h",
       [
           Macro<"stderr">,
           Macro<"stdout">,
+          Macro<"_IOFBF">,
+          Macro<"_IOLBF">,
+          Macro<"_IONBF">,
       ], // Macros
       [ // Types
           SizeTType,
           FILE,
       ],
       [], // Enumerations
       [
           FunctionSpec<

libc/src/__support/File/file.h

public:
 
   // Denotes a file to be created for writing.
   enum class CreateType : ModeFlags {
     EXCLUSIVE = 0x100,
   };
 
 private:
   enum class FileOp : uint8_t { NONE, READ, WRITE, SEEK };
 
   // Platfrom specific functions which create new file objects should initialize
   // these fields suitably via the constructor. Typically, they should be simple
   // syscall wrappers for the corresponding functionality.
   WriteFunc *platform_write;
   ReadFunc *platform_read;
   SeekFunc *platform_seek;
   CloseFunc *platform_close;
   FlushFunc *platform_flush;
size_t read(void *data, size_t len) {
     FileLock l(this);
     return read_unlocked(data, len);
   }
 
   int seek(long offset, int whence);
 
   // If buffer has data written to it, flush it out. Does nothing if the
   // buffer is currently being used as a read buffer.
-  int flush();
+  int flush() {
+    FileLock lock(this);
+    return flush_unlocked();
+  }
+
+  int flush_unlocked();
 
   // Sets the internal buffer to |buffer| with buffering mode |mode|.
   // |size| is the size of |buffer|. This new |buffer| is owned by the
   // stream only if |owned| is true.
   void set_buffer(void *buffer, size_t size, bool owned);
 
   // Closes the file stream and frees up all resources owned by it.
   int close();
public:
   bool iseof() {
     FileLock l(this);
     return iseof_unlocked();
   }
 
   // Returns an bit map of flags corresponding to enumerations of
   // OpenMode, ContentType and CreateType.
   static ModeFlags mode_flags(const char *mode);
+
+private:
+  size_t write_unlocked_lbf(const void *data, size_t len);
+  size_t write_unlocked_fbf(const void *data, size_t len);
+  size_t write_unlocked_nbf(const void *data, size_t len);
 };
 
 // The implementaiton of this function is provided by the platfrom_file
 // library.
 File *openfile(const char *path, const char *mode);
 
 extern File *stdout;
 extern File *stderr;
 
 } // namespace __llvm_libc
 
 #endif // LLVM_LIBC_SRC_SUPPORT_OSUTIL_FILE_H

libc/src/__support/File/file.cpp

size_t File::write_unlocked(const void *data, size_t len) {
   if (!write_allowed()) {
     errno = EBADF;
     err = true;
     return 0;
   }
 
   prev_op = FileOp::WRITE;
 
-  cpp::ArrayRef<uint8_t> dataref(data, len);
-  cpp::MutableArrayRef<uint8_t> bufref(buf, bufsize);
+  if (bufmode == _IOFBF) { // fully buffered
+    return write_unlocked_fbf(data, len);
+  } else if (bufmode == _IOLBF) { // line buffered
+    return write_unlocked_lbf(data, len);
+  } else /*if (bufmode == _IONBF) */ { // unbuffered
+    size_t ret_val = write_unlocked_nbf(data, len);
+    flush_unlocked();
+    return ret_val;
+  }
+}
+
+size_t File::write_unlocked_nbf(const void *data, size_t len) {
+  if (pos > 0) { // If the buffer is not empty
+    // Flush the buffer
+    const size_t write_size = pos;
+    size_t bytes_written = platform_write(this, buf, write_size);
+    pos = 0; // Buffer is now empty so reset pos to the beginning.
+    // If less bytes were written than expected, then an error occurred.
+    if (bytes_written < write_size) {
+      err = true;
+      return 0; // No bytes from data were written, so return 0.
+    }
+  }
 
-  const size_t used = pos;
+  size_t written = platform_write(this, data, len);
+  if (written < len)
+    err = true;
+  return written;
+}
+
+size_t File::write_unlocked_fbf(const void *data, size_t len) {
+  const size_t init_pos = pos;
   const size_t bufspace = bufsize - pos;
-  const size_t write_size = bufspace > len ? len : bufspace;
+
+  // If data is too large to be buffered at all, then just write it unbuffered.
+  if (len > bufspace + bufsize)
+    return write_unlocked_nbf(data, len);
+
+  // we split |data| (conceptually) using the split point. Then we handle the
+  // two pieces separately.
+  const size_t split_point = len < bufspace ? len : bufspace;
+
+  // The primary piece is the piece of |data| we want to write to the buffer
+  // before flushing. It will always fit into the buffer, since the split point
+  // is defined as being min(len, bufspace), and it will always exist if len is
+  // non-zero.
+  cpp::ArrayRef<uint8_t> primary(data, split_point);
+
+  // The second piece is the remainder of |data|. It is written to the buffer if
+  // it fits, or written directly to the output if it doesn't. If the primary
+  // piece fits entirely in the buffer, the remainder may be nothing.
+  cpp::ArrayRef<uint8_t> remainder(
+      static_cast<const uint8_t *>(data) + split_point, len - split_point);
+
+  cpp::MutableArrayRef<uint8_t> bufref(buf, bufsize);
+
+  // Copy the first piece into the buffer.
   // TODO: Replace the for loop below with a call to internal memcpy.
-  for (size_t i = 0; i < write_size; ++i)
-    bufref[pos + i] = dataref[i];
-  pos += write_size;
-  if (len < bufspace)
+  for (size_t i = 0; i < primary.size(); ++i)
+    bufref[pos + i] = primary[i];
+  pos += primary.size();
+
+  // If there is no remainder, we can return early, since the first piece has
+  // fit completely into the buffer.
+  if (remainder.size() == 0)
     return len;
 
-  // If the control reaches beyond this point, it means that |data|
-  // is more than what can be accomodated in the buffer. So, we first
-  // flush out the buffer.
-  size_t bytes_written = platform_write(this, buf, bufsize);
+  // We need to flush the buffer now, since there is still data and the buffer
+  // is full.
+  const size_t write_size = pos;
+  size_t bytes_written = platform_write(this, buf, write_size);
   pos = 0; // Buffer is now empty so reset pos to the beginning.
-  if (bytes_written < bufsize) {
+  // If less bytes were written than expected, then an error occurred. Return
+  // the number of bytes that have been written from |data|.
+  if (bytes_written < write_size) {
     err = true;
-    // If less bytes were written than expected, then there are two
-    // possibilities.
-    // 1. None of the bytes from |data| were flushed out.
-    if (bytes_written <= used)
-      return 0;
-    // 2. Some of the bytes from |data| were written
-    return bytes_written - used;
+    return bytes_written <= init_pos ? 0 : bytes_written - init_pos;
   }
 
-  // If the remaining bytes from |data| can fit in the buffer, write
-  // into it. Else, write it directly to the platform stream.
-  size_t remaining = len - write_size;
-  if (remaining <= len) {
+  // The second piece is handled basically the same as the first, although we
+  // know that if the second piece has data in it then the buffer has been
+  // flushed, meaning that pos is always 0.
+  if (remainder.size() < bufsize) {
     // TODO: Replace the for loop below with a call to internal memcpy.
-    for (size_t i = 0; i < remaining; ++i)
-      bufref[i] = dataref[i];
-    pos += remaining;
-    return len;
-  }
+    for (size_t i = 0; i < remainder.size(); ++i)
+      bufref[i] = remainder[i];
+    pos = remainder.size();
+  } else {
+    size_t bytes_written =
+        platform_write(this, remainder.data(), remainder.size());
+
+    // If less bytes were written than expected, then an error occurred. Return
+    // the number of bytes that have been written from |data|.
+    if (bytes_written < remainder.size()) {
+      err = true;
+      return primary.size() + bytes_written;
+    }
+  }
+
 
-  size_t transferred =
-      platform_write(this, dataref.data() + write_size, remaining);
-  if (transferred < remaining) {
+size_t File::write_unlocked_lbf(const void *data, size_t len) {
+  const size_t init_pos = pos;
+  const size_t bufspace = bufsize - pos;
+
+  constexpr char NEWLINE_CHAR = '\n';
+  size_t last_newline = len;
+  for (size_t i = len - 1; i > 0; --i) {
+    if (static_cast<const char *>(data)[i] == NEWLINE_CHAR) {
+      last_newline = i;
+      break;
+    }
+  }
+
+  // If there is no newline, treat this as fully buffered.
+  if (last_newline == len) {
+    return write_unlocked_fbf(data, len);
+  }
+
+  // we split |data| (conceptually) using the split point. Then we handle the
+  // two pieces separately.
+  const size_t split_point = last_newline + 1;
+
+  // The primary piece is everything in |data| up to the newline. It's written
+  // unbuffered to the output.
+  cpp::ArrayRef<uint8_t> primary(data, split_point);
+
+  // The second piece is the remainder of |data|. It is written fully buffered,
+  // meaning it may stay in the buffer if it fits.
+  cpp::ArrayRef<uint8_t> remainder(
+      static_cast<const uint8_t *>(data) + split_point, len - split_point);
+
+  size_t written = 0;
+
+  written = write_unlocked_nbf(primary.data(), primary.size());
+  if (written < primary.size()) {
     err = true;
-    return write_size + transferred;
+    return written;
   }
+
+  flush_unlocked();
+
+  written += write_unlocked_fbf(remainder.data(), remainder.size());
+  if (written < primary.size() + remainder.size()) {
+    err = true;
+    return written;
+  }
+
   return len;
 }
 
 size_t File::read_unlocked(void *data, size_t len) {
   if (!read_allowed()) {
     errno = EBADF;
     err = true;
     return 0;
   }
 
   prev_op = FileOp::READ;
 
   cpp::MutableArrayRef<uint8_t> bufref(buf, bufsize);
   cpp::MutableArrayRef<uint8_t> dataref(data, len);
 
int File::seek(long offset, int whence) {
   pos = read_limit = 0;
   prev_op = FileOp::SEEK;
   // Reset the eof flag as a seek might move the file positon to some place
   // readable.
   eof = false;
   return platform_seek(this, offset, whence);
 }
 
-int File::flush() {
-  FileLock lock(this);
+int File::flush_unlocked() {
   if (prev_op == FileOp::WRITE && pos > 0) {
     size_t transferred_size = platform_write(this, buf, pos);
     if (transferred_size < pos) {
       err = true;
       return -1;
     }
     pos = 0;
     return platform_flush(this);

libc/src/__support/File/linux_file.cpp

 //===--- Linux specialization of the File data structure ------------------===//
 //
 // 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 "file.h"
 
 #include "src/__support/OSUtil/syscall.h" // For internal syscall function.
 
 #include <errno.h>
-#include <fcntl.h>       // For mode_t and other flags to the open syscall
+#include <fcntl.h> // For mode_t and other flags to the open syscall
+#include <stdio.h>
 #include <stdlib.h>      // For malloc
 #include <sys/syscall.h> // For syscall numbers
 
 namespace __llvm_libc {
 
 namespace {
 
 size_t write_func(File *, const void *, size_t);
#endif
 
   if (fd < 0) {
     errno = -fd;
     return nullptr;
   }
 
   void *buffer = malloc(File::DEFAULT_BUFFER_SIZE);
   auto *file = reinterpret_cast<LinuxFile *>(malloc(sizeof(LinuxFile)));
-  LinuxFile::init(
-      file, fd, buffer, File::DEFAULT_BUFFER_SIZE,
-      0, // TODO: Set the correct buffer mode when buffer mode is available.
-      true, modeflags);
+  LinuxFile::init(file, fd, buffer, File::DEFAULT_BUFFER_SIZE, _IOFBF, true,
+                  modeflags);
   return file;
 }
 
-// TODO: Use the appropriate buffering modes for the standard streams below
-// the different buffering modes are available.
 constexpr size_t STDOUT_BUFFER_SIZE = 1024;
 char stdout_buffer[STDOUT_BUFFER_SIZE];
-static LinuxFile StdOut(1, stdout_buffer, STDOUT_BUFFER_SIZE, 0, false,
+static LinuxFile StdOut(1, stdout_buffer, STDOUT_BUFFER_SIZE, _IOLBF, false,
                         File::ModeFlags(File::OpenMode::APPEND));
 File *stdout = &StdOut;
 
-constexpr size_t STDERR_BUFFER_SIZE = 1024;
-char stderr_buffer[STDERR_BUFFER_SIZE];
-static LinuxFile StdErr(2, stderr_buffer, STDERR_BUFFER_SIZE, 0, false,
+constexpr size_t STDERR_BUFFER_SIZE = 0;
+static LinuxFile StdErr(2, nullptr, STDERR_BUFFER_SIZE, _IONBF, false,
                         File::ModeFlags(File::OpenMode::APPEND));
 File *stderr = &StdErr;
 
 } // namespace __llvm_libc

libc/test/src/__support/File/file_test.cpp

if (whence == SEEK_CUR)
     sf->pos += offset;
   if (whence == SEEK_END)
     sf->pos = SIZE + offset;
   return 0;
 }
 
 StringFile *new_string_file(char *buffer, size_t buflen, int bufmode,
                             bool owned, const char *mode) {
   StringFile *f = reinterpret_cast<StringFile *>(malloc(sizeof(StringFile)));
   f->init(buffer, buflen, bufmode, owned, __llvm_libc::File::mode_flags(mode));
   return f;
 }
 
 TEST(LlvmLibcFileTest, WriteOnly) {
   const char data[] = "hello, file";
   constexpr size_t FILE_BUFFER_SIZE = sizeof(data) * 3 / 2;
   char file_buffer[FILE_BUFFER_SIZE];
-  StringFile *f = new_string_file(file_buffer, FILE_BUFFER_SIZE, 0, false, "w");
+  StringFile *f =
+      new_string_file(file_buffer, FILE_BUFFER_SIZE, _IOFBF, false, "w");
 
   ASSERT_EQ(sizeof(data), f->write(data, sizeof(data)));
   EXPECT_EQ(f->get_pos(), size_t(0)); // Data is buffered in the file stream
   ASSERT_EQ(f->flush(), 0);
   EXPECT_EQ(f->get_pos(), sizeof(data)); // Data should now be available
   EXPECT_STREQ(f->get_str(), data);
 
   f->reset();
   ASSERT_EQ(f->get_pos(), size_t(0));
   ASSERT_EQ(sizeof(data), f->write(data, sizeof(data)));
   EXPECT_EQ(f->get_pos(), size_t(0)); // Data is buffered in the file stream
   // The second write should trigger a buffer flush.
   ASSERT_EQ(sizeof(data), f->write(data, sizeof(data)));
   EXPECT_GE(f->get_pos(), size_t(0));
   ASSERT_EQ(f->flush(), 0);
   EXPECT_EQ(f->get_pos(), 2 * sizeof(data));
+  MemoryView src1("hello, file\0hello, file", sizeof(data) * 2),
+      dst1(f->get_str(), sizeof(data) * 2);
+  EXPECT_MEM_EQ(src1, dst1);
 
   char read_data[sizeof(data)];
   // This is not a readable file.
   EXPECT_EQ(f->read(read_data, sizeof(data)), size_t(0));
   EXPECT_TRUE(f->error());
   EXPECT_NE(errno, 0);
   errno = 0;
 
   ASSERT_EQ(f->close(), 0);
 }
 
+TEST(LlvmLibcFileTest, WriteLineBuffered) {
+  const char data[] = "hello\n file";
+  constexpr size_t FILE_BUFFER_SIZE = sizeof(data) * 3 / 2;
+
+  char file_buffer_line[FILE_BUFFER_SIZE];
+  char file_buffer_full[FILE_BUFFER_SIZE];
+
+  StringFile *f_line =
+      new_string_file(file_buffer_line, FILE_BUFFER_SIZE, _IOLBF, false, "w");
+  // We also initialize a fully buffered file we'll do the same writes to for
+  // comparison.
+  StringFile *f_full =
+      new_string_file(file_buffer_full, FILE_BUFFER_SIZE, _IOFBF, false, "w");
+
+  ASSERT_EQ(sizeof(data), f_line->write(data, sizeof(data)));
+  ASSERT_EQ(sizeof(data), f_full->write(data, sizeof(data)));
+
+  EXPECT_EQ(f_line->get_pos(), size_t(6)); // buffer after the newline
+  EXPECT_EQ(f_full->get_pos(), size_t(0)); // buffer all of data
+
+  MemoryView src1("hello\n", 6), dst1(f_line->get_str(), 6);
+  EXPECT_MEM_EQ(src1, dst1);
+
+  // We can't check the data in f_full, since no data has been written.
+
+  const char data2[] = "longer for an \n overflow";
+
+  ASSERT_EQ(sizeof(data2), f_line->write(data2, sizeof(data2)));
+  // The line buffer's initial contents should be " file\0"
+  // Writing data2 should write up until the newline, even though that doesn't
+  // all fit in the buffer.
+
+  ASSERT_EQ(sizeof(data2), f_full->write(data2, sizeof(data2)));
+  // The full buffer's initial contents should be "hello\n file\0"
+  // Writing data2 should cause a flush of the buffer, as well as the remainder
+  // to be written directly since it doesn't fit in the buffer.
+
+  EXPECT_EQ(f_line->get_pos(), size_t(27));
+  EXPECT_EQ(f_full->get_pos(), sizeof(data) + sizeof(data2));
+
+  MemoryView src2("hello\n file\0longer for an \n", 27),
+      dst2(f_line->get_str(), 27);
+  EXPECT_MEM_EQ(src2, dst2);
+
+  MemoryView src3("hello\n file\0longer for an \n overflow", 37),
+      dst_full_final(f_full->get_str(), 37);
+  EXPECT_MEM_EQ(src3, dst_full_final);
+
+  ASSERT_EQ(f_line->flush(), 0);
+  ASSERT_EQ(f_full->flush(), 0);
+
+  EXPECT_EQ(f_line->get_pos(), sizeof(data) + sizeof(data2));
+  MemoryView dst_line_final(f_line->get_str(), 37);
+  EXPECT_MEM_EQ(src3, dst_line_final);
+  EXPECT_MEM_EQ(src3, dst_full_final);
+
+  ASSERT_EQ(f_line->close(), 0);
+  ASSERT_EQ(f_full->close(), 0);
+}
+
+TEST(LlvmLibcFileTest, WriteUnbuffered) {
+  const char data[] = "written immediately";
+  constexpr size_t FILE_BUFFER_SIZE = sizeof(data) + 1;
+  char file_buffer[FILE_BUFFER_SIZE];
+  StringFile *f =
+      new_string_file(file_buffer, FILE_BUFFER_SIZE, _IONBF, false, "w");
+
+  ASSERT_EQ(sizeof(data), f->write(data, sizeof(data)));
+  EXPECT_EQ(f->get_pos(),
+            sizeof(data)); // no buffering means this is written immediately.
+  EXPECT_STREQ(f->get_str(), data);
+
+  ASSERT_EQ(f->close(), 0);
+}
+
 TEST(LlvmLibcFileTest, ReadOnly) {
   const char initial_content[] = "1234567890987654321";
   constexpr size_t FILE_BUFFER_SIZE = sizeof(initial_content);
   char file_buffer[FILE_BUFFER_SIZE];
-  StringFile *f = new_string_file(file_buffer, FILE_BUFFER_SIZE, 0, false, "r");
+  StringFile *f =
+      new_string_file(file_buffer, FILE_BUFFER_SIZE, _IOFBF, false, "r");
   f->reset_and_fill(initial_content, sizeof(initial_content));
 
   constexpr size_t READ_SIZE = sizeof(initial_content) / 2;
   char read_data[READ_SIZE];
   ASSERT_EQ(READ_SIZE, f->read(read_data, READ_SIZE));
   EXPECT_FALSE(f->iseof());
   // Reading less than file buffer worth will still read one
   // full buffer worth of data.
TEST(LlvmLibcFileTest, ReadOnly) {
 
   ASSERT_EQ(f->close(), 0);
 }
 
 TEST(LlvmLibcFileTest, ReadSeekCurAndRead) {
   const char initial_content[] = "1234567890987654321";
   constexpr size_t FILE_BUFFER_SIZE = sizeof(initial_content);
   char file_buffer[FILE_BUFFER_SIZE];
-  StringFile *f = new_string_file(file_buffer, FILE_BUFFER_SIZE, 0, false, "r");
+  StringFile *f =
+      new_string_file(file_buffer, FILE_BUFFER_SIZE, _IOFBF, false, "r");
   f->reset_and_fill(initial_content, sizeof(initial_content));
 
   constexpr size_t READ_SIZE = 5;
   char data[READ_SIZE];
   data[READ_SIZE - 1] = '\0';
   ASSERT_EQ(f->read(data, READ_SIZE - 1), READ_SIZE - 1);
   ASSERT_STREQ(data, "1234");
   ASSERT_EQ(f->seek(5, SEEK_CUR), 0);
   ASSERT_EQ(f->read(data, READ_SIZE - 1), READ_SIZE - 1);
   ASSERT_STREQ(data, "0987");
   ASSERT_EQ(f->seek(-5, SEEK_CUR), 0);
   ASSERT_EQ(f->read(data, READ_SIZE - 1), READ_SIZE - 1);
   ASSERT_STREQ(data, "9098");
   ASSERT_EQ(f->close(), 0);
 }
 
 TEST(LlvmLibcFileTest, AppendOnly) {
   const char initial_content[] = "1234567890987654321";
   const char write_data[] = "append";
   constexpr size_t FILE_BUFFER_SIZE = sizeof(write_data) * 3 / 2;
   char file_buffer[FILE_BUFFER_SIZE];
-  StringFile *f = new_string_file(file_buffer, FILE_BUFFER_SIZE, 0, false, "a");
+  StringFile *f =
+      new_string_file(file_buffer, FILE_BUFFER_SIZE, _IOFBF, false, "a");
   f->reset_and_fill(initial_content, sizeof(initial_content));
 
   constexpr size_t READ_SIZE = 5;
   char read_data[READ_SIZE];
   // This is not a readable file.
   ASSERT_EQ(f->read(read_data, READ_SIZE), size_t(0));
   EXPECT_TRUE(f->error());
   EXPECT_NE(errno, 0);
TEST(LlvmLibcFileTest, AppendOnly) {
   ASSERT_EQ(f->close(), 0);
 }
 
 TEST(LlvmLibcFileTest, WriteUpdate) {
   const char data[] = "hello, file";
   constexpr size_t FILE_BUFFER_SIZE = sizeof(data) * 3 / 2;
   char file_buffer[FILE_BUFFER_SIZE];
   StringFile *f =
-      new_string_file(file_buffer, FILE_BUFFER_SIZE, 0, false, "w+");
+      new_string_file(file_buffer, FILE_BUFFER_SIZE, _IOFBF, false, "w+");
 
   ASSERT_EQ(sizeof(data), f->write(data, sizeof(data)));
   EXPECT_EQ(f->get_pos(), size_t(0)); // Data is buffered in the file stream
 
   ASSERT_EQ(f->seek(0, SEEK_SET), 0);
 
   // Seek flushes the stream buffer so we can read the previously written data.
   char read_data[sizeof(data)];
   ASSERT_EQ(f->read(read_data, sizeof(data)), sizeof(data));
   EXPECT_STREQ(read_data, data);
 
   ASSERT_EQ(f->close(), 0);
 }
 
 TEST(LlvmLibcFileTest, ReadUpdate) {
   const char initial_content[] = "1234567890987654321";
   constexpr size_t FILE_BUFFER_SIZE = sizeof(initial_content);
   char file_buffer[FILE_BUFFER_SIZE];
   StringFile *f =
-      new_string_file(file_buffer, FILE_BUFFER_SIZE, 0, false, "r+");
+      new_string_file(file_buffer, FILE_BUFFER_SIZE, _IOFBF, false, "r+");
   f->reset_and_fill(initial_content, sizeof(initial_content));
 
   constexpr size_t READ_SIZE = sizeof(initial_content) / 2;
   char read_data[READ_SIZE];
   ASSERT_EQ(READ_SIZE, f->read(read_data, READ_SIZE));
   EXPECT_FALSE(f->iseof());
   // Reading less than file buffer worth will still read one
   // full buffer worth of data.

 }
 
 TEST(LlvmLibcFileTest, AppendUpdate) {
   const char initial_content[] = "1234567890987654321";
   const char data[] = "hello, file";
   constexpr size_t FILE_BUFFER_SIZE = sizeof(data) * 3 / 2;
   char file_buffer[FILE_BUFFER_SIZE];
   StringFile *f =
-      new_string_file(file_buffer, FILE_BUFFER_SIZE, 0, false, "a+");
+      new_string_file(file_buffer, FILE_BUFFER_SIZE, _IOFBF, false, "a+");
   f->reset_and_fill(initial_content, sizeof(initial_content));
 
   ASSERT_EQ(sizeof(data), f->write(data, sizeof(data)));
   EXPECT_EQ(f->get_pos(), size_t(0)); // Data is buffered in the file stream
   ASSERT_EQ(f->flush(), 0);
   // The flush should write |data| to the endof the file.
   EXPECT_EQ(f->get_pos(), sizeof(data) + sizeof(initial_content));
 
TEST(LlvmLibcFileTest, AppendUpdate) {
   constexpr size_t READ_SIZE = 10;
   char read_data[READ_SIZE];
   ASSERT_EQ(READ_SIZE, f->read(read_data, READ_SIZE));
   MemoryView src4(initial_content, READ_SIZE), dst4(read_data, READ_SIZE);
   EXPECT_MEM_EQ(src4, dst4);
 
   ASSERT_EQ(f->close(), 0);
 }
+
+TEST(LlvmLibcFileTest, SmallBuffer) {
+  const char WRITE_DATA[] = "small buffer";
+  constexpr size_t WRITE_SIZE = sizeof(WRITE_DATA);
+  constexpr size_t FILE_BUFFER_SIZE = sizeof(WRITE_DATA) / 2 - 1;
+  char file_buffer[FILE_BUFFER_SIZE];
+  StringFile *f =
+      new_string_file(file_buffer, FILE_BUFFER_SIZE, _IOFBF, false, "w");
+
+  ASSERT_EQ(WRITE_SIZE, f->write(WRITE_DATA, WRITE_SIZE));
+  // Since data much larger than the buffer is being written, all of it should
+  // be available in the file without a flush operation.
+  EXPECT_EQ(f->get_pos(), sizeof(WRITE_DATA));
+  ASSERT_STREQ(f->get_str(), WRITE_DATA);
+
+  ASSERT_EQ(f->close(), 0);
+}
+
+TEST(LlvmLibcFileTest, ZeroLengthBuffer) {
+  const char WRITE_DATA[] = "small buffer";
+  constexpr size_t WRITE_SIZE = sizeof(WRITE_DATA);
+  StringFile *f_fbf = new_string_file(nullptr, 0, _IOFBF, true, "w");
+  StringFile *f_lbf = new_string_file(nullptr, 0, _IOLBF, true, "w");
+  StringFile *f_nbf = new_string_file(nullptr, 0, _IONBF, true, "w");
+
+  ASSERT_EQ(WRITE_SIZE, f_fbf->write(WRITE_DATA, WRITE_SIZE));
+  ASSERT_EQ(WRITE_SIZE, f_lbf->write(WRITE_DATA, WRITE_SIZE));
+  ASSERT_EQ(WRITE_SIZE, f_nbf->write(WRITE_DATA, WRITE_SIZE));
+  // Since there is no buffer space, all of the written data should
+  // be available in the file without a flush operation.
+  EXPECT_EQ(f_fbf->get_pos(), sizeof(WRITE_DATA));
+  EXPECT_EQ(f_lbf->get_pos(), sizeof(WRITE_DATA));
+  EXPECT_EQ(f_nbf->get_pos(), sizeof(WRITE_DATA));
+  ASSERT_STREQ(f_fbf->get_str(), WRITE_DATA);
+  ASSERT_STREQ(f_lbf->get_str(), WRITE_DATA);
+  ASSERT_STREQ(f_nbf->get_str(), WRITE_DATA);
+
+  ASSERT_EQ(f_fbf->close(), 0);
+  ASSERT_EQ(f_lbf->close(), 0);
+  ASSERT_EQ(f_nbf->close(), 0);
+}