Looks great, but one minor comment below....
On Tue, 2009-11-24 at 00:37 +0100, Andreas Fritiofson wrote:
> The previous implementation was unnecessarily complex. Get rid of the loops,
> let vsnprintf() tell us directly how much storage we need and allocate that. A
> second pass writes the actual string. Also add a va_end() that was missing.
> This should be much faster for large strings and less wasteful for small ones.
>
> A quirk that has been retained is that some callers patch in a newline at the
> end of the returned string and depend on alloc_vprintf to allocate at least
> one byte extra.
>
> Signed-off-by: Andreas Fritiofson <andreas.fritiof...@gmail.com>
> ---
> src/helper/log.c | 43 ++++++++++++++++---------------------------
> 1 files changed, 16 insertions(+), 27 deletions(-)
>
> diff --git a/src/helper/log.c b/src/helper/log.c
> index caaed42..6869d2e 100644
> --- a/src/helper/log.c
> +++ b/src/helper/log.c
> @@ -395,37 +395,26 @@ int log_remove_callback(log_callback_fn fn, void *priv)
> /* return allocated string w/printf() result */
> char *alloc_vprintf(const char *fmt, va_list ap)
> {
> - /* no buffer at the beginning, force realloc to do the job */
> - char *string = NULL;
> -
> - /* start with buffer size suitable for typical messages */
> - int size = 128;
> -
> - for (;;)
> - {
> - char *t = string;
> - va_list ap_copy;
> - int ret;
> - string = realloc(string, size);
> - if (string == NULL)
> - {
> - if (t != NULL)
> - free(t);
> - return NULL;
> - }
> + va_list ap_copy;
> + int len;
> + char *string;
>
> - va_copy(ap_copy, ap);
> + /* determine the length of the buffer needed */
> + va_copy(ap_copy, ap);
> + len = vsnprintf(NULL, 0, fmt, ap_copy);
> + va_end(ap_copy);
>
> - ret = vsnprintf(string, size, fmt, ap_copy);
> - /* NB! The result of the vsnprintf() might be an *EMPTY*
> string! */
> - if ((ret >= 0) && ((ret + 1) < size))
> - break;
> + /* allocate and make room for terminating zero. */
> + /* FIXME: The old version always allocated at least one byte extra and
> + * other code depend on that. They should be probably be fixed, but for
> + * now reserve the extra byte. */
> + string = malloc(len + 2);
> + if (string == NULL)
> + return NULL;
>
> - /* there was just enough or not enough space, allocate more in
> the next round */
> - size *= 2; /* double the buffer size */
> - }
> + /* do the real work */
> + vsnprintf(string, len + 1, fmt, ap_copy);
I think you meant for this to be ap, not ap_copy.
> - /* the returned buffer is by principle guaranteed to be at least one
> character longer */
> return string;
> }
>
Fix this minor detail, and I'll commit both of your patches.
--Z
_______________________________________________
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development
