[07/11] libio: Extract __printf_buffer_asprintf_init from asprintf implementation
Checks
Context |
Check |
Description |
redhat-pt-bot/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_glibc_build--master-arm |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_glibc_check--master-arm |
success
|
Testing passed
|
Commit Message
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.
---
include/printf_buffer.h | 30 +++++++++++++++++++++++++-----
libio/vasprintf.c | 40 +++++++++++++++++-----------------------
2 files changed, 42 insertions(+), 28 deletions(-)
Comments
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. */
@@ -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. */
@@ -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. */