From c638a9970d9392ecd9f31a810d07c07c71f8c74a Mon Sep 17 00:00:00 2001 From: Marko Lindqvist Date: Sun, 23 Oct 2022 02:57:49 +0300 Subject: [PATCH 50/50] Reimplement astr_vadd_at() - Make it to use va_copy() instead of trying to reuse same list when it needs to call fc_vsnprintf() multiple times - astr_buffer_grow() interface changed so that we get the required size at once, no need to grow buffer in a loop - Buffer size is no longer power of two. I don't think that property really gave anything in the case of a single global buffer living over entire lifetime of the process - Now that buffer growing actully works, made the initial buffer a bit smaller Problems originally reported by louis94 and Daavko See osdn #45903 Signed-off-by: Marko Lindqvist --- configure.ac | 1 + gen_headers/meson_fc_config.h.in | 3 ++ m4/c99.m4 | 19 ++++++++++- meson.build | 3 +- utility/astring.c | 58 +++++++++++++++++++++++++------- 5 files changed, 70 insertions(+), 14 deletions(-) diff --git a/configure.ac b/configure.ac index 9f1a421196..8f8d6610a7 100644 --- a/configure.ac +++ b/configure.ac @@ -766,6 +766,7 @@ FC_C99_VARIABLE_ARRAYS FC_C99_INITIALIZERS FC_C99_COMPOUND_LITERALS FC_C99_STDINT_H +FC_C99_VA_COPY FC_INITIALIZER_BRACES diff --git a/gen_headers/meson_fc_config.h.in b/gen_headers/meson_fc_config.h.in index 5e28d2ff91..e0b309e5da 100644 --- a/gen_headers/meson_fc_config.h.in +++ b/gen_headers/meson_fc_config.h.in @@ -358,6 +358,9 @@ /* usleep() available */ #mesondefine HAVE_USLEEP +/* Have C99 va_copy() */ +#mesondefine HAVE_VA_COPY + /* vprintf() available */ #mesondefine HAVE_VPRINTF diff --git a/m4/c99.m4 b/m4/c99.m4 index 235d2653f9..4db46e09e6 100644 --- a/m4/c99.m4 +++ b/m4/c99.m4 @@ -1,4 +1,4 @@ -# Check for the presence of C99 features. Generally the check will fail +# Check for the presence of C99 features. Generally the check will fail # if the feature isn't present (a C99 compiler isn't that much to ask, # right?). @@ -123,3 +123,20 @@ AC_CACHE_CHECK([whether preprocessor token concenation works], AC_MSG_ERROR([A preprocessor supporting token concenation is required]) fi ]) + +AC_DEFUN([FC_C99_VA_COPY], +[ +dnl Check for C99 va_copy() +AC_CACHE_CHECK([for C99 va_copy], + [ac_cv_c99_va_copy], + [AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include ]], + [[va_list orig; + va_list copy; + va_copy(copy, orig);]])], + [ac_cv_c99_va_copy=yes], [ac_cv_c99_va_copy=no])]) + if test "x${ac_cv_c99_va_copy}" = "xyes" ; then + AC_DEFINE([HAVE_VA_COPY], [1], [va_copy() available]) + else + AC_MSG_WARN([lack of va_copy() support is going to be mandatory soon]) + fi +]) diff --git a/meson.build b/meson.build index 1955fc2d95..f7fe50bf6f 100644 --- a/meson.build +++ b/meson.build @@ -308,7 +308,8 @@ priv_functions = [ '_stricoll', 'fcntl', 'ioctl', - 'vsnprintf' + 'vsnprintf', + 'va_copy' ] foreach func : priv_functions diff --git a/utility/astring.c b/utility/astring.c index 1efceb7f35..25f516d20e 100644 --- a/utility/astring.c +++ b/utility/astring.c @@ -81,17 +81,22 @@ static char *astr_buffer = NULL; static size_t astr_buffer_alloc = 0; static inline char *astr_buffer_get(size_t *alloc); -static inline char *astr_buffer_grow(size_t *alloc); +static inline char *astr_buffer_grow(size_t request, size_t *alloc); static void astr_buffer_free(void); - /************************************************************************//** Returns the astring buffer. Create it if necessary. ****************************************************************************/ static inline char *astr_buffer_get(size_t *alloc) { if (!astr_buffer) { +#ifndef HAVE_VA_COPY + /* This buffer will never be grown, so it should be big enough + * from the beginning. */ astr_buffer_alloc = 65536; +#else + astr_buffer_alloc = 4096; +#endif astr_buffer = fc_malloc(astr_buffer_alloc); atexit(astr_buffer_free); } @@ -103,9 +108,18 @@ static inline char *astr_buffer_get(size_t *alloc) /************************************************************************//** Grow the astring buffer. ****************************************************************************/ -static inline char *astr_buffer_grow(size_t *alloc) +static inline char *astr_buffer_grow(size_t request, size_t *alloc) { - astr_buffer_alloc *= 2; + if (request > astr_buffer_alloc) { + /* We simply set buffer size to the requested one here. + * Old implementation went on by doubling the buffer size, + * presumably to match what low level memory handling does + * anyway. But need to increase the buffer size is so rare + * that we can as well call this function again, even when + * the increase would fall within limits of what doubling + * would have given us. */ + astr_buffer_alloc = request; + } astr_buffer = fc_realloc(astr_buffer, astr_buffer_alloc); *alloc = astr_buffer_alloc; @@ -202,20 +216,40 @@ static inline void astr_vadd_at(struct astring *astr, size_t at, { char *buffer; size_t buffer_size; - size_t new_len; + size_t req_len; + +#ifdef HAVE_VA_COPY + va_list copy; buffer = astr_buffer_get(&buffer_size); - for (;;) { - new_len = fc_vsnprintf(buffer, buffer_size, format, ap); - if (new_len < buffer_size && (size_t) -1 != new_len) { - break; + + va_copy(copy, ap); + + req_len = fc_vsnprintf(buffer, buffer_size, format, ap); + if (req_len > buffer_size) { + buffer = astr_buffer_grow(req_len, &buffer_size); + /* Even if buffer is *still* too small, we fill what we can */ + req_len = fc_vsnprintf(buffer, buffer_size, format, copy); + if (req_len > buffer_size) { + /* What we actually got */ + req_len = buffer_size; } - buffer = astr_buffer_grow(&buffer_size); } + va_end(copy); +#else /* HAVE_VA_COPY */ + buffer = astr_buffer_get(&buffer_size); + + req_len = fc_vsnprintf(buffer, buffer_size, format, ap); + + if (req_len > buffer_size) { + /* What we actually got */ + req_len = buffer_size; + } +#endif /* HAVE_VA_COPY */ - new_len += at + 1; + req_len += at + 1; - astr_reserve(astr, new_len); + astr_reserve(astr, req_len); fc_strlcpy(astr->str + at, buffer, astr->n_alloc - at); } -- 2.35.1