bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: extend the scope of xasprintf


From: Eric Blake
Subject: Re: extend the scope of xasprintf
Date: Mon, 8 May 2006 23:53:43 +0000 (UTC)
User-agent: Loom/3.14 (http://gmane.org/)

Bruno Haible <bruno <at> clisp.org> writes:

> 
> But the third case is frequent, and I wouldn't like it to have a big speed
> penalty (due to format string parsing and use of sprintf). Therefore I propose
> to put this special case optimization into gnulib's xasprintf.
> 
> OK to commit (and update the dependencies of the xvasprintf module to include
> 'stdarg' and 'xsize')?

Looks reasonable to me.  However, some nits below...

> + static inline char *
> + xstrcat (size_t argcount, va_list args)
...
> + 
> +   /* Allocate and fill the result string.  */
> +   result = (char *) xmalloc (totalsize + 1);

What if totalsize is INT_MAX?

> +   p = result;
> +   va_copy (ap, args);

Why are you using va_copy a second time?  vsprintf is allowed to consume its
va_list argument (aka use va_arg on it), so long as it leaves the va_close for
the caller.

> +   for (i = argcount; i > 0; i--)
> +     {
> +       const char *next = va_arg (ap, const char *);
> +       size_t len = strlen (next);
> +       memcpy (p, next, len);
> +       p += len;

This has now traversed each string three times (two strlen, one memcpy).  Would
something like strpcpy be more efficient, to cut out one of the traversals by
looking for string end while copying?

Also, should we worry about a multithreaded environment, if another thread
changes the string length between when you malloc'd the buffer and copy the
contents into the buffer?

> +     }
> +   *p = '\0';
> + 
> +   return result;
> + }
> + 
>   char *
>   xvasprintf (const char *format, va_list args)
>   {

Should we be robust to a NULL format?

>     char *result;
> 
> +   /* Recognize the special case format = "%s...%s".  It is a frequently used
> +      idiom for string concatenation and needs to be fast.  We don't want to
> +      have a separate function xstrcat() for this purpose.  */

Is it worth recognizing the special case of a format string with no "%"
formatting directives, and do the equivalent of strdup(format) in that case?

-- 
Eric Blake






reply via email to

[Prev in Thread] Current Thread [Next in Thread]