From 5fa42012dab61dc19fd0d44462539127552e3744 Mon Sep 17 00:00:00 2001 From: Ivan Komarov Date: Mon, 19 Feb 2024 22:34:46 +0100 Subject: [PATCH] Fix reading the same symbol twice when using `{f,}scanf()` PR #924 appears to use `unget()` subtly incorrectly when parsing floating point numbers. The rest of the code only uses `unget()` immediately followed by `goto Done;` to return back the symbol that can't possibly belong to the directive we're processing. With floating-point, however, the ungot characters could very well be valid for the *next* directive, so we will essentially read them twice. It can't be seen in `sscanf()` tests because `unget()` is a no-op there, but the test I added for `fscanf()` fails like this: ... EXPECT_EQ(0xDEAD, i1) need 57005 (or 0xdead) = got 908973 (or 0x000ddead) ... EXPECT_EQ(0xBEEF, i2) need 48879 (or 0xbeef) = got 769775 (or 0x000bbeef) This means we read 0xDDEAD instead of 0xDEAD and 0xBBEEF instead of 0xBEEF. I checked that both musl and glibc read 0xDEAD/0xBEEF, as expected. Fix the failing test by removing the unneeded `unget()` calls. --- libc/stdio/vcscanf.c | 6 ------ test/libc/stdio/fscanf_test.c | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 6 deletions(-) create mode 100644 test/libc/stdio/fscanf_test.c diff --git a/libc/stdio/vcscanf.c b/libc/stdio/vcscanf.c index cb890edb4..c11a21e49 100644 --- a/libc/stdio/vcscanf.c +++ b/libc/stdio/vcscanf.c @@ -410,9 +410,6 @@ int __vcscanf(int callback(void *), // goto Done; } } else { - if (c != -1 && unget) { - unget(c, arg); - } goto GotFloatingPointNumber; } } else { @@ -465,9 +462,6 @@ int __vcscanf(int callback(void *), // Continue: continue; Break: - if (c != -1 && unget) { - unget(c, arg); - } break; } while ((c = BUFFER) != -1); GotFloatingPointNumber: diff --git a/test/libc/stdio/fscanf_test.c b/test/libc/stdio/fscanf_test.c new file mode 100644 index 000000000..d5b4e9de0 --- /dev/null +++ b/test/libc/stdio/fscanf_test.c @@ -0,0 +1,33 @@ +/*-*- mode:c;indent-tabs-mode:nil;c-basic-offset:2;tab-width:8;coding:utf-8 -*-│ +│ vi: set et ft=c ts=2 sts=2 sw=2 fenc=utf-8 :vi │ +╞══════════════════════════════════════════════════════════════════════════════╡ +│ Copyright 2022 Justine Alexandra Roberts Tunney │ +│ │ +│ Permission to use, copy, modify, and/or distribute this software for │ +│ any purpose with or without fee is hereby granted, provided that the │ +│ above copyright notice and this permission notice appear in all copies. │ +│ │ +│ THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL │ +│ WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED │ +│ WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE │ +│ AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL │ +│ DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR │ +│ PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER │ +│ TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR │ +│ PERFORMANCE OF THIS SOFTWARE. │ +╚─────────────────────────────────────────────────────────────────────────────*/ +#include "libc/math.h" +#include "libc/stdio/stdio.h" +#include "libc/testlib/testlib.h" + +TEST(fscanf, test_readAfterFloat) { + FILE *f = fmemopen("infDEAD-.125e-2BEEF", 19, "r"); + float f1 = 666.666f, f2 = f1; + int i1 = 666, i2 = i1; + EXPECT_EQ(4, fscanf(f, "%f%x%f%x", &f1, &i1, &f2, &i2)); + EXPECT_TRUE(isinf(f1)); + EXPECT_EQ(0xDEAD, i1); + EXPECT_EQ(-0.125e-2f, f2); + EXPECT_EQ(0xBEEF, i2); + fclose(f); +}