[PATCH:xrdb 2/3] Replace complex malloc calculations with asprintf()

[Guillem Jover]

Hi!

On Tue, 2011-01-04 at 20:31:41 -0800, Alan Coopersmith wrote:
> diff --git a/xrdb.c b/xrdb.c
> index 33547f8..1f21d82 100644
> --- a/xrdb.c
> +++ b/xrdb.c
> @@ -148,6 +148,42 @@ static void Process ( int scrno, Bool doScreen, Bool execute );

> +#ifndef HAVE_ASPRINTF
> +/* sprintf variant found in newer libc's which allocates string to print to */
> +static int _X_ATTRIBUTE_PRINTF(2,3)
> +asprintf(char ** ret, const char *format, ...)
> +{
> +    char buf[256];
> +    int len;
> +    va_list ap;
> +
> +    va_start(ap, format);
> +    len = vsnprintf(buf, sizeof(buf), format, ap);
> +    va_end(ap);

len should be checked in case vsnprintf failed.

> +    len += 1; /* snprintf doesn't count trailing '\0' */

I'd avoid incrementing it, as it produces code a bit harder to follow,
and possible bugs like the one below.

> +    if (len <= sizeof(buf))
> +    {
> +	*ret = strdup(buf);
> +    }
> +    else
> +    {
> +	*ret = malloc(len);
> +	if (*ret != NULL)
> +	{
> +	    va_start(ap, format);
> +	    len = vsnprintf(*ret, len, format, ap);

This will make the return value be 1 less than written. And len should
be checked here too, in case this call failed, which could happen due to
memory exhaustion for example (depending on the implementation).

> +	    va_end(ap);
> +	}
> +    }
> +
> +    if (*ret == NULL)
> +	return -1;
> +
> +    return (len - 1);
> +}
> +#endif /* HAVE_ASPRINTF */

regards,
guillem