[libc] Fix EOF handling in fscanf (#211982)
`fscanf` family of functions should return EOF if: - end of input is reached before either the first successful conversion or a matching failure occurs; - a read error occurs. This wasn't handled correctly before - in "system FILE" mode (in overlay build) `EOF` return value from `getc` was passed through to parser, violating `Reader` interface, which triggered infinite loops on "while-not-EOF" `fscanf` loops. In llvm-libc-FILE mode `fscanf` simply returned zero instead of EOF, because the code only checked error indicator on a stream. This PR removes _any_ lookups of eof/error indicators on an input stream - instead we can simply rely on the `getc` (system or internal) function behavior, which returns `EOF` on unsuccessful reads. Extend the test cases for `fscanf` to cover various return values in cases when `EOF` is reached (before or after conversions / matching errors).
This commit is contained in:
Alexey Samsonov
GitHub
parent
326d563cf7
commit
98fb5dae73
4 changed files with 163 additions and 19 deletions
|
|
@ -14,7 +14,6 @@ if(LIBC_TARGET_OS_IS_GPU)
|
|||
list(APPEND file_deps
|
||||
libc.src.stdio.getc
|
||||
libc.src.stdio.ungetc
|
||||
libc.src.stdio.ferror
|
||||
)
|
||||
elseif(LLVM_LIBC_FULL_BUILD)
|
||||
list(APPEND file_deps
|
||||
|
|
|
|||
|
|
@ -17,7 +17,6 @@
|
|||
#include "src/stdio/scanf_core/scanf_main.h"
|
||||
|
||||
#if defined(LIBC_TARGET_ARCH_IS_GPU)
|
||||
#include "src/stdio/ferror.h"
|
||||
#include "src/stdio/getc.h"
|
||||
#include "src/stdio/ungetc.h"
|
||||
#endif
|
||||
|
|
@ -38,8 +37,6 @@ LIBC_INLINE void flockfile(::FILE *) { return; }
|
|||
|
||||
LIBC_INLINE void funlockfile(::FILE *) { return; }
|
||||
|
||||
LIBC_INLINE int ferror_unlocked(::FILE *f) { return LIBC_NAMESPACE::ferror(f); }
|
||||
|
||||
LIBC_INLINE int getc(::FILE *f) { return LIBC_NAMESPACE::getc(f); }
|
||||
|
||||
LIBC_INLINE void ungetc(int c, ::FILE *f) { LIBC_NAMESPACE::ungetc(c, f); }
|
||||
|
|
@ -54,17 +51,13 @@ LIBC_INLINE void funlockfile(FILE *f) {
|
|||
reinterpret_cast<LIBC_NAMESPACE::File *>(f)->unlock();
|
||||
}
|
||||
|
||||
LIBC_INLINE int ferror_unlocked(FILE *f) {
|
||||
return reinterpret_cast<LIBC_NAMESPACE::File *>(f)->error_unlocked();
|
||||
}
|
||||
|
||||
LIBC_INLINE int getc(FILE *f) {
|
||||
unsigned char c;
|
||||
auto result =
|
||||
reinterpret_cast<LIBC_NAMESPACE::File *>(f)->read_unlocked(&c, 1);
|
||||
size_t r = result.value;
|
||||
if (result.has_error() || r != 1)
|
||||
return '\0';
|
||||
return EOF;
|
||||
|
||||
return c;
|
||||
}
|
||||
|
|
@ -81,8 +74,6 @@ LIBC_INLINE void flockfile(::FILE *) { return; }
|
|||
|
||||
LIBC_INLINE void funlockfile(::FILE *) { return; }
|
||||
|
||||
LIBC_INLINE int ferror_unlocked(::FILE *f) { return ::ferror(f); }
|
||||
|
||||
LIBC_INLINE int getc(::FILE *f) { return ::getc(f); }
|
||||
|
||||
LIBC_INLINE void ungetc(int c, ::FILE *f) { ::ungetc(c, f); }
|
||||
|
|
@ -95,16 +86,25 @@ namespace scanf_core {
|
|||
|
||||
class StreamReader : public Reader<StreamReader> {
|
||||
::FILE *stream;
|
||||
bool eof = false;
|
||||
|
||||
public:
|
||||
LIBC_INLINE StreamReader(::FILE *stream) : stream(stream) {}
|
||||
|
||||
LIBC_INLINE char getc() {
|
||||
return static_cast<char>(internal::getc(static_cast<FILE *>(stream)));
|
||||
int c = internal::getc(stream);
|
||||
if (c == EOF) {
|
||||
eof = true;
|
||||
return '\0';
|
||||
}
|
||||
return static_cast<char>(c);
|
||||
}
|
||||
LIBC_INLINE void ungetc(int c) {
|
||||
internal::ungetc(c, static_cast<FILE *>(stream));
|
||||
// Don't ungetc the EOF to prevent clearing EOF indicator on stream.
|
||||
if (!eof)
|
||||
internal::ungetc(c, stream);
|
||||
}
|
||||
LIBC_INLINE bool reached_eof() const { return eof; }
|
||||
};
|
||||
|
||||
LIBC_INLINE int vfscanf_internal(::FILE *__restrict stream,
|
||||
|
|
@ -113,7 +113,7 @@ LIBC_INLINE int vfscanf_internal(::FILE *__restrict stream,
|
|||
internal::flockfile(stream);
|
||||
scanf_core::StreamReader reader(stream);
|
||||
int retval = scanf_core::scanf_main(&reader, format, args);
|
||||
if (retval == 0 && internal::ferror_unlocked(stream))
|
||||
if (retval == 0 && reader.reached_eof())
|
||||
retval = EOF;
|
||||
internal::funlockfile(stream);
|
||||
|
||||
|
|
|
|||
|
|
@ -309,6 +309,7 @@ if(LLVM_LIBC_FULL_BUILD)
|
|||
# In fullbuild mode, fscanf's tests use the internal FILE for other functions.
|
||||
list(APPEND fscanf_test_deps
|
||||
libc.src.stdio.fclose
|
||||
libc.src.stdio.feof
|
||||
libc.src.stdio.ferror
|
||||
libc.src.stdio.fopen
|
||||
libc.src.stdio.fwrite
|
||||
|
|
@ -325,6 +326,7 @@ if(NOT LIBC_TARGET_ARCHITECTURE_IS_AMDGPU)
|
|||
SRCS
|
||||
fscanf_test.cpp
|
||||
DEPENDS
|
||||
libc.hdr.stdio_macros
|
||||
libc.src.stdio.fscanf
|
||||
${fscanf_test_deps}
|
||||
libc.src.__support.CPP.string_view
|
||||
|
|
|
|||
|
|
@ -6,27 +6,29 @@
|
|||
//
|
||||
//===----------------------------------------------------------------------===//
|
||||
|
||||
#include "hdr/stdio_macros.h"
|
||||
#include "src/__support/CPP/string_view.h"
|
||||
#include "src/stdio/fscanf.h"
|
||||
#include "test/UnitTest/Test.h"
|
||||
|
||||
#ifndef LIBC_COPT_STDIO_USE_SYSTEM_FILE
|
||||
#include "src/stdio/fclose.h"
|
||||
#include "src/stdio/feof.h"
|
||||
#include "src/stdio/ferror.h"
|
||||
#include "src/stdio/fopen.h"
|
||||
#include "src/stdio/fwrite.h"
|
||||
#endif // LIBC_COPT_STDIO_USE_SYSTEM_FILE
|
||||
|
||||
#include "src/stdio/fscanf.h"
|
||||
|
||||
#include "test/UnitTest/Test.h"
|
||||
|
||||
namespace scanf_test {
|
||||
#ifndef LIBC_COPT_STDIO_USE_SYSTEM_FILE
|
||||
using LIBC_NAMESPACE::fclose;
|
||||
using LIBC_NAMESPACE::feof;
|
||||
using LIBC_NAMESPACE::ferror;
|
||||
using LIBC_NAMESPACE::fopen;
|
||||
using LIBC_NAMESPACE::fwrite;
|
||||
#else // defined(LIBC_COPT_STDIO_USE_SYSTEM_FILE)
|
||||
using ::fclose;
|
||||
using ::feof;
|
||||
using ::ferror;
|
||||
using ::fopen;
|
||||
using ::fwrite;
|
||||
|
|
@ -59,7 +61,8 @@ TEST(LlvmLibcFScanfTest, WriteToFile) {
|
|||
|
||||
read = LIBC_NAMESPACE::fscanf(file,
|
||||
"Reading from a write-only file should fail.");
|
||||
EXPECT_LT(read, 0);
|
||||
EXPECT_NE(scanf_test::ferror(file), 0);
|
||||
EXPECT_EQ(read, EOF);
|
||||
|
||||
ASSERT_EQ(0, scanf_test::fclose(file));
|
||||
|
||||
|
|
@ -87,3 +90,143 @@ TEST(LlvmLibcFScanfTest, WriteToFile) {
|
|||
ASSERT_EQ(scanf_test::ferror(file), 0);
|
||||
ASSERT_EQ(scanf_test::fclose(file), 0);
|
||||
}
|
||||
|
||||
TEST(LlvmLibcFScanfTest, ProcNetIfInet6Sample) {
|
||||
const char *FILENAME = APPEND_LIBC_TEST("proc_net_if_inet6.txt");
|
||||
auto FILE_PATH = libc_make_test_file_path(FILENAME);
|
||||
::FILE *file = scanf_test::fopen(FILE_PATH, "w");
|
||||
ASSERT_FALSE(file == nullptr);
|
||||
|
||||
// Sample contents of /proc/net/if_inet6 on Linux.
|
||||
constexpr char sample_contents[] =
|
||||
"fe80000000000000adaf264669baa4c7 02 40 20 80 ens4\n"
|
||||
"00000000000000000000000000000001 01 80 10 80 lo\n";
|
||||
constexpr char entry_format[] =
|
||||
"%4s%4s%4s%4s%4s%4s%4s%4s %08x %02x %02x %02x %20s\n";
|
||||
|
||||
ASSERT_EQ(sizeof(sample_contents) - 1,
|
||||
scanf_test::fwrite(sample_contents, 1, sizeof(sample_contents) - 1,
|
||||
file));
|
||||
ASSERT_EQ(0, scanf_test::fclose(file));
|
||||
|
||||
file = scanf_test::fopen(FILE_PATH, "r");
|
||||
ASSERT_FALSE(file == nullptr);
|
||||
|
||||
char addr6p[8][5];
|
||||
int if_idx, prefix, scope, dad_status;
|
||||
char devname[21];
|
||||
|
||||
// Validate first entry.
|
||||
int ret = LIBC_NAMESPACE::fscanf(file, entry_format, addr6p[0], addr6p[1],
|
||||
addr6p[2], addr6p[3], addr6p[4], addr6p[5],
|
||||
addr6p[6], addr6p[7], &if_idx, &prefix,
|
||||
&scope, &dad_status, devname);
|
||||
ASSERT_EQ(ret, 13);
|
||||
EXPECT_STREQ(addr6p[0], "fe80");
|
||||
EXPECT_STREQ(addr6p[1], "0000");
|
||||
EXPECT_STREQ(addr6p[2], "0000");
|
||||
EXPECT_STREQ(addr6p[3], "0000");
|
||||
EXPECT_STREQ(addr6p[4], "adaf");
|
||||
EXPECT_STREQ(addr6p[5], "2646");
|
||||
EXPECT_STREQ(addr6p[6], "69ba");
|
||||
EXPECT_STREQ(addr6p[7], "a4c7");
|
||||
EXPECT_EQ(if_idx, 2);
|
||||
EXPECT_EQ(prefix, 64);
|
||||
EXPECT_EQ(scope, 32);
|
||||
EXPECT_EQ(dad_status, 128);
|
||||
EXPECT_STREQ(devname, "ens4");
|
||||
|
||||
// Validate second entry.
|
||||
ret = LIBC_NAMESPACE::fscanf(file, entry_format, addr6p[0], addr6p[1],
|
||||
addr6p[2], addr6p[3], addr6p[4], addr6p[5],
|
||||
addr6p[6], addr6p[7], &if_idx, &prefix, &scope,
|
||||
&dad_status, devname);
|
||||
ASSERT_EQ(ret, 13);
|
||||
for (int i = 0; i < 7; i++)
|
||||
EXPECT_STREQ(addr6p[i], "0000");
|
||||
EXPECT_STREQ(addr6p[7], "0001");
|
||||
EXPECT_EQ(if_idx, 1);
|
||||
EXPECT_EQ(prefix, 128);
|
||||
EXPECT_EQ(scope, 16);
|
||||
EXPECT_EQ(dad_status, 128);
|
||||
EXPECT_STREQ(devname, "lo");
|
||||
|
||||
// No more entries, return EOF.
|
||||
ret = LIBC_NAMESPACE::fscanf(file, entry_format, addr6p[0], addr6p[1],
|
||||
addr6p[2], addr6p[3], addr6p[4], addr6p[5],
|
||||
addr6p[6], addr6p[7], &if_idx, &prefix, &scope,
|
||||
&dad_status, devname);
|
||||
EXPECT_EQ(ret, EOF);
|
||||
EXPECT_NE(scanf_test::feof(file), 0);
|
||||
EXPECT_EQ(scanf_test::ferror(file), 0);
|
||||
|
||||
ASSERT_EQ(scanf_test::fclose(file), 0);
|
||||
}
|
||||
|
||||
TEST(LlvmLibcFScanfTest, EofPartialMatch) {
|
||||
const char *FILENAME = APPEND_LIBC_TEST("eof_partial_match.txt");
|
||||
auto FILE_PATH = libc_make_test_file_path(FILENAME);
|
||||
::FILE *file = scanf_test::fopen(FILE_PATH, "w");
|
||||
ASSERT_FALSE(file == nullptr);
|
||||
|
||||
constexpr char contents[] = "1 2 3";
|
||||
ASSERT_EQ(sizeof(contents) - 1,
|
||||
scanf_test::fwrite(contents, 1, sizeof(contents) - 1, file));
|
||||
ASSERT_EQ(0, scanf_test::fclose(file));
|
||||
|
||||
file = scanf_test::fopen(FILE_PATH, "r");
|
||||
ASSERT_FALSE(file == nullptr);
|
||||
|
||||
int vals[4] = {0};
|
||||
int ret = LIBC_NAMESPACE::fscanf(file, "%d %d %d %d", &vals[0], &vals[1],
|
||||
&vals[2], &vals[3]);
|
||||
// Returns 3 for number of matches despite EOF.
|
||||
EXPECT_EQ(ret, 3);
|
||||
EXPECT_EQ(vals[0], 1);
|
||||
EXPECT_EQ(vals[1], 2);
|
||||
EXPECT_EQ(vals[2], 3);
|
||||
|
||||
EXPECT_NE(scanf_test::feof(file), 0);
|
||||
EXPECT_EQ(scanf_test::ferror(file), 0);
|
||||
|
||||
ASSERT_EQ(scanf_test::fclose(file), 0);
|
||||
}
|
||||
|
||||
TEST(LlvmLibcFScanfTest, MatchingErrors) {
|
||||
const char *FILENAME = APPEND_LIBC_TEST("matching_error_partial_match.txt");
|
||||
auto FILE_PATH = libc_make_test_file_path(FILENAME);
|
||||
::FILE *file = scanf_test::fopen(FILE_PATH, "w");
|
||||
ASSERT_FALSE(file == nullptr);
|
||||
|
||||
constexpr char contents[] = "one is 1 two is 2";
|
||||
ASSERT_EQ(sizeof(contents) - 1,
|
||||
scanf_test::fwrite(contents, 1, sizeof(contents) - 1, file));
|
||||
ASSERT_EQ(0, scanf_test::fclose(file));
|
||||
|
||||
file = scanf_test::fopen(FILE_PATH, "r");
|
||||
ASSERT_FALSE(file == nullptr);
|
||||
|
||||
int vals[2] = {0};
|
||||
// Immediate matching error.
|
||||
int ret =
|
||||
LIBC_NAMESPACE::fscanf(file, "zzz is %d two is %d", &vals[0], &vals[1]);
|
||||
EXPECT_EQ(ret, 0);
|
||||
EXPECT_EQ(scanf_test::feof(file), 0);
|
||||
EXPECT_EQ(scanf_test::ferror(file), 0);
|
||||
|
||||
// Only the first item is matched.
|
||||
ret = LIBC_NAMESPACE::fscanf(file, "one is %d zzz is %d", &vals[0], &vals[1]);
|
||||
EXPECT_EQ(ret, 1);
|
||||
EXPECT_EQ(vals[0], 1);
|
||||
EXPECT_EQ(scanf_test::feof(file), 0);
|
||||
EXPECT_EQ(scanf_test::ferror(file), 0);
|
||||
|
||||
// Second item is matched before EOF.
|
||||
ret = LIBC_NAMESPACE::fscanf(file, "two is %d zzz is %d", &vals[0], &vals[1]);
|
||||
EXPECT_EQ(ret, 1);
|
||||
EXPECT_EQ(vals[0], 2);
|
||||
EXPECT_NE(scanf_test::feof(file), 0);
|
||||
EXPECT_EQ(scanf_test::ferror(file), 0);
|
||||
|
||||
ASSERT_EQ(scanf_test::fclose(file), 0);
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue