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

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