[libc] Implement 'vfscanf' and 'vscanf' #105293

jhuber6 · 2024-08-20T19:37:40Z

Summary:
These are simply forwarding the vlist to the existing helper.

llvmbot · 2024-08-20T19:38:17Z

@llvm/pr-subscribers-libc

Author: Joseph Huber (jhuber6)

Changes

Summary:
These are simply forwarding the vlist to the existing helper.

Full diff: https://github.com/llvm/llvm-project/pull/105293.diff

10 Files Affected:

(modified) libc/config/linux/aarch64/entrypoints.txt (+2)
(modified) libc/config/linux/riscv/entrypoints.txt (+2)
(modified) libc/config/linux/x86_64/entrypoints.txt (+2)
(modified) libc/src/stdio/CMakeLists.txt (+20)
(added) libc/src/stdio/vfscanf.cpp (+34)
(added) libc/src/stdio/vfscanf.h (+24)
(added) libc/src/stdio/vscanf.cpp (+40)
(added) libc/src/stdio/vscanf.h (+23)
(modified) libc/test/src/stdio/CMakeLists.txt (+14)
(added) libc/test/src/stdio/vfscanf_test.cpp (+98)

diff --git a/libc/config/linux/aarch64/entrypoints.txt b/libc/config/linux/aarch64/entrypoints.txt
index d22bd1153598eb..60aa7f5ccb319a 100644
--- a/libc/config/linux/aarch64/entrypoints.txt
+++ b/libc/config/linux/aarch64/entrypoints.txt
@@ -211,10 +211,12 @@ set(TARGET_LIBC_ENTRYPOINTS
     libc.src.stdio.fileno
     libc.src.stdio.fprintf
     libc.src.stdio.fscanf
+    libc.src.stdio.vfscanf
     libc.src.stdio.printf
     libc.src.stdio.remove
     libc.src.stdio.rename
     libc.src.stdio.scanf
+    libc.src.stdio.vscanf
     libc.src.stdio.snprintf
     libc.src.stdio.sprintf
     libc.src.stdio.asprintf
diff --git a/libc/config/linux/riscv/entrypoints.txt b/libc/config/linux/riscv/entrypoints.txt
index 1a647737ec455a..9a2746dcb86f87 100644
--- a/libc/config/linux/riscv/entrypoints.txt
+++ b/libc/config/linux/riscv/entrypoints.txt
@@ -210,10 +210,12 @@ set(TARGET_LIBC_ENTRYPOINTS
     libc.src.stdio.fileno
     libc.src.stdio.fprintf
     libc.src.stdio.fscanf
+    libc.src.stdio.vfscanf
     libc.src.stdio.printf
     libc.src.stdio.remove
     libc.src.stdio.rename
     libc.src.stdio.scanf
+    libc.src.stdio.vscanf
     libc.src.stdio.snprintf
     libc.src.stdio.sprintf
     libc.src.stdio.asprintf
diff --git a/libc/config/linux/x86_64/entrypoints.txt b/libc/config/linux/x86_64/entrypoints.txt
index 65c5757efe6274..71590ece1d3f9f 100644
--- a/libc/config/linux/x86_64/entrypoints.txt
+++ b/libc/config/linux/x86_64/entrypoints.txt
@@ -210,10 +210,12 @@ set(TARGET_LIBC_ENTRYPOINTS
     libc.src.stdio.fileno
     libc.src.stdio.fprintf
     libc.src.stdio.fscanf
+    libc.src.stdio.vfscanf
     libc.src.stdio.printf
     libc.src.stdio.remove
     libc.src.stdio.rename
     libc.src.stdio.scanf
+    libc.src.stdio.vscanf
     libc.src.stdio.snprintf
     libc.src.stdio.sprintf
     libc.src.stdio.asprintf
diff --git a/libc/src/stdio/CMakeLists.txt b/libc/src/stdio/CMakeLists.txt
index bc5ef5fe0e9b48..79069f1e74f05e 100644
--- a/libc/src/stdio/CMakeLists.txt
+++ b/libc/src/stdio/CMakeLists.txt
@@ -143,6 +143,16 @@ add_entrypoint_object(
     ${scanf_deps}
 )
 
+add_entrypoint_object(
+  vfscanf
+  SRCS
+    vfscanf.cpp
+  HDRS
+    vfscanf.h
+  DEPENDS
+    ${scanf_deps}
+)
+
 add_entrypoint_object(
   scanf
   SRCS
@@ -153,6 +163,16 @@ add_entrypoint_object(
     ${scanf_deps}
 )
 
+add_entrypoint_object(
+  vscanf
+  SRCS
+    vscanf.cpp
+  HDRS
+    vscanf.h
+  DEPENDS
+    ${scanf_deps}
+)
+
 add_entrypoint_object(
   sprintf
   SRCS
diff --git a/libc/src/stdio/vfscanf.cpp b/libc/src/stdio/vfscanf.cpp
new file mode 100644
index 00000000000000..220576522d0fdb
--- /dev/null
+++ b/libc/src/stdio/vfscanf.cpp
@@ -0,0 +1,34 @@
+//===-- 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/vfscanf.h"
+
+#include "src/__support/File/file.h"
+#include "src/__support/arg_list.h"
+#include "src/__support/macros/config.h"
+#include "src/stdio/scanf_core/vfscanf_internal.h"
+
+#include "hdr/types/FILE.h"
+#include <stdarg.h>
+
+namespace LIBC_NAMESPACE_DECL {
+
+LLVM_LIBC_FUNCTION(int, vfscanf,
+                   (::FILE *__restrict stream, const char *__restrict format,
+                    va_list vlist)) {
+  internal::ArgList args(vlist); // This holder class allows for easier copying
+                                 // and pointer semantics, as well as handling
+                                 // destruction automatically.
+  va_end(vlist);
+  int ret_val = scanf_core::vfscanf_internal(stream, 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;
+}
+
+} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/stdio/vfscanf.h b/libc/src/stdio/vfscanf.h
new file mode 100644
index 00000000000000..1a0a12d9eb4cd3
--- /dev/null
+++ b/libc/src/stdio/vfscanf.h
@@ -0,0 +1,24 @@
+//===-- Implementation header 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_STDIO_VFSCANF_H
+#define LLVM_LIBC_SRC_STDIO_VFSCANF_H
+
+#include "hdr/types/FILE.h"
+#include "src/__support/macros/config.h"
+
+#include <stdarg.h>
+
+namespace LIBC_NAMESPACE_DECL {
+
+int vfscanf(::FILE *__restrict stream, const char *__restrict format,
+            va_list vlist);
+
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif // LLVM_LIBC_SRC_STDIO_VFSCANF_H
diff --git a/libc/src/stdio/vscanf.cpp b/libc/src/stdio/vscanf.cpp
new file mode 100644
index 00000000000000..64f5cc1d6962a1
--- /dev/null
+++ b/libc/src/stdio/vscanf.cpp
@@ -0,0 +1,40 @@
+//===-- Implementation of vscanf --------------------------------*- 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/vscanf.h"
+
+#include "src/__support/File/file.h"
+#include "src/__support/arg_list.h"
+#include "src/__support/macros/config.h"
+#include "src/stdio/scanf_core/vfscanf_internal.h"
+
+#include "hdr/types/FILE.h"
+#include <stdarg.h>
+
+#ifndef LIBC_COPT_STDIO_USE_SYSTEM_FILE
+#define SCANF_STDIN LIBC_NAMESPACE::stdin
+#else // LIBC_COPT_STDIO_USE_SYSTEM_FILE
+#define SCANF_STDIN ::stdin
+#endif // LIBC_COPT_STDIO_USE_SYSTEM_FILE
+
+namespace LIBC_NAMESPACE_DECL {
+
+LLVM_LIBC_FUNCTION(int, vscanf,
+                   (const char *__restrict format, va_list vlist)) {
+  internal::ArgList args(vlist); // This holder class allows for easier copying
+                                 // and pointer semantics, as well as handling
+                                 // destruction automatically.
+  va_end(vlist);
+  int ret_val = scanf_core::vfscanf_internal(
+      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;
+}
+
+} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/stdio/vscanf.h b/libc/src/stdio/vscanf.h
new file mode 100644
index 00000000000000..5c59b91128ea32
--- /dev/null
+++ b/libc/src/stdio/vscanf.h
@@ -0,0 +1,23 @@
+//===-- Implementation header of vscanf -------------------------*- 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_VSCANF_H
+#define LLVM_LIBC_SRC_STDIO_VSCANF_H
+
+#include "hdr/types/FILE.h"
+#include "src/__support/macros/config.h"
+
+#include <stdarg.h>
+
+namespace LIBC_NAMESPACE_DECL {
+
+int vscanf(const char *__restrict format, va_list vlist);
+
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif // LLVM_LIBC_SRC_STDIO_VSCANF_H
diff --git a/libc/test/src/stdio/CMakeLists.txt b/libc/test/src/stdio/CMakeLists.txt
index 8b05b928a02695..f250d94786e661 100644
--- a/libc/test/src/stdio/CMakeLists.txt
+++ b/libc/test/src/stdio/CMakeLists.txt
@@ -286,6 +286,20 @@ add_libc_test(
     ${use_system_file}
 )
 
+add_libc_test(
+  vfscanf_test
+  SUITE
+    libc_stdio_unittests
+  SRCS
+    fscanf_test.cpp
+  DEPENDS
+    libc.src.stdio.vfscanf
+    ${fscanf_test_deps}
+    libc.src.__support.CPP.string_view
+  COMPILE_OPTIONS
+    ${use_system_file}
+)
+
 if(LIBC_CONF_SCANF_DISABLE_FLOAT)
   list(APPEND sscanf_test_copts "-DLIBC_COPT_SCANF_DISABLE_FLOAT")
 endif()
diff --git a/libc/test/src/stdio/vfscanf_test.cpp b/libc/test/src/stdio/vfscanf_test.cpp
new file mode 100644
index 00000000000000..7a9cbf7f123880
--- /dev/null
+++ b/libc/test/src/stdio/vfscanf_test.cpp
@@ -0,0 +1,98 @@
+//===-- Unittests for vfscanf ---------------------------------------------===//
+//
+// 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/__support/CPP/string_view.h"
+
+#ifndef LIBC_COPT_STDIO_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_STDIO_USE_SYSTEM_FILE
+
+#include "src/stdio/vfscanf.h"
+
+#include "test/UnitTest/Test.h"
+
+#include <stdio.h>
+
+namespace scanf_test {
+#ifndef LIBC_COPT_STDIO_USE_SYSTEM_FILE
+using LIBC_NAMESPACE::fclose;
+using LIBC_NAMESPACE::ferror;
+using LIBC_NAMESPACE::fopen;
+using LIBC_NAMESPACE::fwrite;
+#else  // defined(LIBC_COPT_STDIO_USE_SYSTEM_FILE)
+using ::fclose;
+using ::ferror;
+using ::fopen;
+using ::fwrite;
+#endif // LIBC_COPT_STDIO_USE_SYSTEM_FILE
+} // namespace scanf_test
+
+static int call_vfscanf(::FILE *stream, const char *__restrict format, ...) {
+  va_list vlist;
+  va_start(vlist, format);
+  int ret = LIBC_NAMESPACE::vfscanf(stream, format, vlist);
+  va_end(vlist);
+  return ret;
+}
+
+TEST(LlvmLibcFScanfTest, WriteToFile) {
+  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;
+
+  constexpr char simple[] = "A simple string with no conversions.\n";
+
+  ASSERT_EQ(sizeof(simple) - 1,
+            scanf_test::fwrite(simple, 1, sizeof(simple) - 1, file));
+
+  constexpr char numbers[] = "1234567890\n";
+
+  ASSERT_EQ(sizeof(numbers) - 1,
+            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,
+            scanf_test::fwrite(numbers_and_more, 1,
+                               sizeof(numbers_and_more) - 1, file));
+
+  read = call_vfscanf(file, "Reading from a write-only file should fail.");
+  EXPECT_LT(read, 0);
+
+  ASSERT_EQ(0, scanf_test::fclose(file));
+
+  file = scanf_test::fopen(FILE_PATH, "r");
+  ASSERT_FALSE(file == nullptr);
+
+  char data[50];
+  read = call_vfscanf(file, "%[A-Za-z .\n]", data);
+  ASSERT_EQ(read, 1);
+  ASSERT_STREQ(simple, data);
+
+  read = call_vfscanf(file, "%s", data);
+  ASSERT_EQ(read, 1);
+  ASSERT_EQ(LIBC_NAMESPACE::cpp::string_view(numbers, 10),
+            LIBC_NAMESPACE::cpp::string_view(data));
+
+  // The format string starts with a space to handle the fact that the %s leaves
+  // a trailing \n and %c doesn't strip leading whitespace.
+  read = call_vfscanf(file, " %50c", data);
+  ASSERT_EQ(read, 1);
+  ASSERT_EQ(
+      LIBC_NAMESPACE::cpp::string_view(numbers_and_more),
+      LIBC_NAMESPACE::cpp::string_view(data, sizeof(numbers_and_more) - 1));
+
+  ASSERT_EQ(scanf_test::ferror(file), 0);
+  ASSERT_EQ(scanf_test::fclose(file), 0);
+}

jhuber6 · 2024-08-23T19:11:22Z

ping

michaelrj-google

LGTM

michaelrj-google

Wait, it looks like you forgot the headergen stuff for these. Please add those before landing

Summary: These are simply forwarding the vlist to the existing helper.

jhuber6 · 2024-08-23T21:25:39Z

Wait, it looks like you forgot the headergen stuff for these. Please add those before landing

Done

michaelrj-google

LGTM

In patch llvm#105293 tests for vfscanf were added, meant to be identical to the fscanf tests. Unfortunately, the author forgot to rename the target file causing an occasional test flake where one test writes to the file while the other is trying to read it. This patch fixes the issue by renaming the target test file for the vfscanf test.

In patch #105293 tests for vfscanf were added, meant to be identical to the fscanf tests. Unfortunately, the author forgot to rename the target file causing an occasional test flake where one test writes to the file while the other is trying to read it. This patch fixes the issue by renaming the target test file for the vfscanf test.

jhuber6 requested a review from michaelrj-google August 20, 2024 19:37

llvmbot added the libc label Aug 20, 2024

jhuber6 force-pushed the vfscanf branch from 72215fb to 795d5e3 Compare August 20, 2024 20:01

jhuber6 requested review from SchrodingerZhu and lntue August 20, 2024 20:05

michaelrj-google approved these changes Aug 23, 2024

View reviewed changes

michaelrj-google requested changes Aug 23, 2024

View reviewed changes

[libc] Implement 'vfscanf' and 'vscanf'

940bbc7

Summary: These are simply forwarding the vlist to the existing helper.

jhuber6 force-pushed the vfscanf branch from 795d5e3 to 940bbc7 Compare August 23, 2024 21:25

michaelrj-google approved these changes Aug 23, 2024

View reviewed changes

jhuber6 merged commit b8f134f into llvm:main Aug 26, 2024
7 checks passed

michaelrj-google mentioned this pull request Aug 26, 2024

[libc] Fix file collision causing test flake #106119

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[libc] Implement 'vfscanf' and 'vscanf' #105293

[libc] Implement 'vfscanf' and 'vscanf' #105293

Uh oh!

jhuber6 commented Aug 20, 2024

Uh oh!

llvmbot commented Aug 20, 2024

Uh oh!

jhuber6 commented Aug 23, 2024

Uh oh!

michaelrj-google left a comment

Uh oh!

michaelrj-google left a comment

Uh oh!

jhuber6 commented Aug 23, 2024

Uh oh!

michaelrj-google left a comment

Uh oh!

Uh oh!

Uh oh!

[libc] Implement 'vfscanf' and 'vscanf' #105293

[libc] Implement 'vfscanf' and 'vscanf' #105293

Uh oh!

Conversation

jhuber6 commented Aug 20, 2024

Uh oh!

llvmbot commented Aug 20, 2024

Uh oh!

jhuber6 commented Aug 23, 2024

Uh oh!

michaelrj-google left a comment

Choose a reason for hiding this comment

Uh oh!

michaelrj-google left a comment

Choose a reason for hiding this comment

Uh oh!

jhuber6 commented Aug 23, 2024

Uh oh!

michaelrj-google left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!