Skip to content

Commit d194975

Browse files
mleleszimleleszi
committed
Return system IO errors return values
1 parent 6d05774 commit d194975

File tree

8 files changed

+73
-75
lines changed

8 files changed

+73
-75
lines changed

‎libc/src/__support/File/file.cpp‎

Lines changed: 28 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ namespace LIBC_NAMESPACE_DECL {
2020

2121
FileIOResult File::write_unlocked(const void *data, size_t len) {
2222
if (!write_allowed()) {
23-
err_code = EBADF;
23+
err = true;
2424
return {0, EBADF};
2525
}
2626

@@ -45,16 +45,16 @@ FileIOResult File::write_unlocked_nbf(const uint8_t *data, size_t len) {
4545
FileIOResult write_result = platform_write(this, buf, write_size);
4646
pos = 0; // Buffer is now empty so reset pos to the beginning.
4747
// If less bytes were written than expected, then an error occurred.
48-
if (write_result.has_error() || write_result < write_size) {
49-
err_code = write_result.has_error() ? write_result.error : EIO;
48+
if (write_result < write_size) {
49+
err = true;
5050
// No bytes from data were written, so return 0.
51-
return {0, err_code};
51+
return {0, write_result.error};
5252
}
5353
}
5454

5555
FileIOResult write_result = platform_write(this, data, len);
56-
if (write_result.has_error() || write_result < len)
57-
err_code = write_result.has_error() ? write_result.error : EIO;
56+
if (write_result < len)
57+
err = true;
5858
return write_result;
5959
}
6060

@@ -106,8 +106,9 @@ FileIOResult File::write_unlocked_fbf(const uint8_t *data, size_t len) {
106106
// If less bytes were written than expected, then an error occurred. Return
107107
// the number of bytes that have been written from |data|.
108108
if (buf_result.has_error() || bytes_written < write_size) {
109-
err_code = buf_result.has_error() ? buf_result.error : EIO;
110-
return {bytes_written <= init_pos ? 0 : bytes_written - init_pos, err_code};
109+
err = true;
110+
return {bytes_written <= init_pos ? 0 : bytes_written - init_pos,
111+
buf_result.error};
111112
}
112113

113114
// The second piece is handled basically the same as the first, although we
@@ -127,8 +128,8 @@ FileIOResult File::write_unlocked_fbf(const uint8_t *data, size_t len) {
127128
// If less bytes were written than expected, then an error occurred. Return
128129
// the number of bytes that have been written from |data|.
129130
if (result.has_error() || bytes_written < remainder.size()) {
130-
err_code = result.has_error() ? result.error : EIO;
131-
return {primary.size() + bytes_written, err_code};
131+
err = true;
132+
return {primary.size() + bytes_written, result.error};
132133
}
133134
}
134135

@@ -165,29 +166,26 @@ FileIOResult File::write_unlocked_lbf(const uint8_t *data, size_t len) {
165166

166167
size_t written = 0;
167168

168-
auto write_result = write_unlocked_nbf(primary.data(), primary.size());
169-
written += write_result;
170-
if (write_result.has_error() || written < primary.size()) {
171-
err_code = write_result.has_error() ? write_result.error : EIO;
172-
return {written, err_code};
169+
written = write_unlocked_nbf(primary.data(), primary.size());
170+
if (written < primary.size()) {
171+
err = true;
172+
return written;
173173
}
174174

175175
flush_unlocked();
176176

177-
write_result = write_unlocked_fbf(remainder.data(), remainder.size());
178-
written += write_result;
179-
;
180-
if (write_result.has_error() || written < len) {
181-
err_code = write_result.has_error() ? write_result.error : EIO;
182-
return {written, err_code};
177+
written += write_unlocked_fbf(remainder.data(), remainder.size());
178+
if (written < len) {
179+
err = true;
180+
return written;
183181
}
184182

185183
return len;
186184
}
187185

188186
FileIOResult File::read_unlocked(void *data, size_t len) {
189187
if (!read_allowed()) {
190-
err_code = EBADF;
188+
err = true;
191189
return {0, EBADF};
192190
}
193191

@@ -246,7 +244,7 @@ FileIOResult File::read_unlocked_fbf(uint8_t *data, size_t len) {
246244
if (!result.has_error())
247245
eof = true;
248246
else
249-
err_code = result.error;
247+
err = true;
250248
return {available_data + fetched_size, result.error};
251249
}
252250
return len;
@@ -264,7 +262,7 @@ FileIOResult File::read_unlocked_fbf(uint8_t *data, size_t len) {
264262
if (!result.has_error())
265263
eof = true;
266264
else
267-
err_code = result.error;
265+
err = true;
268266
}
269267
return {transfer_size + available_data, result.error};
270268
}
@@ -284,7 +282,7 @@ FileIOResult File::read_unlocked_nbf(uint8_t *data, size_t len) {
284282
if (!result.has_error())
285283
eof = true;
286284
else
287-
err_code = result.error;
285+
err = true;
288286
}
289287
return {result + available_data, result.error};
290288
}
@@ -323,7 +321,7 @@ int File::ungetc_unlocked(int c) {
323321
}
324322

325323
eof = false; // There is atleast one character that can be read now.
326-
err_code = 0; // This operation was a success.
324+
err = false; // This operation was a success.
327325
return c;
328326
}
329327

@@ -333,8 +331,8 @@ ErrorOr<int> File::seek(off_t offset, int whence) {
333331

334332
FileIOResult buf_result = platform_write(this, buf, pos);
335333
if (buf_result.has_error() || buf_result.value < pos) {
336-
err_code = buf_result.has_error() ? buf_result.error : EIO;
337-
return Error(err_code);
334+
err = true;
335+
return Error(buf_result.error);
338336
}
339337
} else if (prev_op == FileOp::READ && whence == SEEK_CUR) {
340338
// More data could have been read out from the platform file than was
@@ -371,8 +369,8 @@ int File::flush_unlocked() {
371369
if (prev_op == FileOp::WRITE && pos > 0) {
372370
FileIOResult buf_result = platform_write(this, buf, pos);
373371
if (buf_result.has_error() || buf_result.value < pos) {
374-
err_code = buf_result.has_error() ? buf_result.error : EIO;
375-
return err_code;
372+
err = true;
373+
return buf_result.error;
376374
}
377375
pos = 0;
378376
}

‎libc/src/__support/File/file.h‎

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ class File {
118118
size_t read_limit;
119119

120120
bool eof;
121-
int err_code;
121+
bool err;
122122

123123
// This is a convenience RAII class to lock and unlock file objects.
124124
class FileLock {
@@ -161,7 +161,7 @@ class File {
161161
/*robust=*/false, /*pshared=*/false),
162162
ungetc_buf(0), buf(buffer), bufsize(buffer_size), bufmode(buffer_mode),
163163
own_buf(owned), mode(modeflags), pos(0), prev_op(FileOp::NONE),
164-
read_limit(0), eof(false), err_code(0) {
164+
read_limit(0), eof(false), err(false) {
165165
adjust_buf();
166166
}
167167

@@ -214,8 +214,8 @@ class File {
214214
if (prev_op == FileOp::WRITE && pos > 0) {
215215
auto buf_result = platform_write(this, buf, pos);
216216
if (buf_result.has_error() || buf_result.value < pos) {
217-
err_code = buf_result.has_error() ? buf_result.error : EIO;
218-
return err_code;
217+
err = true;
218+
return buf_result.error;
219219
}
220220
}
221221
}
@@ -250,21 +250,14 @@ class File {
250250
void lock() { mutex.lock(); }
251251
void unlock() { mutex.unlock(); }
252252

253-
bool error_unlocked() const { return err_code != 0; }
253+
bool error_unlocked() const { return err; }
254254

255255
bool error() {
256256
FileLock l(this);
257257
return error_unlocked();
258258
}
259259

260-
int error_code_unlocked() const { return err_code; }
261-
262-
int error_code() {
263-
FileLock l(this);
264-
return error_code_unlocked();
265-
}
266-
267-
void clearerr_unlocked() { err_code = 0; }
260+
void clearerr_unlocked() { err = false; }
268261

269262
void clearerr() {
270263
FileLock l(this);

‎libc/src/stdio/generic/fprintf.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ LLVM_LIBC_FUNCTION(int, fprintf,
2929
va_end(vlist);
3030
auto ret_val = printf_core::vfprintf_internal(stream, format, args);
3131
if (ret_val.has_error()) {
32-
libc_errno = printf_core::internal_error_to_errno(ret_val.error, stream);
32+
libc_errno = printf_core::internal_error_to_errno(ret_val.error);
3333
return -1;
3434
}
3535
if (ret_val.value > cpp::numeric_limits<int>::max()) {

‎libc/src/stdio/generic/printf.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,7 @@ LLVM_LIBC_FUNCTION(int, printf, (const char *__restrict format, ...)) {
3535
auto ret_val = printf_core::vfprintf_internal(
3636
reinterpret_cast<::FILE *>(PRINTF_STDOUT), format, args);
3737
if (ret_val.has_error()) {
38-
libc_errno = printf_core::internal_error_to_errno(
39-
ret_val.error, reinterpret_cast<::FILE *>(PRINTF_STDOUT));
38+
libc_errno = printf_core::internal_error_to_errno(ret_val.error);
4039
return -1;
4140
;
4241
return -1;

‎libc/src/stdio/generic/vfprintf.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ LLVM_LIBC_FUNCTION(int, vfprintf,
2727
// destruction automatically.
2828
auto ret_val = printf_core::vfprintf_internal(stream, format, args);
2929
if (ret_val.has_error()) {
30-
libc_errno = printf_core::internal_error_to_errno(ret_val.error, stream);
30+
libc_errno = printf_core::internal_error_to_errno(ret_val.error);
3131
return -1;
3232
}
3333
if (ret_val.value > cpp::numeric_limits<int>::max()) {

‎libc/src/stdio/generic/vprintf.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,7 @@ LLVM_LIBC_FUNCTION(int, vprintf,
3232
auto ret_val = printf_core::vfprintf_internal(
3333
reinterpret_cast<::FILE *>(PRINTF_STDOUT), format, args);
3434
if (ret_val.has_error()) {
35-
libc_errno = printf_core::internal_error_to_errno(
36-
ret_val.error, reinterpret_cast<::FILE *>(PRINTF_STDOUT));
35+
libc_errno = printf_core::internal_error_to_errno(ret_val.error);
3736
return -1;
3837
}
3938
if (ret_val.value > cpp::numeric_limits<int>::max()) {

‎libc/src/stdio/printf_core/core_structs.h‎

Lines changed: 15 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -148,29 +148,24 @@ template <typename T> LIBC_INLINE constexpr TypeDesc type_desc_from_type() {
148148
constexpr int WRITE_OK = 0;
149149
// These are the printf return values for when an error has occurred. They are
150150
// all negative, and should be distinct.
151-
constexpr int FILE_WRITE_ERROR = -1;
152-
constexpr int FILE_STATUS_ERROR = -2;
153-
constexpr int NULLPTR_WRITE_ERROR = -3;
154-
constexpr int INT_CONVERSION_ERROR = -4;
155-
constexpr int FIXED_POINT_CONVERSION_ERROR = -5;
156-
constexpr int ALLOCATION_ERROR = -6;
157-
158-
LIBC_INLINE static int internal_error_to_errno(int internal_errno,
159-
FILE *f = nullptr) {
160-
#if !defined(LIBC_COPT_STDIO_USE_SYSTEM_FILE)
161-
LIBC_NAMESPACE::File *file = reinterpret_cast<LIBC_NAMESPACE::File *>(f);
162-
#else
163-
LIBC_NAMESPACE::File *file = nullptr;
164-
(void)f;
165-
#endif
151+
constexpr int FILE_WRITE_ERROR = -1001;
152+
constexpr int FILE_STATUS_ERROR = -1002;
153+
constexpr int NULLPTR_WRITE_ERROR = -1003;
154+
constexpr int INT_CONVERSION_ERROR = -1004;
155+
constexpr int FIXED_POINT_CONVERSION_ERROR = -1005;
156+
constexpr int ALLOCATION_ERROR = -1006;
157+
constexpr int SHORT_WRITE_ERROR = -1007;
158+
159+
LIBC_INLINE static int internal_error_to_errno(int internal_errno) {
160+
if (internal_errno < 1001) {
161+
return internal_errno;
162+
}
166163

167164
switch (-internal_errno) {
168165
case WRITE_OK:
169166
return 0;
170167
case FILE_WRITE_ERROR:
171-
if (file == nullptr)
172-
return EIO;
173-
return file->error_unlocked() ? file->error_code_unlocked() : EIO;
168+
return EIO;
174169
case FILE_STATUS_ERROR:
175170
return EIO;
176171
case NULLPTR_WRITE_ERROR:
@@ -181,6 +176,8 @@ LIBC_INLINE static int internal_error_to_errno(int internal_errno,
181176
return EINVAL;
182177
case ALLOCATION_ERROR:
183178
return ENOMEM;
179+
case SHORT_WRITE_ERROR:
180+
return EIO;
184181
default:
185182
LIBC_ASSERT(
186183
false &&

‎libc/src/stdio/printf_core/vfprintf_internal.h‎

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@ LIBC_INLINE void funlockfile(FILE *f) {
3636
reinterpret_cast<LIBC_NAMESPACE::File *>(f)->unlock();
3737
}
3838

39-
LIBC_INLINE size_t fwrite_unlocked(const void *ptr, size_t size, size_t nmemb,
40-
FILE *f) {
39+
LIBC_INLINE FileIOResult fwrite_unlocked(const void *ptr, size_t size,
40+
size_t nmemb, FILE *f) {
4141
return reinterpret_cast<LIBC_NAMESPACE::File *>(f)->write_unlocked(
4242
ptr, size * nmemb);
4343
}
@@ -48,9 +48,9 @@ LIBC_INLINE void flockfile(::FILE *f) { ::flockfile(f); }
4848

4949
LIBC_INLINE void funlockfile(::FILE *f) { ::funlockfile(f); }
5050

51-
LIBC_INLINE size_t fwrite_unlocked(const void *ptr, size_t size, size_t nmemb,
52-
::FILE *f) {
53-
return ::fwrite_unlocked(ptr, size, nmemb, f);
51+
LIBC_INLINE FileIOResult fwrite_unlocked(const void *ptr, size_t size,
52+
size_t nmemb, ::FILE *f) {
53+
return {::fwrite_unlocked(ptr, size, nmemb, f), 0}; // todo err
5454
}
5555
#endif // LIBC_COPT_STDIO_USE_SYSTEM_FILE
5656
} // namespace internal
@@ -61,10 +61,22 @@ LIBC_INLINE int file_write_hook(cpp::string_view new_str, void *fp) {
6161
::FILE *target_file = reinterpret_cast<::FILE *>(fp);
6262
// Write new_str to the target file. The logic preventing a zero-length write
6363
// is in the writer, so we don't check here.
64-
size_t written = internal::fwrite_unlocked(new_str.data(), sizeof(char),
65-
new_str.size(), target_file);
66-
if (written != new_str.size() || internal::ferror_unlocked(target_file))
64+
auto write_result = internal::fwrite_unlocked(new_str.data(), sizeof(char),
65+
new_str.size(), target_file);
66+
// Propagate actual system error in FileIOResult.
67+
if (write_result.has_error())
68+
return -write_result.error;
69+
70+
// On short write no system error is returned, so return custom error.
71+
if (write_result.value != new_str.size())
72+
return SHORT_WRITE_ERROR;
73+
74+
// Return custom error in case error wasn't set on FileIOResult for some
75+
// reason (like using LIBC_COPT_STDIO_USE_SYSTEM_FILE)
76+
if (internal::ferror_unlocked(target_file)) {
6777
return FILE_WRITE_ERROR;
78+
}
79+
6880
return WRITE_OK;
6981
}
7082

0 commit comments

Comments
 (0)