[libc-commits] [libc] [libc] implement fflush(NULL) support (PR #188217)
via libc-commits
libc-commits at lists.llvm.org
Tue Mar 24 03:55:36 PDT 2026
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-libc
Author: Jeff Bailey (kaladron)
<details>
<summary>Changes</summary>
Implement support for flushing all open streams when fflush is called with a NULL pointer.
* Added a global linked list to track all open File objects.
* Updated File class to include prev/next pointers and list management methods.
* Implemented POSIX requirement for fflush to sync seekable input streams back to the host environment.
* Updated Linux-specific file creation to register new files in the global list.
* Fixed a memory safety bug in create_file_from_fd using delete instead of free.
* Added unit test for fflush(NULL).
* Added explanatory comments to fflush.cpp and file.cpp.
---
Full diff: https://github.com/llvm/llvm-project/pull/188217.diff
6 Files Affected:
- (modified) libc/src/__support/File/file.cpp (+41-1)
- (modified) libc/src/__support/File/file.h (+16-3)
- (modified) libc/src/__support/File/linux/file.cpp (+5-1)
- (modified) libc/src/stdio/generic/CMakeLists.txt (+4)
- (modified) libc/src/stdio/generic/fflush.cpp (+39-3)
- (modified) libc/test/src/stdio/fileop_test.cpp (+20)
``````````diff
diff --git a/libc/src/__support/File/file.cpp b/libc/src/__support/File/file.cpp
index 07af685f99663..7b38c543d748d 100644
--- a/libc/src/__support/File/file.cpp
+++ b/libc/src/__support/File/file.cpp
@@ -19,6 +19,38 @@
#include "src/string/memory_utils/inline_memcpy.h"
namespace LIBC_NAMESPACE_DECL {
+File *File::list_all = nullptr;
+Mutex File::list_lock(/*timed=*/false, /*recursive=*/false, /*robust=*/false,
+ /*pshared=*/false);
+
+// Adds a file to the global list of all open files.
+void File::add_file(File *f) {
+ File::list_lock.lock();
+ f->next = File::list_all;
+ f->prev = nullptr;
+ if (File::list_all != nullptr)
+ File::list_all->prev = f;
+ File::list_all = f;
+ File::list_lock.unlock();
+}
+
+// Removes a file from the global list of all open files.
+void File::remove_file(File *f) {
+ File::list_lock.lock();
+ if (f->prev != nullptr)
+ f->prev->next = f->next;
+ if (f->next != nullptr)
+ f->next->prev = f->prev;
+ if (File::list_all == f)
+ File::list_all = f->next;
+ f->prev = nullptr;
+ f->next = nullptr;
+ File::list_lock.unlock();
+}
+
+File *File::get_first_file() { return File::list_all; }
+void File::lock_list() { File::list_lock.lock(); }
+void File::unlock_list() { File::list_lock.unlock(); }
FileIOResult File::write_unlocked(const void *data, size_t len) {
if (!write_allowed()) {
@@ -368,8 +400,16 @@ int File::flush_unlocked() {
return buf_result.error;
}
pos = 0;
+ } else if (prev_op == FileOp::READ) {
+ if (read_limit > pos) {
+ if (!platform_seek(this, -static_cast<off_t>(read_limit - pos), SEEK_CUR)
+ .has_value()) {
+ // We ignore seek errors for non-seekable files (like pipes) as per
+ // POSIX.
+ }
+ }
+ pos = read_limit = 0;
}
- // TODO: Add POSIX behavior for input streams.
return 0;
}
diff --git a/libc/src/__support/File/file.h b/libc/src/__support/File/file.h
index 1ce60bc01806e..c569f24995a9b 100644
--- a/libc/src/__support/File/file.h
+++ b/libc/src/__support/File/file.h
@@ -39,6 +39,17 @@ struct FileIOResult {
// suitable for their platform.
class File {
public:
+ static void add_file(File *f);
+ static void remove_file(File *f);
+ static File *get_first_file();
+ static void lock_list();
+ static void unlock_list();
+
+ static File *list_all;
+ static Mutex list_lock;
+
+ File *get_next() const { return next; }
+
static constexpr size_t DEFAULT_BUFFER_SIZE = 1024;
using LockFunc = void(File *);
@@ -161,7 +172,7 @@ class File {
/*robust=*/false, /*pshared=*/false),
ungetc_buf(0), buf(buffer), bufsize(buffer_size), bufmode(buffer_mode),
own_buf(owned), mode(modeflags), pos(0), prev_op(FileOp::NONE),
- read_limit(0), eof(false), err(false) {
+ read_limit(0), eof(false), err(false), prev(nullptr), next(nullptr) {
adjust_buf();
}
@@ -310,12 +321,14 @@ class File {
// affect the behavior experienced by the user.
buf = &ungetc_buf;
bufsize = 1;
- own_buf = false; // We shouldn't call free on |buf| when closing the file.
}
}
+
+ File *prev;
+ File *next;
};
-// The implementaiton of this function is provided by the platform_file
+// The implementation of this function is provided by the platform_file
// library.
ErrorOr<File *> openfile(const char *path, const char *mode);
diff --git a/libc/src/__support/File/linux/file.cpp b/libc/src/__support/File/linux/file.cpp
index 28f819db83d49..13065300e5a34 100644
--- a/libc/src/__support/File/linux/file.cpp
+++ b/libc/src/__support/File/linux/file.cpp
@@ -54,6 +54,7 @@ ErrorOr<off_t> linux_file_seek(File *f, off_t offset, int whence) {
}
int linux_file_close(File *f) {
+ File::remove_file(f);
auto *lf = reinterpret_cast<LinuxFile *>(f);
int ret = LIBC_NAMESPACE::syscall_impl<int>(SYS_close, lf->get_fd());
if (ret < 0) {
@@ -119,6 +120,7 @@ ErrorOr<File *> openfile(const char *path, const char *mode) {
LinuxFile(fd, buffer, File::DEFAULT_BUFFER_SIZE, _IOFBF, true, modeflags);
if (!ac)
return Error(ENOMEM);
+ File::add_file(file);
return file;
}
@@ -168,10 +170,12 @@ ErrorOr<LinuxFile *> create_file_from_fd(int fd, const char *mode) {
if (!ac) {
return Error(ENOMEM);
}
+ File::add_file(file);
if (do_seek) {
auto result = file->seek(0, SEEK_END);
if (!result.has_value()) {
- free(file);
+ File::remove_file(file);
+ delete file;
return Error(result.error());
}
}
diff --git a/libc/src/stdio/generic/CMakeLists.txt b/libc/src/stdio/generic/CMakeLists.txt
index 0641bd4634297..9cde0980d6f32 100644
--- a/libc/src/stdio/generic/CMakeLists.txt
+++ b/libc/src/stdio/generic/CMakeLists.txt
@@ -105,8 +105,12 @@ add_generic_entrypoint_object(
libc.hdr.types.FILE
libc.src.__support.File.file
libc.src.__support.File.platform_file
+ libc.src.stdio.stdin
+ libc.src.stdio.stdout
+ libc.src.stdio.stderr
)
+
add_generic_entrypoint_object(
fseek
SRCS
diff --git a/libc/src/stdio/generic/fflush.cpp b/libc/src/stdio/generic/fflush.cpp
index d0271d9154c87..1455b720b6de9 100644
--- a/libc/src/stdio/generic/fflush.cpp
+++ b/libc/src/stdio/generic/fflush.cpp
@@ -16,9 +16,45 @@
namespace LIBC_NAMESPACE_DECL {
LLVM_LIBC_FUNCTION(int, fflush, (::FILE * stream)) {
- int result = reinterpret_cast<LIBC_NAMESPACE::File *>(stream)->flush();
- if (result != 0) {
- libc_errno = result;
+ // If a non-null stream is specified, we only flush that single stream.
+ if (stream != nullptr) {
+ int result = reinterpret_cast<LIBC_NAMESPACE::File *>(stream)->flush();
+ if (result != 0) {
+ libc_errno = result;
+ return EOF;
+ }
+ return 0;
+ }
+
+ // If the stream is null, we flush all open streams as per C and POSIX
+ // requirements.
+ int total_error = 0;
+
+ // We explicitly flush the standard streams as they may not be part of the
+ // global file list if they are statically initialized.
+ LIBC_NAMESPACE::File *std_streams[] = {
+ LIBC_NAMESPACE::stdin, LIBC_NAMESPACE::stdout, LIBC_NAMESPACE::stderr};
+ for (auto *s : std_streams) {
+ if (s != nullptr) {
+ int result = s->flush();
+ if (result != 0)
+ total_error = result;
+ }
+ }
+
+ // We iterate over the global list of all open File objects to flush any
+ // other streams that were opened via fopen.
+ LIBC_NAMESPACE::File::lock_list();
+ for (LIBC_NAMESPACE::File *f = LIBC_NAMESPACE::File::get_first_file();
+ f != nullptr; f = f->get_next()) {
+ int result = f->flush();
+ if (result != 0)
+ total_error = result;
+ }
+ LIBC_NAMESPACE::File::unlock_list();
+
+ if (total_error != 0) {
+ libc_errno = total_error;
return EOF;
}
return 0;
diff --git a/libc/test/src/stdio/fileop_test.cpp b/libc/test/src/stdio/fileop_test.cpp
index ed4ba26ed8734..8f6d5d5918646 100644
--- a/libc/test/src/stdio/fileop_test.cpp
+++ b/libc/test/src/stdio/fileop_test.cpp
@@ -148,6 +148,26 @@ TEST_F(LlvmLibcFILETest, FFlush) {
ASSERT_EQ(LIBC_NAMESPACE::fclose(file), 0);
}
+TEST_F(LlvmLibcFILETest, FFlushNull) {
+ constexpr char FILENAME[] = APPEND_LIBC_TEST("testdata/fflush_null.test");
+ ::FILE *file = LIBC_NAMESPACE::fopen(FILENAME, "w+");
+ ASSERT_FALSE(file == nullptr);
+ constexpr char CONTENT[] = "1234567890987654321";
+ ASSERT_EQ(sizeof(CONTENT),
+ LIBC_NAMESPACE::fwrite(CONTENT, 1, sizeof(CONTENT), file));
+
+ // Flushing with NULL should flush all streams, including this one.
+ ASSERT_EQ(0, LIBC_NAMESPACE::fflush(nullptr));
+
+ char data[sizeof(CONTENT)];
+ ASSERT_EQ(LIBC_NAMESPACE::fseek(file, 0, SEEK_SET), 0);
+ ASSERT_EQ(LIBC_NAMESPACE::fread(data, 1, sizeof(CONTENT), file),
+ sizeof(CONTENT));
+ ASSERT_STREQ(data, CONTENT);
+
+ ASSERT_EQ(LIBC_NAMESPACE::fclose(file), 0);
+}
+
TEST_F(LlvmLibcFILETest, FOpenFWriteSizeGreaterThanOne) {
using MyStruct = struct {
char c;
``````````
</details>
https://github.com/llvm/llvm-project/pull/188217
More information about the libc-commits
mailing list