| Message ID | 345c3f94450a38be9408f158a211c82a657034b1.1707491940.git.fweimer@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Build getdomainname, gethostname, syslog with fortification | expand |
On 09/02/24 12:25, Florian Weimer wrote: > And __printf_buffer_asprintf_free as well. This will allow to reuse > the asprintf implementation for more general buffer handling, similar > to open_memstream, but without the mandatory heap allocation. LGTM, thanks. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > --- > include/printf_buffer.h | 30 +++++++++++++++++++++++++----- > libio/vasprintf.c | 40 +++++++++++++++++----------------------- > 2 files changed, 42 insertions(+), 28 deletions(-) > > diff --git a/include/printf_buffer.h b/include/printf_buffer.h > index fb0b42178e..cee5afb581 100644 > --- a/include/printf_buffer.h > +++ b/include/printf_buffer.h > @@ -317,6 +317,31 @@ void __printf_buffer_snprintf_init (struct __printf_buffer_snprintf *, > int __printf_buffer_snprintf_done (struct __printf_buffer_snprintf *) > attribute_hidden; > > +/* Size of the initial on-stack buffer for asprintf. It should be > + large enough to copy almost all asprintf usages with just a single > + (final, correctly sized) heap allocation. */ > +#define PRINTF_BUFFER_SIZE_ASPRINTF 200 On my system the journactl message has a mean size of 124 and a median of so I guess a size of 200 should cover most of the usage. > + > +struct __printf_buffer_asprintf > +{ > + /* base.write_base points either to a heap-allocated buffer, or to > + the direct array below. */ > + struct __printf_buffer base; > + > + /* Initial allocation. */ > + char direct[PRINTF_BUFFER_SIZE_ASPRINTF]; > +}; > + > +/* Sets up BUF for writing via __printf_buffer. */ > +void __printf_buffer_asprintf_init (struct __printf_buffer_asprintf *buf) > + attribute_hidden; > + > +/* Deallocates any allocated memory in *BUF. (This is not the usual > + done routine for asprintf because that has to preserve allocation.) > + Always returns -1 to indicate an error. */ > +int __printf_buffer_asprintf_free (struct __printf_buffer_asprintf *buf) > + attribute_hidden; > + > /* Flush function implementations follow. They are called from > __printf_buffer_flush. Generic code should not call these flush > functions directly. Some modes have inline implementations. */ > @@ -363,11 +388,6 @@ void __wprintf_buffer_flush_to_file (struct __wprintf_buffer_to_file *) > /* Temporary buffer used during floating point digit translation. */ > #define PRINTF_BUFFER_SIZE_DIGITS 64 > > -/* Size of the initial on-stack buffer for asprintf. It should be > - large enough to copy almost all asprintf usages with just a single > - (final, correctly sized) heap allocation. */ > -#define PRINTF_BUFFER_SIZE_ASPRINTF 200 > - > /* This should cover most of the packet-oriented file descriptors, > where boundaries between writes could be visible to readers. But > it is still small enough not to cause too many stack overflow issues. */ > diff --git a/libio/vasprintf.c b/libio/vasprintf.c > index edcfab2323..f8413eedfe 100644 > --- a/libio/vasprintf.c > +++ b/libio/vasprintf.c > @@ -34,18 +34,6 @@ > #include <string.h> > #include <printf_buffer.h> > > -struct __printf_buffer_asprintf > -{ > - /* base.write_base points either to a heap-allocated buffer, or to > - the direct array below. */ > - struct __printf_buffer base; > - > - /* Initial allocation. 200 should be large enough to copy almost > - all asprintf usages with just a single (final, correctly sized) > - heap allocation. */ > - char direct[PRINTF_BUFFER_SIZE_ASPRINTF]; > -}; > - > void > __printf_buffer_flush_asprintf (struct __printf_buffer_asprintf *buf) > { > @@ -90,23 +78,32 @@ __printf_buffer_flush_asprintf (struct __printf_buffer_asprintf *buf) > buf->base.write_end = new_buffer + new_size; > } > > +void > +__printf_buffer_asprintf_init (struct __printf_buffer_asprintf *buf) > +{ > + __printf_buffer_init (&buf->base, buf->direct, array_length (buf->direct), > + __printf_buffer_mode_asprintf); > +} > + > +int > +__printf_buffer_asprintf_free (struct __printf_buffer_asprintf *buf) > +{ > + if (buf->base.write_base != buf->direct) > + free (buf->base.write_base); > + return -1; > +} > > int > __vasprintf_internal (char **result_ptr, const char *format, va_list args, > unsigned int mode_flags) > { > struct __printf_buffer_asprintf buf; > - __printf_buffer_init (&buf.base, buf.direct, array_length (buf.direct), > - __printf_buffer_mode_asprintf); > + __printf_buffer_asprintf_init (&buf); > > __vprintf_buffer (&buf.base, format, args, mode_flags); > int done = __printf_buffer_done (&buf.base); > if (done < 0) > - { > - if (buf.base.write_base != buf.direct) > - free (buf.base.write_base); > - return done; > - } > + return __printf_buffer_asprintf_free (&buf); > > /* Transfer to the final buffer. */ > char *result; > @@ -122,10 +119,7 @@ __vasprintf_internal (char **result_ptr, const char *format, va_list args, > { > result = realloc (buf.base.write_base, size + 1); > if (result == NULL) > - { > - free (buf.base.write_base); > - return -1; > - } > + return __printf_buffer_asprintf_free (&buf); > } > > /* Add NUL termination. */
diff --git a/include/printf_buffer.h b/include/printf_buffer.h index fb0b42178e..cee5afb581 100644 --- a/include/printf_buffer.h +++ b/include/printf_buffer.h @@ -317,6 +317,31 @@ void __printf_buffer_snprintf_init (struct __printf_buffer_snprintf *, int __printf_buffer_snprintf_done (struct __printf_buffer_snprintf *) attribute_hidden; +/* Size of the initial on-stack buffer for asprintf. It should be + large enough to copy almost all asprintf usages with just a single + (final, correctly sized) heap allocation. */ +#define PRINTF_BUFFER_SIZE_ASPRINTF 200 + +struct __printf_buffer_asprintf +{ + /* base.write_base points either to a heap-allocated buffer, or to + the direct array below. */ + struct __printf_buffer base; + + /* Initial allocation. */ + char direct[PRINTF_BUFFER_SIZE_ASPRINTF]; +}; + +/* Sets up BUF for writing via __printf_buffer. */ +void __printf_buffer_asprintf_init (struct __printf_buffer_asprintf *buf) + attribute_hidden; + +/* Deallocates any allocated memory in *BUF. (This is not the usual + done routine for asprintf because that has to preserve allocation.) + Always returns -1 to indicate an error. */ +int __printf_buffer_asprintf_free (struct __printf_buffer_asprintf *buf) + attribute_hidden; + /* Flush function implementations follow. They are called from __printf_buffer_flush. Generic code should not call these flush functions directly. Some modes have inline implementations. */ @@ -363,11 +388,6 @@ void __wprintf_buffer_flush_to_file (struct __wprintf_buffer_to_file *) /* Temporary buffer used during floating point digit translation. */ #define PRINTF_BUFFER_SIZE_DIGITS 64 -/* Size of the initial on-stack buffer for asprintf. It should be - large enough to copy almost all asprintf usages with just a single - (final, correctly sized) heap allocation. */ -#define PRINTF_BUFFER_SIZE_ASPRINTF 200 - /* This should cover most of the packet-oriented file descriptors, where boundaries between writes could be visible to readers. But it is still small enough not to cause too many stack overflow issues. */ diff --git a/libio/vasprintf.c b/libio/vasprintf.c index edcfab2323..f8413eedfe 100644 --- a/libio/vasprintf.c +++ b/libio/vasprintf.c @@ -34,18 +34,6 @@ #include <string.h> #include <printf_buffer.h> -struct __printf_buffer_asprintf -{ - /* base.write_base points either to a heap-allocated buffer, or to - the direct array below. */ - struct __printf_buffer base; - - /* Initial allocation. 200 should be large enough to copy almost - all asprintf usages with just a single (final, correctly sized) - heap allocation. */ - char direct[PRINTF_BUFFER_SIZE_ASPRINTF]; -}; - void __printf_buffer_flush_asprintf (struct __printf_buffer_asprintf *buf) { @@ -90,23 +78,32 @@ __printf_buffer_flush_asprintf (struct __printf_buffer_asprintf *buf) buf->base.write_end = new_buffer + new_size; } +void +__printf_buffer_asprintf_init (struct __printf_buffer_asprintf *buf) +{ + __printf_buffer_init (&buf->base, buf->direct, array_length (buf->direct), + __printf_buffer_mode_asprintf); +} + +int +__printf_buffer_asprintf_free (struct __printf_buffer_asprintf *buf) +{ + if (buf->base.write_base != buf->direct) + free (buf->base.write_base); + return -1; +} int __vasprintf_internal (char **result_ptr, const char *format, va_list args, unsigned int mode_flags) { struct __printf_buffer_asprintf buf; - __printf_buffer_init (&buf.base, buf.direct, array_length (buf.direct), - __printf_buffer_mode_asprintf); + __printf_buffer_asprintf_init (&buf); __vprintf_buffer (&buf.base, format, args, mode_flags); int done = __printf_buffer_done (&buf.base); if (done < 0) - { - if (buf.base.write_base != buf.direct) - free (buf.base.write_base); - return done; - } + return __printf_buffer_asprintf_free (&buf); /* Transfer to the final buffer. */ char *result; @@ -122,10 +119,7 @@ __vasprintf_internal (char **result_ptr, const char *format, va_list args, { result = realloc (buf.base.write_base, size + 1); if (result == NULL) - { - free (buf.base.write_base); - return -1; - } + return __printf_buffer_asprintf_free (&buf); } /* Add NUL termination. */