[libc-commits] [libc] [libc] Support _IONBF buffering for read_unlocked (PR #120677)

[Jack Huang via libc-commits]

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/2] [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 394dd062b1c49f361885b31cf38d47edfc7f0ce6 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/2] [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 | 32 ++++++++++++++++++++++++--------
 libc/src/__support/File/file.h   |  1 +
 2 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/libc/src/__support/File/file.cpp b/libc/src/__support/File/file.cpp
index cb8e1dff9fe96e..492994785474b3 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,21 @@ 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 +254,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 +266,22 @@ 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)) {