From: Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>
To: Florian Weimer <fweimer@redhat.com>, libc-alpha@sourceware.org
Subject: Re: [PATCH v5 10/11] libio: Convert __obstack_vprintf_internal to buffers (bug 27124)
Date: Thu, 15 Dec 2022 16:14:43 -0300 [thread overview]
Message-ID: <9ab5f463-fe9c-8c11-7b83-d9e330bc0275@linaro.org> (raw)
In-Reply-To: <45234c5d6ba9556451424940fa746e9e66b1a6f4.1670858473.git.fweimer@redhat.com>
On 12/12/22 12:24, Florian Weimer via Libc-alpha wrote:
> This fixes bug 27124 because the problematic built-in vtable is gone.
Patch looks good, some minor nit below.
> ---
> include/printf_buffer.h | 4 +
> libio/obprintf.c | 170 +++++++++--------------------
> stdio-common/printf_buffer_flush.c | 4 +
> 3 files changed, 58 insertions(+), 120 deletions(-)
>
> diff --git a/include/printf_buffer.h b/include/printf_buffer.h
> index 3d4ef1d06c..a8211dd657 100644
> --- a/include/printf_buffer.h
> +++ b/include/printf_buffer.h
> @@ -54,6 +54,7 @@ enum __printf_buffer_mode
> __printf_buffer_mode_fp, /* For __printf_fp_l_buffer. */
> __printf_buffer_mode_fp_to_wide, /* For __wprintf_fp_l_buffer. */
> __printf_buffer_mode_fphex_to_wide, /* For __wprintf_fphex_l_buffer. */
> + __printf_buffer_mode_obstack,
Maybe add 'For __printf_buffer_flush_obstack'?
> };
>
> /* Buffer for fast character writing with overflow handling.
> @@ -319,6 +320,9 @@ struct __printf_buffer_fphex_to_wide;
> void __printf_buffer_flush_fphex_to_wide (struct
> __printf_buffer_fphex_to_wide *)
> attribute_hidden;
> +struct __printf_buffer_obstack;
> +void __printf_buffer_flush_obstack (struct __printf_buffer_obstack *)
> + attribute_hidden;
>
> struct __wprintf_buffer_to_file;
> void __wprintf_buffer_flush_to_file (struct __wprintf_buffer_to_file *)
> diff --git a/libio/obprintf.c b/libio/obprintf.c
> index c220be9adc..459d73ebf3 100644
> --- a/libio/obprintf.c
> +++ b/libio/obprintf.c
> @@ -16,131 +16,59 @@
> License along with the GNU C Library; if not, see
> <https://www.gnu.org/licenses/>. */
>
> -
> -#include <stdlib.h>
> -#include "libioP.h"
> -#include "strfile.h"
> #include <assert.h>
> -#include <string.h>
> -#include <errno.h>
> +#include <math_ldbl_opt.h>
> #include <obstack.h>
> +#include <printf.h>
> #include <stdarg.h>
> -#include <stdio_ext.h>
> -
> +#include <printf_buffer.h>
>
> -struct _IO_obstack_file
> +struct __printf_buffer_obstack
> {
> - struct _IO_FILE_plus file;
> + struct __printf_buffer base;
> struct obstack *obstack;
> -};
> -
> -
> -static int
> -_IO_obstack_overflow (FILE *fp, int c)
> -{
> - struct obstack *obstack = ((struct _IO_obstack_file *) fp)->obstack;
> - int size;
> -
> - /* Make room for another character. This might as well allocate a
> - new chunk a memory and moves the old contents over. */
> - assert (c != EOF);
> - obstack_1grow (obstack, c);
> -
> - /* Setup the buffer pointers again. */
> - fp->_IO_write_base = obstack_base (obstack);
> - fp->_IO_write_ptr = obstack_next_free (obstack);
> - size = obstack_room (obstack);
> - fp->_IO_write_end = fp->_IO_write_ptr + size;
> - /* Now allocate the rest of the current chunk. */
> - obstack_blank_fast (obstack, size);
> -
> - return c;
> -}
>
> + /* obstack_1grow is called for compatibility reasons. This needs
> + one extra character, and this is the backing store for it. */
> + char ch;
> +};
>
> -static size_t
> -_IO_obstack_xsputn (FILE *fp, const void *data, size_t n)
> +void
> +__printf_buffer_flush_obstack (struct __printf_buffer_obstack *buf)
> {
> - struct obstack *obstack = ((struct _IO_obstack_file *) fp)->obstack;
> + /* About to switch buffers, so record the bytes written so far. */
> + buf->base.written += buf->base.write_ptr - buf->base.write_base;
>
> - if (fp->_IO_write_ptr + n > fp->_IO_write_end)
> + if (buf->base.write_ptr == &buf->ch + 1)
> {
> - int size;
> -
> - /* We need some more memory. First shrink the buffer to the
> - space we really currently need. */
> - obstack_blank_fast (obstack, fp->_IO_write_ptr - fp->_IO_write_end);
> -
> - /* Now grow for N bytes, and put the data there. */
> - obstack_grow (obstack, data, n);
> -
> - /* Setup the buffer pointers again. */
> - fp->_IO_write_base = obstack_base (obstack);
> - fp->_IO_write_ptr = obstack_next_free (obstack);
> - size = obstack_room (obstack);
> - fp->_IO_write_end = fp->_IO_write_ptr + size;
> - /* Now allocate the rest of the current chunk. */
> - obstack_blank_fast (obstack, size);
> + /* Errors are reported via a callback mechanism (presumably for
> + process termination). */
> + obstack_1grow (buf->obstack, buf->ch);
> + buf->base.write_base = obstack_next_free (buf->obstack);
> + buf->base.write_ptr = buf->base.write_base;
> + size_t size = obstack_room (buf->obstack);
> + buf->base.write_end = buf->base.write_ptr + size;
> + /* Reserve the space on the obstack size. */
> + obstack_blank_fast (buf->obstack, size);
> }
> else
> - fp->_IO_write_ptr = __mempcpy (fp->_IO_write_ptr, data, n);
> -
> - return n;
> + {
> + /* Obtain the extra character. */
> + buf->base.write_base = &buf->ch;
> + buf->base.write_ptr = &buf->ch;
> + buf->base.write_end = &buf->ch + 1;
> + }
> }
>
> -
> -/* the jump table. */
> -const struct _IO_jump_t _IO_obstack_jumps libio_vtable attribute_hidden =
> -{
> - JUMP_INIT_DUMMY,
> - JUMP_INIT(finish, NULL),
> - JUMP_INIT(overflow, _IO_obstack_overflow),
> - JUMP_INIT(underflow, NULL),
> - JUMP_INIT(uflow, NULL),
> - JUMP_INIT(pbackfail, NULL),
> - JUMP_INIT(xsputn, _IO_obstack_xsputn),
> - JUMP_INIT(xsgetn, NULL),
> - JUMP_INIT(seekoff, NULL),
> - JUMP_INIT(seekpos, NULL),
> - JUMP_INIT(setbuf, NULL),
> - JUMP_INIT(sync, NULL),
> - JUMP_INIT(doallocate, NULL),
> - JUMP_INIT(read, NULL),
> - JUMP_INIT(write, NULL),
> - JUMP_INIT(seek, NULL),
> - JUMP_INIT(close, NULL),
> - JUMP_INIT(stat, NULL),
> - JUMP_INIT(showmanyc, NULL),
> - JUMP_INIT(imbue, NULL)
> -};
> -
> -
Remove _IO_obstack_jumps from tst-relro-libc.out list.
> int
> __obstack_vprintf_internal (struct obstack *obstack, const char *format,
> va_list args, unsigned int mode_flags)
> {
> - struct obstack_FILE
> - {
> - struct _IO_obstack_file ofile;
> - } new_f;
> - int result;
> - int size;
> - int room;
> -
> -#ifdef _IO_MTSAFE_IO
> - new_f.ofile.file.file._lock = NULL;
> -#endif
> -
> - _IO_no_init (&new_f.ofile.file.file, _IO_USER_LOCK, -1, NULL, NULL);
> - _IO_JUMPS (&new_f.ofile.file) = &_IO_obstack_jumps;
> - room = obstack_room (obstack);
> - size = obstack_object_size (obstack) + room;
> + /* Legacy setup code for compatibility. */
> + size_t room = obstack_room (obstack);
> + size_t size = obstack_object_size (obstack) + room;
> if (size == 0)
> {
> - /* We have to handle the allocation a bit different since the
> - `_IO_str_init_static' function would handle a size of zero
> - different from what we expect. */
> -
> /* Get more memory. */
> obstack_make_room (obstack, 64);
>
> @@ -151,27 +79,29 @@ __obstack_vprintf_internal (struct obstack *obstack, const char *format,
> assert (size != 0);
> }
>
> - _IO_str_init_static_internal ((struct _IO_strfile_ *) &new_f.ofile,
> - obstack_base (obstack),
> - size, obstack_next_free (obstack));
> + struct __printf_buffer_obstack buf;
> + {
> + /* The obstack write location might be in the middle of an object. */
> + char *ptr = obstack_next_free (obstack);
> + char *end = obstack_base (obstack) + size;
> + __printf_buffer_init (&buf.base, ptr, end - ptr,
> + __printf_buffer_mode_obstack);
> + }
> + buf.obstack = obstack;
> +
> /* Now allocate the rest of the current chunk. */
> - assert (size == (new_f.ofile.file.file._IO_write_end
> - - new_f.ofile.file.file._IO_write_base));
> - assert (new_f.ofile.file.file._IO_write_ptr
> - == (new_f.ofile.file.file._IO_write_base
> - + obstack_object_size (obstack)));
> obstack_blank_fast (obstack, room);
>
> - new_f.ofile.obstack = obstack;
> -
> - result = __vfprintf_internal (&new_f.ofile.file.file, format, args,
> - mode_flags);
> + __printf_buffer (&buf.base, format, args, mode_flags);
>
> - /* Shrink the buffer to the space we really currently need. */
> - obstack_blank_fast (obstack, (new_f.ofile.file.file._IO_write_ptr
> - - new_f.ofile.file.file._IO_write_end));
> + if (buf.base.write_ptr == &buf.ch + 1)
> + /* buf.ch is in use. Put it into the obstack. */
> + obstack_1grow (buf.obstack, buf.ch);
> + else if (buf.base.write_ptr != &buf.ch)
> + /* Shrink the buffer to the space we really currently need. */
> + obstack_blank_fast (buf.obstack, buf.base.write_ptr - buf.base.write_end);
>
> - return result;
> + return __printf_buffer_done (&buf.base);
> }
>
> int
Ok.
> diff --git a/stdio-common/printf_buffer_flush.c b/stdio-common/printf_buffer_flush.c
> index 922340cc54..42afe49413 100644
> --- a/stdio-common/printf_buffer_flush.c
> +++ b/stdio-common/printf_buffer_flush.c
> @@ -32,6 +32,7 @@
> # pragma weak __printf_buffer_flush_fp
> # pragma weak __printf_buffer_flush_fp_to_wide
> # pragma weak __printf_buffer_flush_fphex_to_wide
> +# pragma weak __printf_buffer_flush_obstack
> #endif /* !SHARED */
>
> static void
> @@ -72,6 +73,9 @@ __printf_buffer_do_flush (struct __printf_buffer *buf)
> __printf_buffer_flush_fphex_to_wide
> ((struct __printf_buffer_fphex_to_wide *) buf);
> return;
> + case __printf_buffer_mode_obstack:
> + __printf_buffer_flush_obstack ((struct __printf_buffer_obstack *) buf);
> + return;
> }
> __builtin_trap ();
> }
next prev parent reply other threads:[~2022-12-15 19:14 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-12 15:22 [PATCH v5 00/11] vfprintf refactor Florian Weimer
2022-12-12 15:22 ` [PATCH v5 01/11] locale: Implement struct grouping_iterator Florian Weimer
2022-12-12 15:22 ` [PATCH v5 02/11] stdio-common: Introduce buffers for implementing printf Florian Weimer
2022-12-14 14:10 ` Adhemerval Zanella Netto
2022-12-16 17:51 ` Florian Weimer
2022-12-12 15:23 ` [PATCH v5 03/11] stdio-common: Add __printf_function_invoke Florian Weimer
2022-12-14 14:28 ` Adhemerval Zanella Netto
2022-12-12 15:23 ` [PATCH v5 04/11] stdio-common: Add __translated_number_width Florian Weimer
2022-12-14 14:38 ` Adhemerval Zanella Netto
2022-12-12 15:23 ` [PATCH v5 05/11] stdio-common: Convert vfprintf and related functions to buffers Florian Weimer
2022-12-14 20:54 ` Adhemerval Zanella Netto
2022-12-16 17:58 ` Florian Weimer
2022-12-12 15:23 ` [PATCH v5 06/11] stdio-common: Add lock optimization to vfprintf and vfwprintf Florian Weimer
2022-12-15 15:58 ` Adhemerval Zanella Netto
2022-12-12 15:23 ` [PATCH v5 07/11] libio: Convert __vsprintf_internal to buffers Florian Weimer
2022-12-15 18:16 ` Adhemerval Zanella Netto
2022-12-16 18:07 ` Florian Weimer
2022-12-12 15:23 ` [PATCH v5 08/11] libio: Convert __vasprintf_internal " Florian Weimer
2022-12-15 18:40 ` Adhemerval Zanella Netto
2022-12-16 18:22 ` Florian Weimer
2022-12-12 15:23 ` [PATCH v5 09/11] libio: Convert __vdprintf_internal " Florian Weimer
2022-12-15 19:11 ` Adhemerval Zanella Netto
2022-12-12 15:24 ` [PATCH v5 10/11] libio: Convert __obstack_vprintf_internal to buffers (bug 27124) Florian Weimer
2022-12-15 19:14 ` Adhemerval Zanella Netto [this message]
2022-12-16 18:25 ` Florian Weimer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9ab5f463-fe9c-8c11-7b83-d9e330bc0275@linaro.org \
--to=adhemerval.zanella@linaro.org \
--cc=fweimer@redhat.com \
--cc=libc-alpha@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).