Re: [PATCH] Convert hvmloader sprintf() into snprintf()

[David Vrabel <david.vrabel@citrix.com>]

On 29/11/11 13:42, Paul Durrant wrote:
> # HG changeset patch
> # User Paul Durrant <paul.durrant@citrix.com>
> # Date 1322574114 0
> # Node ID e1e952982cf1d7a0c38a7822a8b5e78ba04b5ba5
> # Parent  225da1242ba979ddc8c48767d3822e0c8d274ae1
> Convert hvmloader sprintf() into snprintf().
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> 
> diff -r 225da1242ba9 -r e1e952982cf1 tools/firmware/hvmloader/acpi/build.c
> --- a/tools/firmware/hvmloader/acpi/build.c	Tue Nov 29 10:48:54 2011 +0000
> +++ b/tools/firmware/hvmloader/acpi/build.c	Tue Nov 29 13:41:54 2011 +0000
> @@ -306,7 +306,8 @@ unsigned long new_vm_gid(void)
>      buf = mem_alloc(8, 8);
>      if (!buf) return 0;
>  
> -    sprintf(addr, "0x%lx", virt_to_phys(buf));
> +    if (snprintf(addr, 11, "0x%lx", virt_to_phys(buf)) >= 11) return 0;
> +
>      xenstore_write("data/generation-id", addr);
>  
>      gid = strtoll(xenstore_read("platform/generation-id", "0"), NULL, 0);
> diff -r 225da1242ba9 -r e1e952982cf1 tools/firmware/hvmloader/util.c
> --- a/tools/firmware/hvmloader/util.c	Tue Nov 29 10:48:54 2011 +0000
> +++ b/tools/firmware/hvmloader/util.c	Tue Nov 29 13:41:54 2011 +0000
> @@ -528,7 +528,7 @@ static char *printnum(char *p, unsigned 
>      return p;
>  }
>  
> -static void _doprint(void (*emit)(char**, char), char **arg, const char *fmt, va_list ap)
> +static void _doprint(void (*emit)(void *, char), void *arg, const char *fmt, va_list ap)
>  {
>      char *str, c;
>      int lflag, zflag, nflag;
> @@ -626,7 +626,7 @@ static void putchar(char c)
>      outb(0xe9, c);
>  }
>  
> -static void __put(char **ignore, char c)
> +static void __put(void *arg, char c)
>  {
>      putchar(c);
>  }
> @@ -648,22 +648,42 @@ int vprintf(const char *fmt, va_list ap)
>      return 0;
>  }
>  
> -static void __copy(char **buf, char c)
> +struct __copy_context {
> +    char *ptr;
> +    size_t emitted;
> +    size_t remaining;
> +};
> +
> +static void __copy(void *arg, char c)
>  {
> -    **buf = c;
> -    (*buf)++;
> +    struct __copy_context *ctxt = arg;
> +
> +    ctxt->emitted++;
> +
> +    if (ctxt->remaining == 0)
> +        return;
> +    
> +    *(ctxt->ptr++) = c;
> +    --ctxt->remaining;
>  }
>  
> -int sprintf(char *buf, const char *fmt, ...)
> +int snprintf(char *buf, size_t size, const char *fmt, ...)
>  {
>      va_list ap;
> +    struct __copy_context ctxt;
> +
> +    ctxt.ptr = buf;
> +    ctxt.emitted = 0;
> +    ctxt.remaining = size;
>  
>      va_start(ap, fmt);
> -    _doprint(__copy, &buf, fmt, ap);
> +    _doprint(__copy, &ctxt, fmt, ap);
>      va_end(ap);
>  
> -    *buf = '\0';
> -    return 0;
> +    if (ctxt.remaining != 0)
> +        *ctxt.ptr = '\0';
> +
> +    return ctxt.emitted;
>  }

This doesn't return the correct value according the C99.  From the
snprintf() man page:

"The functions snprintf() and vsnprintf() do not write  more  than  size
 bytes  (including  the trailing '\0').  If the output was truncated due
 to this limit then the return value is the number  of  characters  (not
 including the trailing '\0') which would have been written to the final
 string if enough space had been available.  Thus,  a  return  value  of
 size  or  more  means  that  the output was truncated."

David