diff --git a/libc/src/__support/File/file.h b/libc/src/__support/File/file.h --- a/libc/src/__support/File/file.h +++ b/libc/src/__support/File/file.h @@ -57,6 +57,10 @@ EXCLUSIVE = 0x100, }; + static constexpr int FULLY_BUFFERED = 0; + static constexpr int LINE_BUFFERED = 1; + static constexpr int UNBUFFERED = 2; + private: enum class FileOp : uint8_t { NONE, READ, WRITE, SEEK }; diff --git a/libc/src/__support/File/file.cpp b/libc/src/__support/File/file.cpp --- a/libc/src/__support/File/file.cpp +++ b/libc/src/__support/File/file.cpp @@ -25,53 +25,62 @@ prev_op = FileOp::WRITE; + if (bufmode == UNBUFFERED) { + size_t written = platform_write(this, data, len); + if (written < len) + err = true; + platform_flush(this); + return written; + } cpp::ArrayRef dataref(data, len); cpp::MutableArrayRef bufref(buf, bufsize); - const size_t used = pos; - const size_t bufspace = bufsize - pos; - const size_t write_size = bufspace > len ? len : bufspace; - // 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) - return len; + size_t written = 0; + + while (written < len) { + const size_t bufspace = bufsize - pos; + size_t write_size = bufspace > (len - written) ? (len - written) : bufspace; + if (bufmode == LINE_BUFFERED) { + size_t newline_pos; + for (newline_pos = 0; newline_pos < write_size; ++newline_pos) { + if (dataref[newline_pos + written] == '\n') { + write_size = newline_pos + 1; // include the newline in the write + break; + } + } + } + // 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 + written]; + written += write_size; + pos += write_size; + + // if we've written all of the characters and the buffer shouldn't be + // flushed, skip to the end and don't flush the buffer. + if (written == len && pos < bufsize && + !(bufmode == LINE_BUFFERED && bufref[pos - 1] == '\n')) + break; - // 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); - pos = 0; // Buffer is now empty so reset pos to the beginning. - if (bytes_written < bufsize) { - 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; - } + // If the control reaches beyond this point, the buffer is intended to be + // flushed. + size_t to_write = pos; // a copy for later comparisons + size_t bytes_written = platform_write(this, buf, to_write); - // 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) { - // 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; + pos = 0; // Buffer is now empty so reset pos to the beginning. + if (bytes_written < to_write) { + 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 (static_cast(bytes_written) < + static_cast(to_write - written)) + return 0; // this comparison has to be signed in case written > to_write + // 2. Some of the bytes from |data| were written + return written - (to_write - bytes_written); + } } - size_t transferred = - platform_write(this, dataref.data() + write_size, remaining); - if (transferred < remaining) { - err = true; - return write_size + transferred; - } - return len; + return written; } size_t File::read_unlocked(void *data, size_t len) { diff --git a/libc/src/__support/File/linux_file.cpp b/libc/src/__support/File/linux_file.cpp --- a/libc/src/__support/File/linux_file.cpp +++ b/libc/src/__support/File/linux_file.cpp @@ -158,25 +158,22 @@ void *buffer = malloc(File::DEFAULT_BUFFER_SIZE); auto *file = reinterpret_cast(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, File::UNBUFFERED, + 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, + File::LINE_BUFFERED, 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, - File::ModeFlags(File::OpenMode::APPEND)); +static LinuxFile StdErr(2, stderr_buffer, STDERR_BUFFER_SIZE, File::UNBUFFERED, + false, File::ModeFlags(File::OpenMode::APPEND)); File *stderr = &StdErr; } // namespace __llvm_libc diff --git a/libc/test/src/__support/File/file_test.cpp b/libc/test/src/__support/File/file_test.cpp --- a/libc/test/src/__support/File/file_test.cpp +++ b/libc/test/src/__support/File/file_test.cpp @@ -107,7 +107,7 @@ StringFile *new_string_file(char *buffer, size_t buflen, int bufmode, bool owned, const char *mode) { - StringFile *f = reinterpret_cast(malloc(sizeof(StringFile))); + StringFile *f = reinterpret_cast(calloc(1, sizeof(StringFile))); f->init(buffer, buflen, bufmode, owned, __llvm_libc::File::mode_flags(mode)); return f; } @@ -116,7 +116,9 @@ 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, + __llvm_libc::File::FULLY_BUFFERED, 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 @@ -144,11 +146,55 @@ 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[FILE_BUFFER_SIZE]; + StringFile *f = new_string_file(file_buffer, FILE_BUFFER_SIZE, + __llvm_libc::File::LINE_BUFFERED, false, "w"); + + ASSERT_EQ(sizeof(data), f->write(data, sizeof(data))); + EXPECT_EQ(f->get_pos(), size_t(6)); // buffer after the newline + EXPECT_STREQ(f->get_str(), "hello\n"); + + const char data2[] = "longer for an \n overflow"; + + ASSERT_EQ(sizeof(data2), f->write(data2, sizeof(data2))); + // the buffer's initial contents should be " file\0" + // Writing data2 should cause two flushes, the first being when the buffer + // fills, flushing " file\0longer for a", then the second being when a newline + // is hit, flushing "n \n" and leaving " overflow\0" in the buffer. + EXPECT_EQ(f->get_pos(), size_t(27)); + EXPECT_STREQ(f->get_str(), data); + EXPECT_STREQ(f->get_str() + sizeof(data), "longer for an \n"); + ASSERT_EQ(f->flush(), 0); + EXPECT_STREQ(f->get_str() + sizeof(data), data2); + + ASSERT_EQ(f->close(), 0); +} + +TEST(LlvmLibcFileTest, WriteUnbuffered) { + const char data[] = "written immediately"; + constexpr size_t FILE_BUFFER_SIZE = (sizeof(data) / 2) + 1; + char file_buffer[FILE_BUFFER_SIZE]; + StringFile *f = new_string_file(file_buffer, FILE_BUFFER_SIZE, + __llvm_libc::File::UNBUFFERED, 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, + __llvm_libc::File::FULLY_BUFFERED, false, "r"); f->reset_and_fill(initial_content, sizeof(initial_content)); constexpr size_t READ_SIZE = sizeof(initial_content) / 2; @@ -198,7 +244,9 @@ 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, + __llvm_libc::File::FULLY_BUFFERED, false, "r"); f->reset_and_fill(initial_content, sizeof(initial_content)); constexpr size_t READ_SIZE = 5; @@ -220,7 +268,9 @@ 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, + __llvm_libc::File::FULLY_BUFFERED, false, "a"); f->reset_and_fill(initial_content, sizeof(initial_content)); constexpr size_t READ_SIZE = 5; @@ -246,7 +296,8 @@ 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, + __llvm_libc::File::FULLY_BUFFERED, 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 @@ -266,7 +317,8 @@ 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, + __llvm_libc::File::FULLY_BUFFERED, false, "r+"); f->reset_and_fill(initial_content, sizeof(initial_content)); constexpr size_t READ_SIZE = sizeof(initial_content) / 2; @@ -300,7 +352,8 @@ 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, + __llvm_libc::File::FULLY_BUFFERED, false, "a+"); f->reset_and_fill(initial_content, sizeof(initial_content)); ASSERT_EQ(sizeof(data), f->write(data, sizeof(data)));