[clang] [libc] Refactor scanf reader to match printf (PR #66023)

via cfe-commits cfe-commits at lists.llvm.org
Wed Sep 13 10:49:22 PDT 2023


https://github.com/michaelrj-google updated https://github.com/llvm/llvm-project/pull/66023:

>From c6330fb734a687d975dc2c6e674d98b03d8f2d1b Mon Sep 17 00:00:00 2001
From: Michael Jones <michaelrj at google.com>
Date: Mon, 11 Sep 2023 15:22:15 -0700
Subject: [PATCH] [libc] Refactor scanf reader to match printf

In a previous patch, the printf writer was rewritten to use a single
writer class with a buffer and a callback hook. This patch refactors
scanf's reader to match conceptually.
---
 libc/config/linux/aarch64/entrypoints.txt     |  4 ++
 libc/config/linux/riscv64/entrypoints.txt     |  7 ++-
 libc/config/linux/x86_64/entrypoints.txt      |  6 +--
 libc/src/stdio/CMakeLists.txt                 | 26 ++++++++--
 libc/src/stdio/scanf.cpp                      |  8 ++-
 libc/src/stdio/scanf_core/CMakeLists.txt      | 40 ++++-----------
 libc/src/stdio/scanf_core/file_reader.cpp     | 27 ----------
 libc/src/stdio/scanf_core/file_reader.h       | 39 ---------------
 libc/src/stdio/scanf_core/reader.cpp          | 32 +++---------
 libc/src/stdio/scanf_core/reader.h            | 49 +++++++++++++------
 libc/src/stdio/scanf_core/scanf_main.cpp      |  5 --
 libc/src/stdio/scanf_core/string_reader.cpp   | 24 ---------
 libc/src/stdio/scanf_core/string_reader.h     | 33 -------------
 .../src/stdio/scanf_core/vfscanf_internal.cpp | 29 -----------
 libc/src/stdio/scanf_core/vfscanf_internal.h  | 46 ++++++++++++++++-
 libc/src/stdio/sscanf.cpp                     |  7 +--
 libc/test/src/stdio/CMakeLists.txt            | 21 ++++++--
 libc/test/src/stdio/fscanf_test.cpp           | 38 ++++++++++----
 libc/test/src/stdio/scanf_core/CMakeLists.txt | 17 +++----
 ...string_reader_test.cpp => reader_test.cpp} | 15 +++---
 20 files changed, 198 insertions(+), 275 deletions(-)
 delete mode 100644 libc/src/stdio/scanf_core/file_reader.cpp
 delete mode 100644 libc/src/stdio/scanf_core/file_reader.h
 delete mode 100644 libc/src/stdio/scanf_core/string_reader.cpp
 delete mode 100644 libc/src/stdio/scanf_core/string_reader.h
 delete mode 100644 libc/src/stdio/scanf_core/vfscanf_internal.cpp
 rename libc/test/src/stdio/scanf_core/{string_reader_test.cpp => reader_test.cpp} (76%)

