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@xxxxxxxxxx | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/