Skip to content

Commit

Permalink
[libc] Remove the requirement of a platform-flush operation in File a…
Browse files Browse the repository at this point in the history
…bstraction.

The libc flush operation is not supposed to trigger a platform level
flush operation. See "Notes" on this Linux man page:
    https://man7.org/linux/man-pages/man3/fflush.3.html

Reviewed By: michaelrj

Differential Revision: https://reviews.llvm.org/D153182
  • Loading branch information
Siva Chandra Reddy committed Jun 19, 2023
1 parent 2da4517 commit 21e1651
Show file tree
Hide file tree
Showing 7 changed files with 12 additions and 33 deletions.
1 change: 0 additions & 1 deletion libc/src/__support/File/file.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,6 @@ int File::flush_unlocked() {
return buf_result.error;
}
pos = 0;
return platform_flush(this);
}
// TODO: Add POSIX behavior for input streams.
return 0;
Expand Down
16 changes: 6 additions & 10 deletions libc/src/__support/File/file.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ class File {
// file position indicator.
using SeekFunc = ErrorOr<long>(File *, long, int);
using CloseFunc = int(File *);
using FlushFunc = int(File *);
// CleanupFunc is a function which does the equivalent of this:
//
// void my_file_cleanup(File *f) {
Expand Down Expand Up @@ -103,7 +102,6 @@ class File {
ReadFunc *platform_read;
SeekFunc *platform_seek;
CloseFunc *platform_close;
FlushFunc *platform_flush;
CleanupFunc *platform_cleanup;

Mutex mutex;
Expand Down Expand Up @@ -202,15 +200,13 @@ class File {
// is zero. This way, we will not have to employ the semantics of
// the set_buffer method and allocate a buffer.
constexpr File(WriteFunc *wf, ReadFunc *rf, SeekFunc *sf, CloseFunc *cf,
FlushFunc *ff, CleanupFunc *clf, uint8_t *buffer,
size_t buffer_size, int buffer_mode, bool owned,
ModeFlags modeflags)
CleanupFunc *clf, uint8_t *buffer, size_t buffer_size,
int buffer_mode, bool owned, ModeFlags modeflags)
: platform_write(wf), platform_read(rf), platform_seek(sf),
platform_close(cf), platform_flush(ff), platform_cleanup(clf),
mutex(false, false, 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) {
platform_close(cf), platform_cleanup(clf), mutex(false, false, 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) {
if constexpr (ENABLE_BUFFER)
adjust_buf();
}
Expand Down
4 changes: 2 additions & 2 deletions libc/src/__support/File/gpu/file.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ class GPUFile : public File {

public:
constexpr GPUFile(uintptr_t file, File::ModeFlags modeflags)
: File(&write_func, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr,
0, _IONBF, false, modeflags),
: File(&write_func, nullptr, nullptr, nullptr, nullptr, nullptr, 0,
_IONBF, false, modeflags),
file(file) {}

uintptr_t get_file() const { return file; }
Expand Down
12 changes: 1 addition & 11 deletions libc/src/__support/File/linux/file.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ FileIOResult write_func(File *, const void *, size_t);
FileIOResult read_func(File *, void *, size_t);
ErrorOr<long> seek_func(File *, long, int);
int close_func(File *);
int flush_func(File *);

} // anonymous namespace

Expand All @@ -34,7 +33,7 @@ class LinuxFile : public File {
public:
constexpr LinuxFile(int file_descriptor, uint8_t *buffer, size_t buffer_size,
int buffer_mode, bool owned, File::ModeFlags modeflags)
: File(&write_func, &read_func, &seek_func, &close_func, flush_func,
: File(&write_func, &read_func, &seek_func, &close_func,
&cleanup_file<LinuxFile>, buffer, buffer_size, buffer_mode, owned,
modeflags),
fd(file_descriptor) {}
Expand Down Expand Up @@ -91,15 +90,6 @@ int close_func(File *f) {
return 0;
}

int flush_func(File *f) {
auto *lf = reinterpret_cast<LinuxFile *>(f);
int ret = __llvm_libc::syscall_impl(SYS_fsync, lf->get_fd());
if (ret < 0) {
return -ret;
}
return 0;
}

} // anonymous namespace

ErrorOr<File *> openfile(const char *path, const char *mode) {
Expand Down
8 changes: 2 additions & 6 deletions libc/src/stdio/fopencookie.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,13 @@ class CookieFile : public __llvm_libc::File {
static FileIOResult cookie_read(File *f, void *data, size_t size);
static ErrorOr<long> cookie_seek(File *f, long offset, int whence);
static int cookie_close(File *f);
static int cookie_flush(File *);

public:
CookieFile(void *c, cookie_io_functions_t cops, uint8_t *buffer,
size_t bufsize, File::ModeFlags mode)
: File(&cookie_write, &cookie_read, &CookieFile::cookie_seek,
&cookie_close, &cookie_flush, &cleanup_file<CookieFile>, buffer,
bufsize, 0 /* default buffering mode */,
true /* File owns buffer */, mode),
&cookie_close, &cleanup_file<CookieFile>, buffer, bufsize,
0 /* default buffering mode */, true /* File owns buffer */, mode),
cookie(c), ops(cops) {}
};

Expand Down Expand Up @@ -74,8 +72,6 @@ int CookieFile::cookie_close(File *f) {
return cookie_file->ops.close(cookie_file->cookie);
}

int CookieFile::cookie_flush(File *) { return 0; }

} // anonymous namespace

LLVM_LIBC_FUNCTION(::FILE *, fopencookie,
Expand Down
3 changes: 1 addition & 2 deletions libc/test/src/__support/File/file_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,12 @@ class StringFile : public File {
size_t len);
static ErrorOr<long> str_seek(__llvm_libc::File *f, long offset, int whence);
static int str_close(__llvm_libc::File *f) { return 0; }
static int str_flush(__llvm_libc::File *f) { return 0; }

public:
explicit StringFile(char *buffer, size_t buflen, int bufmode, bool owned,
ModeFlags modeflags)
: __llvm_libc::File(&str_write, &str_read, &str_seek, &str_close,
&str_flush, &__llvm_libc::cleanup_file<StringFile>,
&__llvm_libc::cleanup_file<StringFile>,
reinterpret_cast<uint8_t *>(buffer), buflen, bufmode,
owned, modeflags),
pos(0), eof_marker(0), write_append(false) {
Expand Down
1 change: 0 additions & 1 deletion libc/test/src/stdio/ftell_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
//===----------------------------------------------------------------------===//

#include "src/stdio/fclose.h"
#include "src/stdio/fflush.h"
#include "src/stdio/fopen.h"
#include "src/stdio/fread.h"
#include "src/stdio/fseek.h"
Expand Down

0 comments on commit 21e1651

Please sign in to comment.