diff --git a/libc/config/linux/aarch64/entrypoints.txt b/libc/config/linux/aarch64/entrypoints.txt
index e0bf9800ec881c2..fc865dc0cbb6a57 100644
--- a/libc/config/linux/aarch64/entrypoints.txt
+++ b/libc/config/linux/aarch64/entrypoints.txt
@@ -126,6 +126,10 @@ set(TARGET_LIBC_ENTRYPOINTS
     libc.src.stdio.snprintf
     libc.src.stdio.vsprintf
     libc.src.stdio.vsnprintf
+    #TODO: Check if scanf can be enabled on aarch64
+    #libc.src.stdio.sscanf
+    #libc.src.stdio.scanf
+    #libc.src.stdio.fscanf
 
     # sys/mman.h entrypoints
     libc.src.sys.mman.madvise
diff --git a/libc/config/linux/riscv64/entrypoints.txt b/libc/config/linux/riscv64/entrypoints.txt
index 2b2f2629f78ce67..9574595ed459383 100644
--- a/libc/config/linux/riscv64/entrypoints.txt
+++ b/libc/config/linux/riscv64/entrypoints.txt
@@ -132,6 +132,9 @@ set(TARGET_LIBC_ENTRYPOINTS
     libc.src.stdio.vsnprintf
     libc.src.stdio.vfprintf
     libc.src.stdio.vprintf
+    libc.src.stdio.sscanf
+    libc.src.stdio.scanf
+    libc.src.stdio.fscanf
 
     # sys/mman.h entrypoints
     libc.src.sys.mman.madvise
@@ -439,10 +442,6 @@ if(LLVM_LIBC_FULL_BUILD)
     libc.src.stdio.getc_unlocked
     libc.src.stdio.getchar
     libc.src.stdio.getchar_unlocked
-    libc.src.stdio.printf
-    libc.src.stdio.sscanf
-    libc.src.stdio.scanf
-    libc.src.stdio.fscanf
     libc.src.stdio.putc
     libc.src.stdio.putchar
     libc.src.stdio.puts
diff --git a/libc/config/linux/x86_64/entrypoints.txt b/libc/config/linux/x86_64/entrypoints.txt
index dcb8c6231432d35..1c3fcb0c1e7c9e9 100644
--- a/libc/config/linux/x86_64/entrypoints.txt
+++ b/libc/config/linux/x86_64/entrypoints.txt
@@ -132,6 +132,9 @@ set(TARGET_LIBC_ENTRYPOINTS
     libc.src.stdio.vsnprintf
     libc.src.stdio.vfprintf
     libc.src.stdio.vprintf
+    libc.src.stdio.sscanf
+    libc.src.stdio.scanf
+    libc.src.stdio.fscanf
 
     # sys/mman.h entrypoints
     libc.src.sys.mman.madvise
@@ -445,9 +448,6 @@ if(LLVM_LIBC_FULL_BUILD)
     libc.src.stdio.getc_unlocked
     libc.src.stdio.getchar
     libc.src.stdio.getchar_unlocked
-    libc.src.stdio.sscanf
-    libc.src.stdio.scanf
-    libc.src.stdio.fscanf
     libc.src.stdio.putc
     libc.src.stdio.putchar
     libc.src.stdio.puts
diff --git a/libc/src/stdio/CMakeLists.txt b/libc/src/stdio/CMakeLists.txt
index 740ec106da2e4d7..6c393855b28cada 100644
--- a/libc/src/stdio/CMakeLists.txt
+++ b/libc/src/stdio/CMakeLists.txt
@@ -292,6 +292,21 @@ add_entrypoint_object(
     libc.src.__support.File.platform_file
 )
 
+list(APPEND scanf_deps
+      libc.src.__support.arg_list
+      libc.src.stdio.scanf_core.vfscanf_internal
+)
+
+if(LLVM_LIBC_FULL_BUILD)
+  list(APPEND scanf_deps
+      libc.src.__support.File.file
+      libc.src.__support.File.platform_file
+      libc.src.__support.File.platform_stdin
+  )
+else()
+  list(APPEND scanf_copts "-DLIBC_COPT_SCANF_USE_SYSTEM_FILE")
+endif()
+
 add_entrypoint_object(
   sscanf
   SRCS
@@ -300,7 +315,6 @@ add_entrypoint_object(
     sscanf.h
   DEPENDS
     libc.src.__support.arg_list
-    libc.src.stdio.scanf_core.string_reader
     libc.src.stdio.scanf_core.reader
     libc.src.stdio.scanf_core.scanf_main
 )
@@ -312,8 +326,9 @@ add_entrypoint_object(
   HDRS
     fscanf.h
   DEPENDS
-    libc.src.__support.arg_list
-    libc.src.stdio.scanf_core.vfscanf_internal
+    ${scanf_deps}
+  COMPILE_OPTIONS
+    ${scanf_copts}
 )
 
 add_entrypoint_object(
@@ -323,8 +338,9 @@ add_entrypoint_object(
   HDRS
     scanf.h
   DEPENDS
-    libc.src.__support.arg_list
-    libc.src.stdio.scanf_core.vfscanf_internal
+    ${scanf_deps}
+  COMPILE_OPTIONS
+    ${scanf_copts}
 )
 
 add_entrypoint_object(
diff --git a/libc/src/stdio/scanf.cpp b/libc/src/stdio/scanf.cpp
index 60e77895edcc3c6..d98cc3d58e23251 100644
--- a/libc/src/stdio/scanf.cpp
+++ b/libc/src/stdio/scanf.cpp
@@ -15,6 +15,12 @@
 #include <stdarg.h>
 #include <stdio.h>
 
+#ifndef LIBC_COPT_SCANF_USE_SYSTEM_FILE
+#define SCANF_STDIN __llvm_libc::stdin
+#else // LIBC_COPT_SCANF_USE_SYSTEM_FILE
+#define SCANF_STDIN ::stdin
+#endif // LIBC_COPT_SCANF_USE_SYSTEM_FILE
+
 namespace __llvm_libc {
 
 LLVM_LIBC_FUNCTION(int, scanf, (const char *__restrict format, ...)) {
@@ -25,7 +31,7 @@ LLVM_LIBC_FUNCTION(int, scanf, (const char *__restrict format, ...)) {
                                  // destruction automatically.
   va_end(vlist);
   int ret_val = scanf_core::vfscanf_internal(
-      reinterpret_cast<::FILE *>(__llvm_libc::stdin), format, args);
+      reinterpret_cast<::FILE *>(SCANF_STDIN), format, args);
   // This is done to avoid including stdio.h in the internals. On most systems
   // EOF is -1, so this will be transformed into just "return ret_val".
   return (ret_val == -1) ? EOF : ret_val;
diff --git a/libc/src/stdio/scanf_core/CMakeLists.txt b/libc/src/stdio/scanf_core/CMakeLists.txt
index 9f6cb9c386eb226..dd775b437a9d840 100644
--- a/libc/src/stdio/scanf_core/CMakeLists.txt
+++ b/libc/src/stdio/scanf_core/CMakeLists.txt
@@ -24,12 +24,6 @@ add_object_library(
     libc.src.__support.CPP.string_view
 )
 
-if(NOT (TARGET libc.src.__support.File.file))
-  # Not all platforms have a file implementation. If file is unvailable,
-  # then we must skip all the parts that need file.
-  return()
-endif()
-
 add_object_library(
   scanf_main
   SRCS
@@ -44,24 +38,6 @@ add_object_library(
     libc.src.__support.arg_list
 )
 
-add_object_library(
-  string_reader
-  SRCS
-    string_reader.cpp
-  HDRS
-    string_reader.h
-)
-
-add_object_library(
-  file_reader
-  SRCS
-    file_reader.cpp
-  HDRS
-    file_reader.h
-  DEPENDS
-    libc.src.__support.File.file
-)
-
 add_object_library(
   reader
   SRCS
@@ -69,8 +45,7 @@ add_object_library(
   HDRS
     reader.h
   DEPENDS
-    .string_reader
-    .file_reader
+    libc.src.__support.macros.attributes
 )
 
 add_object_library(
@@ -101,15 +76,20 @@ add_object_library(
     libc.src.__support.str_to_float
 )
 
-add_object_library(
+if(NOT (TARGET libc.src.__support.File.file) AND LLVM_LIBC_FULL_BUILD)
+  # Not all platforms have a file implementation. If file is unvailable, and a 
+  # full build is requested, then we must skip all file based printf sections.
+  return()
+endif()
+
+add_header_library(
   vfscanf_internal
-  SRCS
-    vfscanf_internal.cpp
   HDRS
     vfscanf_internal.h
   DEPENDS
     .reader
-    .file_reader
     .scanf_main
+    libc.include.stdio
+    libc.src.__support.File.file
     libc.src.__support.arg_list
 )
diff --git a/libc/src/stdio/scanf_core/file_reader.cpp b/libc/src/stdio/scanf_core/file_reader.cpp
deleted file mode 100644
index 51e22299e3dd2c5..000000000000000
--- a/libc/src/stdio/scanf_core/file_reader.cpp
+++ /dev/null
@@ -1,27 +0,0 @@
-//===-- FILE Reader implementation for scanf --------------------*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#include "src/stdio/scanf_core/file_reader.h"
-#include "src/__support/File/file.h"
-#include <stddef.h>
-
-namespace __llvm_libc {
-namespace scanf_core {
-
-char FileReader::get_char() {
-  char tiny_buff = 0;
-  auto result = file->read_unlocked(&tiny_buff, 1);
-  if (result.value != 1 || result.has_error())
-    return 0;
-  return tiny_buff;
-}
-
-void FileReader::unget_char(char c) { file->ungetc_unlocked(c); }
-
-} // namespace scanf_core
-} // namespace __llvm_libc
diff --git a/libc/src/stdio/scanf_core/file_reader.h b/libc/src/stdio/scanf_core/file_reader.h
deleted file mode 100644
index a70e00bc2413963..000000000000000
--- a/libc/src/stdio/scanf_core/file_reader.h
+++ /dev/null
@@ -1,39 +0,0 @@
-//===-- FILE Reader definition for scanf ------------------------*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLVM_LIBC_SRC_STDIO_SCANF_CORE_FILE_READER_H
-#define LLVM_LIBC_SRC_STDIO_SCANF_CORE_FILE_READER_H
-
-#include "src/__support/File/file.h"
-
-#include <stddef.h>
-#include <stdio.h>
-
-namespace __llvm_libc {
-namespace scanf_core {
-
-class FileReader {
-  __llvm_libc::File *file;
-
-public:
-  FileReader(::FILE *init_file) {
-    file = reinterpret_cast<__llvm_libc::File *>(init_file);
-    file->lock();
-  }
-
-  ~FileReader() { file->unlock(); }
-
-  char get_char();
-  void unget_char(char c);
-  bool has_error() { return file->error_unlocked(); }
-};
-
-} // namespace scanf_core
-} // namespace __llvm_libc
-
-#endif // LLVM_LIBC_SRC_STDIO_SCANF_CORE_FILE_READER_H
diff --git a/libc/src/stdio/scanf_core/reader.cpp b/libc/src/stdio/scanf_core/reader.cpp
index 4834f4d6a6e7955..45241d8d86b01fe 100644
--- a/libc/src/stdio/scanf_core/reader.cpp
+++ b/libc/src/stdio/scanf_core/reader.cpp
@@ -12,33 +12,17 @@
 namespace __llvm_libc {
 namespace scanf_core {
 
-char Reader::getc() {
-  ++cur_chars_read;
-  if (reader_type == ReaderType::String) {
-    return string_reader->get_char();
-  } else {
-    return file_reader->get_char();
-  }
-}
-
 void Reader::ungetc(char c) {
   --cur_chars_read;
-  if (reader_type == ReaderType::String) {
-    // The string reader ignores the char c passed to unget since it doesn't
-    // need to place anything back into a buffer, and modifying the source
-    // string would be dangerous.
-    return string_reader->unget_char();
-  } else {
-    return file_reader->unget_char(c);
+  if (rb != nullptr && rb->buff_cur > 0) {
+    // While technically c should be written back to the buffer, in scanf we
+    // always write the character that was already there. Additionally, the
+    // buffer is most likely to contain a string that isn't part of a file,
+    // which may not be writable.
+    --(rb->buff_cur);
+    return;
   }
+  stream_ungetc(static_cast<int>(c), input_stream);
 }
-
-bool Reader::has_error() {
-  if (reader_type == ReaderType::File) {
-    return file_reader->has_error();
-  }
-  return false;
-}
-
 } // namespace scanf_core
 } // namespace __llvm_libc
diff --git a/libc/src/stdio/scanf_core/reader.h b/libc/src/stdio/scanf_core/reader.h
index 000ab02ad28f21e..7e9cfc5c8ca2fb1 100644
--- a/libc/src/stdio/scanf_core/reader.h
+++ b/libc/src/stdio/scanf_core/reader.h
@@ -9,44 +9,61 @@
 #ifndef LLVM_LIBC_SRC_STDIO_SCANF_CORE_READER_H
 #define LLVM_LIBC_SRC_STDIO_SCANF_CORE_READER_H
 
-#include "src/stdio/scanf_core/file_reader.h"
-#include "src/stdio/scanf_core/string_reader.h"
+#include "src/__support/macros/attributes.h" // For LIBC_INLINE
 #include <stddef.h>
 
 namespace __llvm_libc {
 namespace scanf_core {
 
-enum class ReaderType { String, File };
+using StreamGetc = int (*)(void *);
+using StreamUngetc = void (*)(int, void *);
 
-class Reader final {
-  union {
-    StringReader *string_reader;
-    FileReader *file_reader;
-  };
+// This is intended to be either a raw string or a buffer syncronized with the
+// file's internal buffer.
+struct ReadBuffer {
+  char *buffer;
+  size_t buff_len;
+  size_t buff_cur = 0;
+};
+
+class Reader {
+  ReadBuffer *rb;
 
-  const ReaderType reader_type;
+  void *input_stream = nullptr;
+
+  StreamGetc stream_getc = nullptr;
+  StreamUngetc stream_ungetc = nullptr;
 
   size_t cur_chars_read = 0;
 
 public:
-  Reader(StringReader *init_string_reader)
-      : string_reader(init_string_reader), reader_type(ReaderType::String) {}
+  // TODO: Set buff_len with a proper constant
+  Reader(ReadBuffer *string_buffer) : rb(string_buffer) {}
 
-  Reader(FileReader *init_file_reader)
-      : file_reader(init_file_reader), reader_type(ReaderType::File) {}
+  Reader(void *stream, StreamGetc stream_getc_in, StreamUngetc stream_ungetc_in,
+         ReadBuffer *stream_buffer = nullptr)
+      : rb(stream_buffer), input_stream(stream), stream_getc(stream_getc_in),
+        stream_ungetc(stream_ungetc_in) {}
 
   // This returns the next character from the input and advances it by one
   // character. When it hits the end of the string or file it returns '\0' to
   // signal to stop parsing.
-  char getc();
+  LIBC_INLINE char getc() {
+    ++cur_chars_read;
+    if (rb != nullptr) {
+      char output = rb->buffer[rb->buff_cur];
+      ++(rb->buff_cur);
+      return output;
+    }
+    // This should reset the buffer if applicable.
+    return static_cast<char>(stream_getc(input_stream));
+  }
 
   // This moves the input back by one character, placing c into the buffer if
   // this is a file reader, else c is ignored.
   void ungetc(char c);
 
   size_t chars_read() { return cur_chars_read; }
-
-  bool has_error();
 };
 
 } // namespace scanf_core
diff --git a/libc/src/stdio/scanf_core/scanf_main.cpp b/libc/src/stdio/scanf_core/scanf_main.cpp
index 5a79d2e624ab0aa..270024f31d2b68c 100644
--- a/libc/src/stdio/scanf_core/scanf_main.cpp
+++ b/libc/src/stdio/scanf_core/scanf_main.cpp
@@ -38,11 +38,6 @@ int scanf_main(Reader *reader, const char *__restrict str,
     }
   }
 
-  if (conversions == 0 && reader->has_error()) {
-    // This is intended to be converted to EOF in the client call to avoid
-    // including stdio.h in this internal file.
-    return -1;
-  }
   return conversions;
 }
 
diff --git a/libc/src/stdio/scanf_core/string_reader.cpp b/libc/src/stdio/scanf_core/string_reader.cpp
deleted file mode 100644
index 1d728d2b9eb35e6..000000000000000
--- a/libc/src/stdio/scanf_core/string_reader.cpp
+++ /dev/null
@@ -1,24 +0,0 @@
-//===-- String Reader implementation for scanf ------------------*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#include "src/stdio/scanf_core/string_reader.h"
-#include <stddef.h>
-
-namespace __llvm_libc {
-namespace scanf_core {
-
-char StringReader::get_char() {
-  char cur_char = string[cur_index];
-  ++cur_index;
-  return cur_char;
-}
-
-void StringReader::unget_char() { --cur_index; }
-
-} // namespace scanf_core
-} // namespace __llvm_libc
diff --git a/libc/src/stdio/scanf_core/string_reader.h b/libc/src/stdio/scanf_core/string_reader.h
deleted file mode 100644
index 35550b16c32140f..000000000000000
--- a/libc/src/stdio/scanf_core/string_reader.h
+++ /dev/null
@@ -1,33 +0,0 @@
-//===-- String Reader definition for scanf ----------------------*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLVM_LIBC_SRC_STDIO_SCANF_CORE_STRING_READER_H
-#define LLVM_LIBC_SRC_STDIO_SCANF_CORE_STRING_READER_H
-
-#include <stddef.h>
-
-namespace __llvm_libc {
-namespace scanf_core {
-
-class StringReader {
-  const char *string;
-  size_t cur_index = 0;
-
-public:
-  StringReader(const char *init_string) { string = init_string; }
-
-  ~StringReader() {}
-
-  char get_char();
-  void unget_char();
-};
-
-} // namespace scanf_core
-} // namespace __llvm_libc
-
-#endif // LLVM_LIBC_SRC_STDIO_SCANF_CORE_STRING_READER_H
diff --git a/libc/src/stdio/scanf_core/vfscanf_internal.cpp b/libc/src/stdio/scanf_core/vfscanf_internal.cpp
deleted file mode 100644
index af2f6fa01ad475d..000000000000000
--- a/libc/src/stdio/scanf_core/vfscanf_internal.cpp
+++ /dev/null
@@ -1,29 +0,0 @@
-//===-- Internal implementation of vfscanf ---------------------*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#include "src/stdio/scanf_core/vfscanf_internal.h"
-
-#include "src/__support/arg_list.h"
-#include "src/stdio/scanf_core/file_reader.h"
-#include "src/stdio/scanf_core/reader.h"
-#include "src/stdio/scanf_core/scanf_main.h"
-
-#include <stdio.h>
-
-namespace __llvm_libc {
-namespace scanf_core {
-
-int vfscanf_internal(::FILE *__restrict stream, const char *__restrict format,
-                     internal::ArgList &args) {
-  FileReader file_reader(stream);
-  scanf_core::Reader reader(&file_reader);
-  return scanf_core::scanf_main(&reader, format, args);
-}
-
-} // namespace scanf_core
-} // namespace __llvm_libc
diff --git a/libc/src/stdio/scanf_core/vfscanf_internal.h b/libc/src/stdio/scanf_core/vfscanf_internal.h
index 456d1ff92056bed..693230560bc4f30 100644
--- a/libc/src/stdio/scanf_core/vfscanf_internal.h
+++ b/libc/src/stdio/scanf_core/vfscanf_internal.h
@@ -9,15 +9,57 @@
 #ifndef LLVM_LIBC_SRC_STDIO_SCANF_CORE_VFSCANF_INTERNAL_H
 #define LLVM_LIBC_SRC_STDIO_SCANF_CORE_VFSCANF_INTERNAL_H
 
+#include "src/__support/File/file.h"
 #include "src/__support/arg_list.h"
+#include "src/stdio/scanf_core/reader.h"
+#include "src/stdio/scanf_core/scanf_main.h"
 
 #include <stdio.h>
 
 namespace __llvm_libc {
+
+namespace internal {
+#ifndef LIBC_COPT_SCANF_USE_SYSTEM_FILE
+LIBC_INLINE int getc(void *f) {
+  unsigned char c;
+  auto result = reinterpret_cast<__llvm_libc::File *>(f)->read_unlocked(&c, 1);
+  size_t r = result.value;
+  if (result.has_error() || r != 1) {
+    return '\0';
+  }
+  return c;
+}
+
+LIBC_INLINE void ungetc(int c, void *f) {
+  reinterpret_cast<__llvm_libc::File *>(f)->ungetc(c);
+}
+
+LIBC_INLINE int ferror_unlocked(FILE *f) {
+  return reinterpret_cast<__llvm_libc::File *>(f)->error_unlocked();
+}
+#else  // defined(LIBC_COPT_PRINTF_USE_SYSTEM_FILE)
+LIBC_INLINE int getc(void *f) { return ::getc(reinterpret_cast<::FILE *>(f)); }
+
+LIBC_INLINE void ungetc(int c, void *f) {
+  ::ungetc(c, reinterpret_cast<::FILE *>(f));
+}
+
+LIBC_INLINE int ferror_unlocked(::FILE *f) { return ::ferror_unlocked(f); }
+#endif // LIBC_COPT_SCANF_USE_SYSTEM_FILE
+} // namespace internal
+
 namespace scanf_core {
 
-int vfscanf_internal(::FILE *__restrict stream, const char *__restrict format,
-                     internal::ArgList &args);
+LIBC_INLINE int vfscanf_internal(::FILE *__restrict stream,
+                                 const char *__restrict format,
+                                 internal::ArgList &args) {
+  scanf_core::Reader reader(stream, &internal::getc, internal::ungetc);
+  int retval = scanf_core::scanf_main(&reader, format, args);
+  if (retval == 0 && internal::ferror_unlocked(stream)) {
+    return EOF;
+  }
+  return retval;
+}
 } // namespace scanf_core
 } // namespace __llvm_libc
 
diff --git a/libc/src/stdio/sscanf.cpp b/libc/src/stdio/sscanf.cpp
index 205b58fb3390e95..1f3e5ba888cd8fa 100644
--- a/libc/src/stdio/sscanf.cpp
+++ b/libc/src/stdio/sscanf.cpp
@@ -8,10 +8,10 @@
 
 #include "src/stdio/sscanf.h"
 
+#include "src/__support/CPP/limits.h"
 #include "src/__support/arg_list.h"
 #include "src/stdio/scanf_core/reader.h"
 #include "src/stdio/scanf_core/scanf_main.h"
-#include "src/stdio/scanf_core/string_reader.h"
 
 #include <stdarg.h>
 #include <stdio.h>
@@ -27,8 +27,9 @@ LLVM_LIBC_FUNCTION(int, sscanf,
                                  // and pointer semantics, as well as handling
                                  // destruction automatically.
   va_end(vlist);
-  scanf_core::StringReader string_reader(buffer);
-  scanf_core::Reader reader(&string_reader);
+  scanf_core::ReadBuffer rb{const_cast<char *>(buffer),
+                            cpp::numeric_limits<size_t>::max()};
+  scanf_core::Reader reader(&rb);
   int ret_val = scanf_core::scanf_main(&reader, format, args);
   // This is done to avoid including stdio.h in the internals. On most systems
   // EOF is -1, so this will be transformed into just "return ret_val".
diff --git a/libc/test/src/stdio/CMakeLists.txt b/libc/test/src/stdio/CMakeLists.txt
index 6090dc1f46c87ef..c531bbc3d25398a 100644
--- a/libc/test/src/stdio/CMakeLists.txt
+++ b/libc/test/src/stdio/CMakeLists.txt
@@ -220,6 +220,20 @@ add_libc_test(
     libc.src.stdio.vprintf
 )
 
+
+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.ferror
+       libc.src.stdio.fopen
+       libc.src.stdio.fwrite
+  )
+else()
+# Else in overlay mode they use the system's FILE.
+ set(fscanf_test_copts "-DLIBC_COPT_SCANF_USE_SYSTEM_FILE")
+endif()
+
 add_libc_unittest(
   fscanf_test
   SUITE
@@ -228,11 +242,10 @@ add_libc_unittest(
     fscanf_test.cpp
   DEPENDS
     libc.src.stdio.fscanf
-    libc.src.stdio.fclose
-    libc.src.stdio.ferror
-    libc.src.stdio.fopen
-    libc.src.stdio.fwrite
+    ${fscanf_test_deps}
     libc.src.__support.CPP.string_view
+  COMPILE_OPTIONS
+    ${fscanf_test_copts}
 )
 
 add_libc_unittest(
diff --git a/libc/test/src/stdio/fscanf_test.cpp b/libc/test/src/stdio/fscanf_test.cpp
index 71402f0e7c75b4e..35dd32dd8bf3fe0 100644
--- a/libc/test/src/stdio/fscanf_test.cpp
+++ b/libc/test/src/stdio/fscanf_test.cpp
@@ -7,10 +7,13 @@
 //===----------------------------------------------------------------------===//
 
 #include "src/__support/CPP/string_view.h"
+
+#ifndef LIBC_COPT_SCANF_USE_SYSTEM_FILE
 #include "src/stdio/fclose.h"
 #include "src/stdio/ferror.h"
 #include "src/stdio/fopen.h"
 #include "src/stdio/fwrite.h"
+#endif // LIBC_COPT_SCANF_USE_SYSTEM_FILE
 
 #include "src/stdio/fscanf.h"
 
@@ -18,9 +21,24 @@
 
 #include <stdio.h>
 
+namespace scanf_test {
+#ifndef LIBC_COPT_SCANF_USE_SYSTEM_FILE
+using __llvm_libc::fclose;
+using __llvm_libc::ferror;
+using __llvm_libc::fopen;
+using __llvm_libc::fwrite;
+#else  // defined(LIBC_COPT_SCANF_USE_SYSTEM_FILE)
+using ::fclose;
+using ::ferror;
+using ::fopen;
+using ::fwrite;
+#endif // LIBC_COPT_SCANF_USE_SYSTEM_FILE
+} // namespace scanf_test
+
 TEST(LlvmLibcFScanfTest, WriteToFile) {
-  constexpr char FILENAME[] = "testdata/fscanf_output.test";
-  ::FILE *file = __llvm_libc::fopen(FILENAME, "w");
+  const char *FILENAME = "fscanf_output.test";
+  auto FILE_PATH = libc_make_test_file_path(FILENAME);
+  ::FILE *file = scanf_test::fopen(FILE_PATH, "w");
   ASSERT_FALSE(file == nullptr);
 
   int read;
@@ -28,26 +46,26 @@ TEST(LlvmLibcFScanfTest, WriteToFile) {
   constexpr char simple[] = "A simple string with no conversions.\n";
 
   ASSERT_EQ(sizeof(simple) - 1,
-            __llvm_libc::fwrite(simple, 1, sizeof(simple) - 1, file));
+            scanf_test::fwrite(simple, 1, sizeof(simple) - 1, file));
 
   constexpr char numbers[] = "1234567890\n";
 
   ASSERT_EQ(sizeof(numbers) - 1,
-            __llvm_libc::fwrite(numbers, 1, sizeof(numbers) - 1, file));
+            scanf_test::fwrite(numbers, 1, sizeof(numbers) - 1, file));
 
   constexpr char numbers_and_more[] = "1234 and more\n";
 
   ASSERT_EQ(sizeof(numbers_and_more) - 1,
-            __llvm_libc::fwrite(numbers_and_more, 1,
-                                sizeof(numbers_and_more) - 1, file));
+            scanf_test::fwrite(numbers_and_more, 1,
+                               sizeof(numbers_and_more) - 1, file));
 
   read =
       __llvm_libc::fscanf(file, "Reading from a write-only file should fail.");
   EXPECT_LT(read, 0);
 
-  ASSERT_EQ(0, __llvm_libc::fclose(file));
+  ASSERT_EQ(0, scanf_test::fclose(file));
 
-  file = __llvm_libc::fopen(FILENAME, "r");
+  file = scanf_test::fopen(FILE_PATH, "r");
   ASSERT_FALSE(file == nullptr);
 
   char data[50];
@@ -67,6 +85,6 @@ TEST(LlvmLibcFScanfTest, WriteToFile) {
   ASSERT_EQ(__llvm_libc::cpp::string_view(numbers_and_more),
             __llvm_libc::cpp::string_view(data, sizeof(numbers_and_more) - 1));
 
-  ASSERT_EQ(__llvm_libc::ferror(file), 0);
-  ASSERT_EQ(__llvm_libc::fclose(file), 0);
+  ASSERT_EQ(scanf_test::ferror(file), 0);
+  ASSERT_EQ(scanf_test::fclose(file), 0);
 }
diff --git a/libc/test/src/stdio/scanf_core/CMakeLists.txt b/libc/test/src/stdio/scanf_core/CMakeLists.txt
index db20335a5c94301..a6ff3ec66abc326 100644
--- a/libc/test/src/stdio/scanf_core/CMakeLists.txt
+++ b/libc/test/src/stdio/scanf_core/CMakeLists.txt
@@ -13,24 +13,23 @@ add_libc_unittest(
     libc.src.__support.arg_list
 )
 
-if(NOT (TARGET libc.src.__support.File.file))
-  # Not all platforms have a file implementation. If file is unvailable,
-  # then we must skip all the parts that need file.
-  return()
-endif()
-
 add_libc_unittest(
-  string_reader_test
+  reader_test
   SUITE
     libc_stdio_unittests
   SRCS
-    string_reader_test.cpp
+    reader_test.cpp
   DEPENDS
     libc.src.stdio.scanf_core.reader
-    libc.src.stdio.scanf_core.string_reader
     libc.src.__support.CPP.string_view
 )
 
+if(NOT (TARGET libc.src.__support.File.file))
+  # Not all platforms have a file implementation. If file is unvailable,
+  # then we must skip all the parts that need file.
+  return()
+endif()
+
 add_libc_unittest(
   converter_test
   SUITE
diff --git a/libc/test/src/stdio/scanf_core/string_reader_test.cpp b/libc/test/src/stdio/scanf_core/reader_test.cpp
similarity index 76%
rename from libc/test/src/stdio/scanf_core/string_reader_test.cpp
rename to libc/test/src/stdio/scanf_core/reader_test.cpp
index 4b3d14855d5174b..9db916c115a2424 100644
--- a/libc/test/src/stdio/scanf_core/string_reader_test.cpp
+++ b/libc/test/src/stdio/scanf_core/reader_test.cpp
@@ -8,20 +8,21 @@
 
 #include "src/__support/CPP/string_view.h"
 #include "src/stdio/scanf_core/reader.h"
-#include "src/stdio/scanf_core/string_reader.h"
 
 #include "test/UnitTest/Test.h"
 
 TEST(LlvmLibcScanfStringReaderTest, Constructor) {
   char str[10];
-  __llvm_libc::scanf_core::StringReader str_reader(str);
-  __llvm_libc::scanf_core::Reader reader(&str_reader);
+  // buff_len justneeds to be a big number. The specific value isn't important
+  // in the real world.
+  __llvm_libc::scanf_core::ReadBuffer rb{const_cast<char *>(str), 1000000};
+  __llvm_libc::scanf_core::Reader reader(&rb);
 }
 
 TEST(LlvmLibcScanfStringReaderTest, SimpleRead) {
   const char *str = "abc";
-  __llvm_libc::scanf_core::StringReader str_reader(str);
-  __llvm_libc::scanf_core::Reader reader(&str_reader);
+  __llvm_libc::scanf_core::ReadBuffer rb{const_cast<char *>(str), 1000000};
+  __llvm_libc::scanf_core::Reader reader(&rb);
 
   for (size_t i = 0; i < sizeof("abc"); ++i) {
     ASSERT_EQ(str[i], reader.getc());
@@ -30,8 +31,8 @@ TEST(LlvmLibcScanfStringReaderTest, SimpleRead) {
 
 TEST(LlvmLibcScanfStringReaderTest, ReadAndReverse) {
   const char *str = "abcDEF123";
-  __llvm_libc::scanf_core::StringReader str_reader(str);
-  __llvm_libc::scanf_core::Reader reader(&str_reader);
+  __llvm_libc::scanf_core::ReadBuffer rb{const_cast<char *>(str), 1000000};
+  __llvm_libc::scanf_core::Reader reader(&rb);
 
   for (size_t i = 0; i < 5; ++i) {
     ASSERT_EQ(str[i], reader.getc());



More information about the cfe-commits mailing list