On 2023-04-02 18:43:21+0200, Willy Tarreau wrote:
> Thomas,
>
> On Sun, Apr 02, 2023 at 01:02:44PM +0000, Thomas Weißschuh wrote:
> > vfprintf() is complex and so far did not have proper tests.
> >
> > This series is based on the "dev" branch of the RCU tree.
>
> I've just ran it with glibc to see:
>
> $ gcc nolibc-test.c
> $ ./a.out vfprintf
> Running test 'vfprintf'
> 0 empty "" = "" [OK]
> 1 simple written(3) != read(0) [FAIL]
> 2 string written(3) != read(0) [FAIL]
> 3 number written(4) != read(0) [FAIL]
> 4 negnumber written(5) != read(0) [FAIL]
> 5 unsigned written(5) != read(0) [FAIL]
> 6 char written(1) != read(0) [FAIL]
> 7 hex written(1) != read(0) [FAIL]
> 8 pointer written(5) != 3 [FAIL]
> Errors during this test: 8
>
> The main issue was that glibc uses buffered writes by default.
>
> I could fix them with fflush() (which we don't have so it required an
> ifndef), and this also made me realize that we were missing an fclose()
> as well for compatibility with glibc. With this it got better:
>
> Running test 'vfprintf'
> 0 empty "" = "" [OK]
> 1 simple "foo" = "foo" [OK]
> 2 string "foo" = "foo" [OK]
> 3 number "1234" = "1234" [OK]
> 4 negnumber "-1234" = "-1234" [OK]
> 5 unsigned "12345" = "12345" [OK]
> 6 char "c" = "c" [OK]
> 7 hex "f" = "f" [OK]
> 8 pointer written(5) != 3 [FAIL]
> Errors during this test: 1
>
> This is caused by glibc emitting "(nil)" while we emit "0x0" for a NULL
> pointer since we use the same code as when dumping integers. I could fix
> that one as well by printing (void*)1 instead, which shows "0x1" for both.
>
> This gives me the patch below on top of yours, which I think would make
> sense to integrate in this form or a simplified one if we manage to add
> fflush() and fclose() earlier.
>
> What do you think ?
>
> Thanks,
> Willy
>
>
> diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
> index 28a8d77078dc..2958dc3eca93 100644
> --- a/tools/testing/selftests/nolibc/nolibc-test.c
> +++ b/tools/testing/selftests/nolibc/nolibc-test.c
> @@ -678,6 +678,7 @@ static int expect_vfprintf(int llen, size_t c, const char *expected, const char
> int ret, fd, w, r;
> char buf[100];
> va_list args;
> + FILE *memfile;
>
> fd = memfd_create("vfprintf", 0);
> if (fd == -1) {
> @@ -685,8 +686,14 @@ static int expect_vfprintf(int llen, size_t c, const char *expected, const char
> return 1;
> }
>
> + memfile = fdopen(fd, "w+");
> + if (!memfile) {
> + pad_spc(llen, 64, "[FAIL]\n");
> + return 1;
> + }
> +
> va_start(args, fmt);
> - w = vfprintf(fdopen(fd, "w+"), fmt, args);
> + w = vfprintf(memfile, fmt, args);
> va_end(args);
>
> if (w != c) {
> @@ -695,12 +702,19 @@ static int expect_vfprintf(int llen, size_t c, const char *expected, const char
> return 1;
> }
>
> +#ifndef _NOLIBC_STDIO_H
> + fflush(memfile);
> +#endif
> lseek(fd, 0, SEEK_SET);
>
> r = read(fd, buf, sizeof(buf) - 1);
> buf[r] = '\0';
>
> +#ifndef _NOLIBC_STDIO_H
> + fclose(memfile);
> +#else
> close(fd);
> +#endif
Wouldn't it be nicer to implement fflush/fclose in nolibc?
I can send a v3 with that.
> if (r != w) {
> llen += printf(" written(%d) != read(%d)", w, r);
> @@ -737,7 +751,7 @@ static int run_vfprintf(int min, int max)
> CASE_TEST(unsigned); EXPECT_VFPRINTF(5, "12345", "%u", 12345); break;
> CASE_TEST(char); EXPECT_VFPRINTF(1, "c", "%c", 'c'); break;
> CASE_TEST(hex); EXPECT_VFPRINTF(1, "f", "%x", 0xf); break;
> - CASE_TEST(pointer); EXPECT_VFPRINTF(3, "0x0", "%p", NULL); break;
> + CASE_TEST(pointer); EXPECT_VFPRINTF(3, "0x1", "%p", (void*)0x1); break;
> case __LINE__:
> return ret; /* must be last */
> /* note: do not set any defaults so as to permit holes above */
