Message ID | 2d35bb583d26a0dce2d905c2cc79fb87a8e9413b.1647544751.git.fweimer@redhat.com |
---|---|
State | New |
Headers | show |
Series | vfprintf rework to remove vtables | expand |
On 17/03/2022 16:29, Florian Weimer via Libc-alpha wrote: > This code path is exercised indirectly by some of the DNS stub > resolver tests, via their own use of xopen_memstream for constructing > strings describing result data. The relative lack of test suite > coverage became apparent when these tests starting failing after a > printf changes uncovered bug 28949. LGTM, with maybe a suggestio below. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > --- > stdio-common/Makefile | 1 + > stdio-common/tst-memstream-string.c | 85 +++++++++++++++++++++++++++++ > 2 files changed, 86 insertions(+) > create mode 100644 stdio-common/tst-memstream-string.c > > diff --git a/stdio-common/Makefile b/stdio-common/Makefile > index f0e65f0dcd..222c9ea63d 100644 > --- a/stdio-common/Makefile > +++ b/stdio-common/Makefile > @@ -173,6 +173,7 @@ tests := \ > tst-gets \ > tst-grouping \ > tst-long-dbl-fphex \ > + tst-memstream-string \ > tst-obprintf \ > tst-perror \ > tst-popen \ > diff --git a/stdio-common/tst-memstream-string.c b/stdio-common/tst-memstream-string.c > new file mode 100644 > index 0000000000..1c1bf0154a > --- /dev/null > +++ b/stdio-common/tst-memstream-string.c > @@ -0,0 +1,85 @@ > +/* Test writing differently sized strings to a memstream. > + Copyright (C) 2022 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <https://www.gnu.org/licenses/>. */ > + > +#include <stdio.h> > +#include <stdlib.h> > +#include <string.h> > +#include <support/check.h> > +#include <support/support.h> > +#include <support/xmemstream.h> > + > +/* Returns a printable ASCII character based on INDEX. */ > +static inline char > +char_from_index (unsigned int index) > +{ > + return ' ' + (index % 95); > +} > + > +/* Hide fprintf behind a compiler barrier, to avoid the fputs > + transformation. */ > +void __attribute__ ((weak)) > +fprintf_compiler_barrier (FILE *fp, const char *format, const char *arg) > +{ > + fprintf (fp, format, arg); > +} You can also avoid it by explicit disable the builtin on object build: CFLAGS-tst-memstream-string.c += -fno-builtin-fprintf > + > +enum { result_size = 25000 }; > + > +static void > +run_one_size (unsigned int chunk_size) > +{ > + char *chunk = xmalloc (chunk_size + 1); > + > + struct xmemstream mem; > + xopen_memstream (&mem); > + unsigned int written = 0; > + for (unsigned int i = 0; i < result_size; ) > + { > + unsigned int to_print = result_size - i; > + if (to_print > chunk_size) > + to_print = chunk_size; > + for (unsigned int j = 0; j < to_print; ++j) > + chunk[j] = char_from_index(i + j); > + chunk[to_print] = '\0'; > + fprintf_compiler_barrier (mem.out, "%s", chunk); > + i += to_print; > + written += strlen(chunk); > + } > + xfclose_memstream (&mem); > + > + TEST_COMPARE (written, result_size); > + TEST_COMPARE (mem.length, result_size); > + TEST_COMPARE (strlen (mem.buffer), result_size); > + > + for (unsigned int i = 0; i < result_size; ++i) > + TEST_COMPARE (mem.buffer[i], char_from_index (i)); > + > + free (mem.buffer); > + free (chunk); > +} > + > +static int > +do_test (void) > +{ > + for (unsigned int chunk_size = 1; chunk_size <= 30; ++ chunk_size) > + run_one_size (chunk_size); > + > + return 0; > +} > + > +#include <support/test-driver.c>
* Adhemerval Zanella: > On 17/03/2022 16:29, Florian Weimer via Libc-alpha wrote: >> This code path is exercised indirectly by some of the DNS stub >> resolver tests, via their own use of xopen_memstream for constructing >> strings describing result data. The relative lack of test suite >> coverage became apparent when these tests starting failing after a >> printf changes uncovered bug 28949. > > LGTM, with maybe a suggestio below. > > Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> >> +/* Hide fprintf behind a compiler barrier, to avoid the fputs >> + transformation. */ >> +void __attribute__ ((weak)) >> +fprintf_compiler_barrier (FILE *fp, const char *format, const char *arg) >> +{ >> + fprintf (fp, format, arg); >> +} > > You can also avoid it by explicit disable the builtin on object build: > > CFLAGS-tst-memstream-string.c += -fno-builtin-fprintf Thanks, I made this change. Florian
diff --git a/stdio-common/Makefile b/stdio-common/Makefile index f0e65f0dcd..222c9ea63d 100644 --- a/stdio-common/Makefile +++ b/stdio-common/Makefile @@ -173,6 +173,7 @@ tests := \ tst-gets \ tst-grouping \ tst-long-dbl-fphex \ + tst-memstream-string \ tst-obprintf \ tst-perror \ tst-popen \ diff --git a/stdio-common/tst-memstream-string.c b/stdio-common/tst-memstream-string.c new file mode 100644 index 0000000000..1c1bf0154a --- /dev/null +++ b/stdio-common/tst-memstream-string.c @@ -0,0 +1,85 @@ +/* Test writing differently sized strings to a memstream. + Copyright (C) 2022 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <https://www.gnu.org/licenses/>. */ + +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <support/check.h> +#include <support/support.h> +#include <support/xmemstream.h> + +/* Returns a printable ASCII character based on INDEX. */ +static inline char +char_from_index (unsigned int index) +{ + return ' ' + (index % 95); +} + +/* Hide fprintf behind a compiler barrier, to avoid the fputs + transformation. */ +void __attribute__ ((weak)) +fprintf_compiler_barrier (FILE *fp, const char *format, const char *arg) +{ + fprintf (fp, format, arg); +} + +enum { result_size = 25000 }; + +static void +run_one_size (unsigned int chunk_size) +{ + char *chunk = xmalloc (chunk_size + 1); + + struct xmemstream mem; + xopen_memstream (&mem); + unsigned int written = 0; + for (unsigned int i = 0; i < result_size; ) + { + unsigned int to_print = result_size - i; + if (to_print > chunk_size) + to_print = chunk_size; + for (unsigned int j = 0; j < to_print; ++j) + chunk[j] = char_from_index(i + j); + chunk[to_print] = '\0'; + fprintf_compiler_barrier (mem.out, "%s", chunk); + i += to_print; + written += strlen(chunk); + } + xfclose_memstream (&mem); + + TEST_COMPARE (written, result_size); + TEST_COMPARE (mem.length, result_size); + TEST_COMPARE (strlen (mem.buffer), result_size); + + for (unsigned int i = 0; i < result_size; ++i) + TEST_COMPARE (mem.buffer[i], char_from_index (i)); + + free (mem.buffer); + free (chunk); +} + +static int +do_test (void) +{ + for (unsigned int chunk_size = 1; chunk_size <= 30; ++ chunk_size) + run_one_size (chunk_size); + + return 0; +} + +#include <support/test-driver.c>