[libc-commits] [libc] [libc] Support _IONBF buffering for read_unlocked (PR #120677)
Jack Huang via libc-commits
libc-commits at lists.llvm.org
Tue Jan 7 18:53:46 PST 2025
https://github.com/jackhong12 updated https://github.com/llvm/llvm-project/pull/120677
>From 2bea94078fe0ad94969634cdadbb3c7444c09af9 Mon Sep 17 00:00:00 2001
From: jack <jackhuang1205 at gmail.com>
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 972249fef96bcf..cb8e1dff9fe96e 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<uint8_t *>(data), len);
+ } else if (bufmode == _IOFBF) { // fully buffered
+ return read_unlocked_fbf(static_cast<uint8_t *>(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<uint8_t *>(data), len);
+ }
+}
+
+FileIOResult File::read_unlocked_fbf(uint8_t *data, size_t len) {
cpp::span<uint8_t> bufref(static_cast<uint8_t *>(buf), bufsize);
cpp::span<uint8_t> dataref(static_cast<uint8_t *>(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 42e1d11b4ab1a0..548f051a1b9f5c 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 <jackhuang1205 at gmail.com>
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 cb8e1dff9fe96e..8b70db32d4876c 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<uint8_t> bufref(static_cast<uint8_t *>(buf), bufsize);
cpp::span<uint8_t> dataref(static_cast<uint8_t *>(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<uint8_t>(dataref.data() + available_data,
- dataref.size() - available_data);
-
size_t to_fetch = len - available_data;
+ cpp::span<uint8_t> dataref(static_cast<uint8_t *>(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<uint8_t> dataref(static_cast<uint8_t *>(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 548f051a1b9f5c..d7aa824486c567 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 <jackhuang1205 at gmail.com>
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 8b70db32d4876c..3660cf710424e7 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<uint8_t *>(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<uint8_t *>(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<uint8_t> bufref(static_cast<uint8_t *>(buf), bufsize);
cpp::span<uint8_t> dataref(static_cast<uint8_t *>(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 d7aa824486c567..5c97a9c6419f07 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 <jackhuang1205 at gmail.com>
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 3660cf710424e7..5dcddd6e1a3ad6 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<uint8_t> dataref(static_cast<uint8_t *>(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<int> 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<off_t> 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 <jackhuang1205 at gmail.com>
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 5dcddd6e1a3ad6..528542cccf324c 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
More information about the libc-commits
mailing list