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

Daniel Veillard veillard at redhat.com
Wed May 2 10:09:19 UTC 2007


On Mon, Apr 30, 2007 at 10:02:36AM +0200, Karel Zak wrote:
> 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.

  Hum, compared to Xend latency we probably should not care :-)

> >      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 :-)

  Care enough to send a patch from SVN head ;-) ?

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard      | virtualization library  http://libvirt.org/
veillard at redhat.com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/




More information about the libvir-list mailing list