From 92c220edce6b8d7de364ca54dd54465db83a2828 Mon Sep 17 00:00:00 2001 From: Marko Lindqvist Date: Sun, 23 Oct 2022 03:02:37 +0300 Subject: [PATCH 26/26] 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 + m4/c99.m4 | 19 +++++++++++++++- utility/astring.c | 58 +++++++++++++++++++++++++++++++++++++---------- 3 files changed, 65 insertions(+), 13 deletions(-) diff --git a/configure.ac b/configure.ac index 3e91fb6f3f..5dd1d05a2a 100644 --- a/configure.ac +++ b/configure.ac @@ -633,6 +633,7 @@ FC_C99_VARIADIC_MACROS FC_C99_VARIABLE_ARRAYS FC_C99_INITIALIZERS FC_C99_STDINT_H +FC_C99_VA_COPY FC_INITIALIZER_BRACES diff --git a/m4/c99.m4 b/m4/c99.m4 index 2434500831..08fc722ef6 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?). @@ -103,3 +103,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/utility/astring.c b/utility/astring.c index 3864e6efeb..cac376f242 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