[Libvir] Re: [PATCH] allocate virBuffer* routines

[Karel Zak]

On Tue, Apr 24, 2007 at 09:48:09PM +0900, S.Sakamoto wrote:
> diff -u -p -r1.110 xend_internal.c
> --- src/xend_internal.c	23 Apr 2007 07:41:23 -0000	1.110
> +++ src/xend_internal.c	24 Apr 2007 11:00:55 -0000
> @@ -587,24 +587,33 @@ static int
>  xend_op_ext2(virConnectPtr xend, const char *path, char *error,
>               size_t n_error, const char *key, va_list ap)
>  {
> -    char ops[VIR_XML_STRING_BUFLEN];
>      const char *k = key, *v;
> -    int offset = 0;
> +    virBuffer buf;
> +    int ret;
> +
> +    buf.content = malloc(1000);
> +    if (buf.content == NULL)
> +        return -1;
> +    buf.size = 1000;
> +    buf.use = 0;

 Why not virBufferNew(1000) ? 

 Is it really advantage that you save one extra malloc() for the buf
 pointer? I don't think so -- especially if you need to manually set
 internal virBuffer stuff.

>      while (k) {
>          v = va_arg(ap, const char *);
>  
> -        offset += snprintf(ops + offset, sizeof(ops) - offset, "%s", k);
> -        offset += snprintf(ops + offset, sizeof(ops) - offset, "%s", "=");
> -        offset += snprintf(ops + offset, sizeof(ops) - offset, "%s", v);
> +        virBufferVSprintf(&buf, "%s", k);
> +        virBufferVSprintf(&buf, "%s", "=");
> +        virBufferVSprintf(&buf, "%s", v);

 Please, use virBufferStrcat or virBufferAdd if you needn't a string
 formatting. It's cheaper and faster.

           virBufferStrcat(&buf, k, "=", v, NULL);

> -            offset += snprintf(ops + offset,
> -                               sizeof(ops) - offset, "%s", "&");
> +            virBufferVSprintf(&buf, "%s", "&");

           virBufferAdd(&buf, "&", 1);


    Pedantic Karel :-)

-- 
 Karel Zak  <kzak at redhat.com>
 
 Red Hat Czech s.r.o.
 Purkynova 99/71, 612 45 Brno, Czech Republic
 Reg.id: CZ27690016