From afb571ba8fbc46cbe6c6d22421e32a13e66a6cbf Mon Sep 17 00:00:00 2001 From: Tilghman Lesher Date: Wed, 19 Nov 2008 01:02:45 +0000 Subject: [PATCH] Starting with a change to ensure that ast_verbose() preserves ABI compatibility in 1.6.1 (as compared to 1.6.0 and versions of 1.4), this change also deprecates the use of Asterisk with FreeBSD 4, given the central use of va_copy in core functions. va_copy() is C99, anyway, and we already require C99 for other purposes, so this isn't really a big change anyway. This change also simplifies some of the core ast_str_* functions. git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@157639 65c4cc65-6c06-0410-ace0-fbb531ad65f3 --- include/asterisk/logger.h | 4 ++ include/asterisk/strings.h | 106 ++++++++++++++++--------------------- main/logger.c | 24 +++++++-- main/utils.c | 64 ++++++++++++---------- 4 files changed, 106 insertions(+), 92 deletions(-) diff --git a/include/asterisk/logger.h b/include/asterisk/logger.h index 7d03d88940..91be731812 100644 --- a/include/asterisk/logger.h +++ b/include/asterisk/logger.h @@ -80,6 +80,10 @@ void __ast_verbose(const char *file, int line, const char *func, const char *fmt #define ast_verbose(...) __ast_verbose(__FILE__, __LINE__, __PRETTY_FUNCTION__, __VA_ARGS__) +void __ast_verbose_ap(const char *file, int line, const char *func, const char *fmt, va_list ap); + +#define ast_verbose_ap(fmt, ap) __ast_verbose_ap(__FILE__, __LINE__, __PRETTY_FUNCTION__, fmt, ap) + void ast_child_verbose(int level, const char *fmt, ...) __attribute__ ((format (printf, 2, 3))); diff --git a/include/asterisk/strings.h b/include/asterisk/strings.h index 142fc7c627..7c25ecd5e0 100644 --- a/include/asterisk/strings.h +++ b/include/asterisk/strings.h @@ -425,15 +425,17 @@ void ast_str_trim_blanks(struct ast_str *buf), AST_INLINE_API( int _ast_str_make_space(struct ast_str **buf, size_t new_len, const char *file, int lineno, const char *function), { - _DB1(struct ast_str *old_buf = *buf;) + struct ast_str *old_buf = *buf; if (new_len <= (*buf)->len) return 0; /* success */ if ((*buf)->ts == DS_ALLOCA || (*buf)->ts == DS_STATIC) return -1; /* cannot extend */ *buf = (struct ast_str *)__ast_realloc(*buf, new_len + sizeof(struct ast_str), file, lineno, function); - if (*buf == NULL) /* XXX watch out, we leak memory here */ + if (*buf == NULL) { + *buf = old_buf; return -1; + } if ((*buf)->ts != DS_MALLOC) { pthread_setspecific((*buf)->ts->key, *buf); _DB1(__ast_threadstorage_object_replace(old_buf, *buf, new_len + sizeof(struct ast_str));) @@ -448,15 +450,17 @@ int _ast_str_make_space(struct ast_str **buf, size_t new_len, const char *file, AST_INLINE_API( int ast_str_make_space(struct ast_str **buf, size_t new_len), { - _DB1(struct ast_str *old_buf = *buf;) + struct ast_str *old_buf = *buf; if (new_len <= (*buf)->len) return 0; /* success */ if ((*buf)->ts == DS_ALLOCA || (*buf)->ts == DS_STATIC) return -1; /* cannot extend */ *buf = (struct ast_str *)ast_realloc(*buf, new_len + sizeof(struct ast_str)); - if (*buf == NULL) /* XXX watch out, we leak memory here */ + if (*buf == NULL) { + *buf = old_buf; return -1; + } if ((*buf)->ts != DS_MALLOC) { pthread_setspecific((*buf)->ts->key, *buf); _DB1(__ast_threadstorage_object_replace(old_buf, *buf, new_len + sizeof(struct ast_str));) @@ -557,12 +561,8 @@ struct ast_str *__ast_str_thread_get(struct ast_threadstorage *ts, /*! * \brief Error codes from __ast_str_helper() * The undelying processing to manipulate dynamic string is done - * by __ast_str_helper(), which can return a success, a - * permanent failure (e.g. no memory), or a temporary one (when - * the string needs to be reallocated, and we must run va_start() - * again; XXX this convoluted interface is only here because - * FreeBSD 4 lacks va_copy, but this will be fixed and the - * interface simplified). + * by __ast_str_helper(), which can return a success or a + * permanent failure (e.g. no memory). */ enum { /*! An error has occurred and the contents of the dynamic string @@ -570,11 +570,36 @@ enum { AST_DYNSTR_BUILD_FAILED = -1, /*! The buffer size for the dynamic string had to be increased, and * __ast_str_helper() needs to be called again after - * a va_end() and va_start(). + * a va_end() and va_start(). This return value is legacy and will + * no longer be used. */ AST_DYNSTR_BUILD_RETRY = -2 }; +/*! + * \brief Core functionality of ast_str_(set|append)_va + * + * The arguments to this function are the same as those described for + * ast_str_set_va except for an addition argument, append. + * If append is non-zero, this will append to the current string instead of + * writing over it. + * + * AST_DYNSTR_BUILD_RETRY is a legacy define. It should probably never + * again be used. + * + * A return of AST_DYNSTR_BUILD_FAILED indicates a memory allocation error. + * + * A return value greater than or equal to zero indicates the number of + * characters that have been written, not including the terminating '\0'. + * In the append case, this only includes the number of characters appended. + * + * \note This function should never need to be called directly. It should + * through calling one of the other functions or macros defined in this + * file. + */ +int __ast_str_helper(struct ast_str **buf, size_t max_len, + int append, const char *fmt, va_list ap); + /*! * \brief Set a dynamic string from a va_list * @@ -612,62 +637,23 @@ enum { * ... * } * \endcode - * - * \note: the following two functions must be implemented as macros - * because we must do va_end()/va_start() on the original arguments. */ -#define ast_str_set_va(buf, max_len, fmt, ap) \ - ({ \ - int __res; \ - while ((__res = __ast_str_helper(buf, max_len, \ - 0, fmt, ap)) == AST_DYNSTR_BUILD_RETRY) { \ - va_end(ap); \ - va_start(ap, fmt); \ - } \ - (__res); \ - }) +AST_INLINE_API(int ast_str_set_va(struct ast_str **buf, size_t max_len, const char *fmt, va_list ap), +{ + return __ast_str_helper(buf, max_len, 0, fmt, ap); +} +) /*! * \brief Append to a dynamic string using a va_list * * Same as ast_str_set_va(), but append to the current content. */ -#define ast_str_append_va(buf, max_len, fmt, ap) \ - ({ \ - int __res; \ - while ((__res = __ast_str_helper(buf, max_len, \ - 1, fmt, ap)) == AST_DYNSTR_BUILD_RETRY) { \ - va_end(ap); \ - va_start(ap, fmt); \ - } \ - (__res); \ - }) - -/*! - * \brief Core functionality of ast_str_(set|append)_va - * - * The arguments to this function are the same as those described for - * ast_str_set_va except for an addition argument, append. - * If append is non-zero, this will append to the current string instead of - * writing over it. - * - * In the case that this function is called and the buffer was not large enough - * to hold the result, the partial write will be truncated, and the result - * AST_DYNSTR_BUILD_RETRY will be returned to indicate that the buffer size - * was increased, and the function should be called a second time. - * - * A return of AST_DYNSTR_BUILD_FAILED indicates a memory allocation error. - * - * A return value greater than or equal to zero indicates the number of - * characters that have been written, not including the terminating '\0'. - * In the append case, this only includes the number of characters appended. - * - * \note This function should never need to be called directly. It should - * through calling one of the other functions or macros defined in this - * file. - */ -int __ast_str_helper(struct ast_str **buf, size_t max_len, - int append, const char *fmt, va_list ap); +AST_INLINE_API(int ast_str_append_va(struct ast_str **buf, size_t max_len, const char *fmt, va_list ap), +{ + return __ast_str_helper(buf, max_len, 1, fmt, ap); +} +) /*! * \brief Set a dynamic string using variable arguments diff --git a/main/logger.c b/main/logger.c index 4e3a2b73ca..b4f4c4dd0c 100644 --- a/main/logger.c +++ b/main/logger.c @@ -1232,12 +1232,11 @@ void ast_backtrace(void) #endif } -void __ast_verbose(const char *file, int line, const char *func, const char *fmt, ...) +void __ast_verbose_ap(const char *file, int line, const char *func, const char *fmt, va_list ap) { struct logmsg *logmsg = NULL; struct ast_str *buf = NULL; int res = 0; - va_list ap; if (!(buf = ast_str_thread_get(&verbose_buf, VERBOSE_BUF_INIT_SIZE))) return; @@ -1261,9 +1260,7 @@ void __ast_verbose(const char *file, int line, const char *func, const char *fmt } /* Build string */ - va_start(ap, fmt); res = ast_str_set_va(&buf, 0, fmt, ap); - va_end(ap); /* If the build failed then we can drop this allocated message */ if (res == AST_DYNSTR_BUILD_FAILED) @@ -1291,6 +1288,25 @@ void __ast_verbose(const char *file, int line, const char *func, const char *fmt } } +void __ast_verbose(const char *file, int line, const char *func, const char *fmt, ...) +{ + va_list ap; + va_start(ap, fmt); + __ast_verbose_ap(file, line, func, fmt, ap); + va_end(ap); +} + +/* No new code should use this directly, but we have the ABI for backwards compat */ +#undef ast_verbose +void ast_verbose(const char *fmt, ...) __attribute__ ((format (printf, 1, 2))); +void ast_verbose(const char *fmt, ...) +{ + va_list ap; + va_start(ap, fmt); + __ast_verbose_ap("", 0, "", fmt, ap); + va_end(ap); +} + int ast_register_verbose(void (*v)(const char *string)) { struct verb *verb; diff --git a/main/utils.c b/main/utils.c index 9a7a3acd45..0101d93ed0 100644 --- a/main/utils.c +++ b/main/utils.c @@ -1667,37 +1667,45 @@ int __ast_str_helper(struct ast_str **buf, size_t max_len, { int res, need; int offset = (append && (*buf)->len) ? (*buf)->used : 0; + va_list aq; - if (max_len < 0) - max_len = (*buf)->len; /* don't exceed the allocated space */ - /* - * Ask vsnprintf how much space we need. Remember that vsnprintf - * does not count the final '\0' so we must add 1. - */ - res = vsnprintf((*buf)->str + offset, (*buf)->len - offset, fmt, ap); - - need = res + offset + 1; - /* - * If there is not enough space and we are below the max length, - * reallocate the buffer and return a message telling to retry. - */ - if (need > (*buf)->len && (max_len == 0 || (*buf)->len < max_len) ) { - if (max_len && max_len < need) /* truncate as needed */ - need = max_len; - else if (max_len == 0) /* if unbounded, give more room for next time */ - need += 16 + need/4; - if (0) /* debugging */ - ast_verbose("extend from %d to %d\n", (int)(*buf)->len, need); - if (ast_str_make_space(buf, need)) { - ast_verbose("failed to extend from %d to %d\n", (int)(*buf)->len, need); - return AST_DYNSTR_BUILD_FAILED; + do { + if (max_len < 0) { + max_len = (*buf)->len; /* don't exceed the allocated space */ } - (*buf)->str[offset] = '\0'; /* Truncate the partial write. */ + /* + * Ask vsnprintf how much space we need. Remember that vsnprintf + * does not count the final '\0' so we must add 1. + */ + va_copy(aq, ap); + res = vsnprintf((*buf)->str + offset, (*buf)->len - offset, fmt, aq); - /* va_end() and va_start() must be done before calling - * vsnprintf() again. */ - return AST_DYNSTR_BUILD_RETRY; - } + need = res + offset + 1; + /* + * If there is not enough space and we are below the max length, + * reallocate the buffer and return a message telling to retry. + */ + if (need > (*buf)->len && (max_len == 0 || (*buf)->len < max_len) ) { + if (max_len && max_len < need) { /* truncate as needed */ + need = max_len; + } else if (max_len == 0) { /* if unbounded, give more room for next time */ + need += 16 + need / 4; + } + if (0) { /* debugging */ + ast_verbose("extend from %d to %d\n", (int)(*buf)->len, need); + } + if (ast_str_make_space(buf, need)) { + ast_verbose("failed to extend from %d to %d\n", (int)(*buf)->len, need); + return AST_DYNSTR_BUILD_FAILED; + } + (*buf)->str[offset] = '\0'; /* Truncate the partial write. */ + + /* Restart va_copy before calling vsnprintf() again. */ + va_end(aq); + continue; + } + break; + } while (1); /* update space used, keep in mind the truncation */ (*buf)->used = (res + offset > (*buf)->len) ? (*buf)->len : res + offset;