From: Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>
To: Florian Weimer <fweimer@redhat.com>, libc-alpha@sourceware.org
Subject: Re: [PATCH v5 09/11] libio: Convert __vdprintf_internal to buffers
Date: Thu, 15 Dec 2022 16:11:14 -0300 [thread overview]
Message-ID: <858408de-a8e2-1a14-1074-11092228c9d9@linaro.org> (raw)
In-Reply-To: <2deb7d2cffb76dbcaf7ba051e616ccb7e275ed54.1670858473.git.fweimer@redhat.com>
On 12/12/22 12:23, Florian Weimer via Libc-alpha wrote:
> The internal buffer size is set to 2048 bytes. This is less than
> the original BUFSIZ value used by buffered_vfprintf before
> the conversion, but it hopefully covers all cases where write
> boundaries matter.
> ---
> include/printf_buffer.h | 4 ++
> libio/iovdprintf.c | 69 ++++++++++++++++++------------
> stdio-common/printf_buffer_flush.c | 4 ++
> 3 files changed, 49 insertions(+), 28 deletions(-)
>
> diff --git a/include/printf_buffer.h b/include/printf_buffer.h
> index 47e252b52e..3d4ef1d06c 100644
> --- a/include/printf_buffer.h
> +++ b/include/printf_buffer.h
> @@ -49,6 +49,7 @@ enum __printf_buffer_mode
> __printf_buffer_mode_sprintf_chk,
> __printf_buffer_mode_to_file,
> __printf_buffer_mode_asprintf,
> + __printf_buffer_mode_dprintf,
> __printf_buffer_mode_strfmon,
> __printf_buffer_mode_fp, /* For __printf_fp_l_buffer. */
> __printf_buffer_mode_fp_to_wide, /* For __wprintf_fp_l_buffer. */
> @@ -305,6 +306,9 @@ void __printf_buffer_flush_to_file (struct __printf_buffer_to_file *)
> struct __printf_buffer_asprintf;
> void __printf_buffer_flush_asprintf (struct __printf_buffer_asprintf *)
> attribute_hidden;
> +struct __printf_buffer_dprintf;
> +void __printf_buffer_flush_dprintf (struct __printf_buffer_dprintf *)
> + attribute_hidden;
> struct __printf_buffer_fp;
> void __printf_buffer_flush_fp (struct __printf_buffer_fp *)
> attribute_hidden;
> diff --git a/libio/iovdprintf.c b/libio/iovdprintf.c
> index 1e483868c9..f4cdd6b537 100644
> --- a/libio/iovdprintf.c
> +++ b/libio/iovdprintf.c
> @@ -24,41 +24,54 @@
> This exception applies to code released by its copyright holders
> in files containing the exception. */
>
> -#include <libioP.h>
> +#include <array_length.h>
> +#include <math_ldbl_opt.h>
> +#include <printf.h>
> #include <stdio_ext.h>
> +#include <unistd.h>
> +#include <printf_buffer.h>
>
> -int
> -__vdprintf_internal (int d, const char *format, va_list arg,
> - unsigned int mode_flags)
> +struct __printf_buffer_dprintf
> {
> - struct _IO_FILE_plus tmpfil;
> - struct _IO_wide_data wd;
> - int done;
> + struct __printf_buffer base;
> + int fd;
> +
> + /* This should cover most of the packet-oriented file descriptors,
> + where boundaries between writes could be visible to readers. */
> + char buf[2048];
> +};
I am even more inclined to move these magic buffer size constants to a
specific header.
Rest looks ok.
>
> -#ifdef _IO_MTSAFE_IO
> - tmpfil.file._lock = NULL;
> -#endif
> - _IO_no_init (&tmpfil.file, _IO_USER_LOCK, 0, &wd, &_IO_wfile_jumps);
> - _IO_JUMPS (&tmpfil) = &_IO_file_jumps;
> - _IO_new_file_init_internal (&tmpfil);
> - if (_IO_file_attach (&tmpfil.file, d) == NULL)
> +void
> +__printf_buffer_flush_dprintf (struct __printf_buffer_dprintf *buf)
> +{
> + char *p = buf->buf;
> + char *end = buf->base.write_ptr;
> + while (p < end)
> {
> - _IO_un_link (&tmpfil);
> - return EOF;
> + ssize_t ret = TEMP_FAILURE_RETRY (write (buf->fd, p, end - p));
> + if (ret < 0)
> + {
> + __printf_buffer_mark_failed (&buf->base);
> + return;
> + }
> + p += ret;
> }
> - tmpfil.file._flags |= _IO_DELETE_DONT_CLOSE;
> -
> - _IO_mask_flags (&tmpfil.file, _IO_NO_READS,
> - _IO_NO_READS+_IO_NO_WRITES+_IO_IS_APPENDING);
> -
> - done = __vfprintf_internal (&tmpfil.file, format, arg, mode_flags);
> -
> - if (done != EOF && _IO_do_flush (&tmpfil.file) == EOF)
> - done = EOF;
> -
> - _IO_FINISH (&tmpfil.file);
> + buf->base.write_ptr = buf->buf;
> +}
>
> - return done;
> +int
> +__vdprintf_internal (int d, const char *format, va_list arg,
> + unsigned int mode_flags)
> +{
> + struct __printf_buffer_dprintf buf;
> + __printf_buffer_init (&buf.base, buf.buf, array_length (buf.buf),
> + __printf_buffer_mode_dprintf);
> + buf.fd = d;
> + __printf_buffer (&buf.base, format, arg, mode_flags);
> + if (__printf_buffer_has_failed (&buf.base))
> + return -1;
> + __printf_buffer_flush_dprintf (&buf);
> + return __printf_buffer_done (&buf.base);
> }
>
> int
> diff --git a/stdio-common/printf_buffer_flush.c b/stdio-common/printf_buffer_flush.c
> index 14fe1b2df4..922340cc54 100644
> --- a/stdio-common/printf_buffer_flush.c
> +++ b/stdio-common/printf_buffer_flush.c
> @@ -28,6 +28,7 @@
> # pragma weak __printf_buffer_flush_snprintf
> # pragma weak __printf_buffer_flush_to_file
> # pragma weak __printf_buffer_flush_asprintf
> +# pragma weak __printf_buffer_flush_dprintf
> # pragma weak __printf_buffer_flush_fp
> # pragma weak __printf_buffer_flush_fp_to_wide
> # pragma weak __printf_buffer_flush_fphex_to_wide
> @@ -53,6 +54,9 @@ __printf_buffer_do_flush (struct __printf_buffer *buf)
> case __printf_buffer_mode_asprintf:
> __printf_buffer_flush_asprintf ((struct __printf_buffer_asprintf *) buf);
> return;
> + case __printf_buffer_mode_dprintf:
> + __printf_buffer_flush_dprintf ((struct __printf_buffer_dprintf *) buf);
> + return;
> case __printf_buffer_mode_strfmon:
> __set_errno (E2BIG);
> __printf_buffer_mark_failed (buf);
Ok.
next prev parent reply other threads:[~2022-12-15 19:11 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 [this message]
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
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=858408de-a8e2-1a14-1074-11092228c9d9@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).