From 2bea94078fe0ad94969634cdadbb3c7444c09af9 Mon Sep 17 00:00:00 2001 From: jack Date: Fri, 20 Dec 2024 11:25:52 +0800 Subject: [PATCH 1/5] [libc] Support _IONBF buffering for read_unlocked - Add the functions read_unlocked_nbf and read_unlocked_fbf. --- libc/src/__support/File/file.cpp | 23 +++++++++++++++++++++++ libc/src/__support/File/file.h | 3 +++ 2 files changed, 26 insertions(+) diff --git a/libc/src/__support/File/file.cpp b/libc/src/__support/File/file.cpp index 972249fef96bc..cb8e1dff9fe96 100644 --- a/libc/src/__support/File/file.cpp +++ b/libc/src/__support/File/file.cpp @@ -190,6 +190,17 @@ FileIOResult File::read_unlocked(void *data, size_t len) { prev_op = FileOp::READ; + if (bufmode == _IONBF) { // unbuffered. + return read_unlocked_nbf(static_cast(data), len); + } else if (bufmode == _IOFBF) { // fully buffered + return read_unlocked_fbf(static_cast(data), len); + } else /*if (bufmode == _IOLBF) */ { // line buffered + // There is no line buffered mode for read. Use fully buffer instead. + return read_unlocked_fbf(static_cast(data), len); + } +} + +FileIOResult File::read_unlocked_fbf(uint8_t *data, size_t len) { cpp::span bufref(static_cast(buf), bufsize); cpp::span dataref(static_cast(data), len); @@ -245,6 +256,18 @@ FileIOResult File::read_unlocked(void *data, size_t len) { return {transfer_size + available_data, result.error}; } +FileIOResult File::read_unlocked_nbf(uint8_t *data, size_t len) { + auto result = platform_read(this, data, len); + + if (result.has_error() || result < len) { + if (!result.has_error()) + eof = true; + else + err = true; + } + return result; +} + int File::ungetc_unlocked(int c) { // There is no meaning to unget if: // 1. You are trying to push back EOF. diff --git a/libc/src/__support/File/file.h b/libc/src/__support/File/file.h index 42e1d11b4ab1a..548f051a1b9f5 100644 --- a/libc/src/__support/File/file.h +++ b/libc/src/__support/File/file.h @@ -280,6 +280,9 @@ class File { FileIOResult write_unlocked_fbf(const uint8_t *data, size_t len); FileIOResult write_unlocked_nbf(const uint8_t *data, size_t len); + FileIOResult read_unlocked_fbf(uint8_t *data, size_t len); + FileIOResult read_unlocked_nbf(uint8_t *data, size_t len); + constexpr void adjust_buf() { if (read_allowed() && (buf == nullptr || bufsize == 0)) { // We should allow atleast one ungetc operation. From 26e848f1cb709ccc7331286fa594acb68070e18c Mon Sep 17 00:00:00 2001 From: jack Date: Fri, 20 Dec 2024 17:56:45 +0800 Subject: [PATCH 2/5] [libc] Copy the data from the buffer first. For read_unlocked() with the _IONBUF mode, there is still an one-character buffer which is used to store the data inserted by ungetc(), so we always need to check the buffer first. --- libc/src/__support/File/file.cpp | 34 ++++++++++++++++++++++++-------- libc/src/__support/File/file.h | 1 + 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/libc/src/__support/File/file.cpp b/libc/src/__support/File/file.cpp index cb8e1dff9fe96..8b70db32d4876 100644 --- a/libc/src/__support/File/file.cpp +++ b/libc/src/__support/File/file.cpp @@ -200,7 +200,7 @@ FileIOResult File::read_unlocked(void *data, size_t len) { } } -FileIOResult File::read_unlocked_fbf(uint8_t *data, size_t len) { +FileIOResult File::copy_data_from_buf(uint8_t *data, size_t len) { cpp::span bufref(static_cast(buf), bufsize); cpp::span dataref(static_cast(data), len); @@ -220,12 +220,22 @@ FileIOResult File::read_unlocked_fbf(uint8_t *data, size_t len) { for (size_t i = 0; i < available_data; ++i) dataref[i] = bufref[i + pos]; read_limit = pos = 0; // Reset the pointers. + + return available_data; +} + +FileIOResult File::read_unlocked_fbf(uint8_t *data, size_t len) { + // Read data from the buffer first. + size_t available_data = copy_data_from_buf(data, len); + if (available_data == len) + return available_data; + // Update the dataref to reflect that fact that we have already // copied |available_data| into |data|. - dataref = cpp::span(dataref.data() + available_data, - dataref.size() - available_data); - size_t to_fetch = len - available_data; + cpp::span dataref(static_cast(data) + available_data, + to_fetch); + if (to_fetch > bufsize) { auto result = platform_read(this, dataref.data(), to_fetch); size_t fetched_size = result.value; @@ -245,7 +255,7 @@ FileIOResult File::read_unlocked_fbf(uint8_t *data, size_t len) { read_limit += fetched_size; size_t transfer_size = fetched_size >= to_fetch ? to_fetch : fetched_size; for (size_t i = 0; i < transfer_size; ++i) - dataref[i] = bufref[i]; + dataref[i] = buf[i]; pos += transfer_size; if (result.has_error() || fetched_size < to_fetch) { if (!result.has_error()) @@ -257,15 +267,23 @@ FileIOResult File::read_unlocked_fbf(uint8_t *data, size_t len) { } FileIOResult File::read_unlocked_nbf(uint8_t *data, size_t len) { - auto result = platform_read(this, data, len); + // Check whether there is a character in the ungetc buffer. + size_t available_data = copy_data_from_buf(data, len); + if (available_data == len) + return available_data; + + // Directly copy the data into |data|. + cpp::span dataref(static_cast(data) + available_data, + len - available_data); + auto result = platform_read(this, dataref.data(), dataref.size()); - if (result.has_error() || result < len) { + if (result.has_error() || result < dataref.size()) { if (!result.has_error()) eof = true; else err = true; } - return result; + return {result + available_data, result.has_error()}; } int File::ungetc_unlocked(int c) { diff --git a/libc/src/__support/File/file.h b/libc/src/__support/File/file.h index 548f051a1b9f5..d7aa824486c56 100644 --- a/libc/src/__support/File/file.h +++ b/libc/src/__support/File/file.h @@ -282,6 +282,7 @@ class File { FileIOResult read_unlocked_fbf(uint8_t *data, size_t len); FileIOResult read_unlocked_nbf(uint8_t *data, size_t len); + FileIOResult copy_data_from_buf(uint8_t *data, size_t len); constexpr void adjust_buf() { if (read_allowed() && (buf == nullptr || bufsize == 0)) { From 14e60be2c0c2262c09494b5f3d6ee9fc106cf06a Mon Sep 17 00:00:00 2001 From: jack Date: Tue, 7 Jan 2025 14:41:07 +0800 Subject: [PATCH 3/5] [libc] Change the code according to the review - The function copy_data_from_buf returns size_t. - Fix the spelling error. - Fix the incorrect data, error, in FileIOResult. --- libc/src/__support/File/file.cpp | 8 ++++---- libc/src/__support/File/file.h | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/libc/src/__support/File/file.cpp b/libc/src/__support/File/file.cpp index 8b70db32d4876..3660cf710424e 100644 --- a/libc/src/__support/File/file.cpp +++ b/libc/src/__support/File/file.cpp @@ -195,12 +195,12 @@ FileIOResult File::read_unlocked(void *data, size_t len) { } else if (bufmode == _IOFBF) { // fully buffered return read_unlocked_fbf(static_cast(data), len); } else /*if (bufmode == _IOLBF) */ { // line buffered - // There is no line buffered mode for read. Use fully buffer instead. + // There is no line buffered mode for read. Use fully buffered instead. return read_unlocked_fbf(static_cast(data), len); } } -FileIOResult File::copy_data_from_buf(uint8_t *data, size_t len) { +size_t File::copy_data_from_buf(uint8_t *data, size_t len) { cpp::span bufref(static_cast(buf), bufsize); cpp::span dataref(static_cast(data), len); @@ -244,7 +244,7 @@ FileIOResult File::read_unlocked_fbf(uint8_t *data, size_t len) { eof = true; else err = true; - return {available_data + fetched_size, result.has_error()}; + return {available_data + fetched_size, result.error}; } return len; } @@ -283,7 +283,7 @@ FileIOResult File::read_unlocked_nbf(uint8_t *data, size_t len) { else err = true; } - return {result + available_data, result.has_error()}; + return {result + available_data, result.error}; } int File::ungetc_unlocked(int c) { diff --git a/libc/src/__support/File/file.h b/libc/src/__support/File/file.h index d7aa824486c56..5c97a9c6419f0 100644 --- a/libc/src/__support/File/file.h +++ b/libc/src/__support/File/file.h @@ -282,7 +282,7 @@ class File { FileIOResult read_unlocked_fbf(uint8_t *data, size_t len); FileIOResult read_unlocked_nbf(uint8_t *data, size_t len); - FileIOResult copy_data_from_buf(uint8_t *data, size_t len); + size_t copy_data_from_buf(uint8_t *data, size_t len); constexpr void adjust_buf() { if (read_allowed() && (buf == nullptr || bufsize == 0)) { From e0cdb2679bb1e09f51efc32341def6b5ea6a5489 Mon Sep 17 00:00:00 2001 From: jack Date: Tue, 7 Jan 2025 19:01:26 +0800 Subject: [PATCH 4/5] [libc] Use the specific data type FileIOResult --- libc/src/__support/File/file.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/libc/src/__support/File/file.cpp b/libc/src/__support/File/file.cpp index 3660cf710424e..5dcddd6e1a3ad 100644 --- a/libc/src/__support/File/file.cpp +++ b/libc/src/__support/File/file.cpp @@ -42,7 +42,7 @@ FileIOResult File::write_unlocked_nbf(const uint8_t *data, size_t len) { if (pos > 0) { // If the buffer is not empty // Flush the buffer const size_t write_size = pos; - auto write_result = platform_write(this, buf, write_size); + FileIOResult write_result = 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 (write_result < write_size) { @@ -52,7 +52,7 @@ FileIOResult File::write_unlocked_nbf(const uint8_t *data, size_t len) { } } - auto write_result = platform_write(this, data, len); + FileIOResult write_result = platform_write(this, data, len); if (write_result < len) err = true; return write_result; @@ -99,7 +99,7 @@ FileIOResult File::write_unlocked_fbf(const uint8_t *data, size_t len) { // is full. const size_t write_size = pos; - auto buf_result = platform_write(this, buf, write_size); + FileIOResult buf_result = platform_write(this, buf, write_size); size_t bytes_written = buf_result.value; pos = 0; // Buffer is now empty so reset pos to the beginning. @@ -121,7 +121,7 @@ FileIOResult File::write_unlocked_fbf(const uint8_t *data, size_t len) { pos = remainder.size(); } else { - auto result = platform_write(this, remainder.data(), remainder.size()); + FileIOResult result = platform_write(this, remainder.data(), remainder.size()); size_t bytes_written = buf_result.value; // If less bytes were written than expected, then an error occurred. Return @@ -237,7 +237,7 @@ FileIOResult File::read_unlocked_fbf(uint8_t *data, size_t len) { to_fetch); if (to_fetch > bufsize) { - auto result = platform_read(this, dataref.data(), to_fetch); + FileIOResult result = platform_read(this, dataref.data(), to_fetch); size_t fetched_size = result.value; if (result.has_error() || fetched_size < to_fetch) { if (!result.has_error()) @@ -250,7 +250,7 @@ FileIOResult File::read_unlocked_fbf(uint8_t *data, size_t len) { } // Fetch and buffer another buffer worth of data. - auto result = platform_read(this, buf, bufsize); + FileIOResult result = platform_read(this, buf, bufsize); size_t fetched_size = result.value; read_limit += fetched_size; size_t transfer_size = fetched_size >= to_fetch ? to_fetch : fetched_size; @@ -275,7 +275,7 @@ FileIOResult File::read_unlocked_nbf(uint8_t *data, size_t len) { // Directly copy the data into |data|. cpp::span dataref(static_cast(data) + available_data, len - available_data); - auto result = platform_read(this, dataref.data(), dataref.size()); + FileIOResult result = platform_read(this, dataref.data(), dataref.size()); if (result.has_error() || result < dataref.size()) { if (!result.has_error()) @@ -328,7 +328,7 @@ ErrorOr File::seek(off_t offset, int whence) { FileLock lock(this); if (prev_op == FileOp::WRITE && pos > 0) { - auto buf_result = platform_write(this, buf, pos); + FileIOResult buf_result = platform_write(this, buf, pos); if (buf_result.has_error() || buf_result.value < pos) { err = true; return Error(buf_result.error); @@ -366,7 +366,7 @@ ErrorOr File::tell() { int File::flush_unlocked() { if (prev_op == FileOp::WRITE && pos > 0) { - auto buf_result = platform_write(this, buf, pos); + FileIOResult buf_result = platform_write(this, buf, pos); if (buf_result.has_error() || buf_result.value < pos) { err = true; return buf_result.error; From 3db5bcdc3a767450bcef2697228b56fb8e268f2c Mon Sep 17 00:00:00 2001 From: jack Date: Tue, 7 Jan 2025 19:14:41 +0800 Subject: [PATCH 5/5] [libc] Format the change --- libc/src/__support/File/file.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libc/src/__support/File/file.cpp b/libc/src/__support/File/file.cpp index 5dcddd6e1a3ad..528542cccf324 100644 --- a/libc/src/__support/File/file.cpp +++ b/libc/src/__support/File/file.cpp @@ -121,7 +121,8 @@ FileIOResult File::write_unlocked_fbf(const uint8_t *data, size_t len) { pos = remainder.size(); } else { - FileIOResult result = platform_write(this, remainder.data(), remainder.size()); + FileIOResult result = + platform_write(this, remainder.data(), remainder.size()); size_t bytes_written = buf_result.value; // If less bytes were written than expected, then an error occurred. Return