On 06/18/2010 11:27 AM, Justin Clift wrote:
>>> + /* Retrieve the list of volume names in the pool */
>>> + if (numVolumes) {
>>> + activeNames = vshMalloc(ctl, sizeof(char *) * numVolumes);
>>
>> Wondering if VIR_ALLOC_N is better than vshMalloc here. Both styles
>> occur in virsh.c, so you're probably okay as-is.
>
> Cool, hadn't come across VIR_ALLOC_N before. You've probably picked up
> that I was using vshMalloc() having seen it used elsewhere in the virsh
> code before. :)
Outside of virsh, VIR_ALLOC_N is a winner, hands down. But inside
virsh.c, the vshMalloc wrapper reports a line number where any
allocation failure occurs, which might be considered useful in debugging
an over-allocation failure. And given that VIR_ALLOC_N is only used
once in virsh.c, consistency would argue for vshMalloc.
Anyone else have an opinion?
>
>>> + /* Create the output template */
>>> + char *outputStr;
>>> + virBuffer bufStr = VIR_BUFFER_INITIALIZER;
>>> + virBufferVSprintf(&bufStr, "%%-%us %%-%us %%-%us %%-%us
>>> %%-%us\n",
>>> + maxName, maxPath, maxType, maxCap, maxAlloc);
>>> + outputStr = virBufferContentAndReset(&bufStr);
>>
>> If you generate a printf format string like this, you need to check for
>> allocation failure. Otherwise, you could end up passing NULL as the
>> format string in the vshPrint below. Having said that...
>
> Good point. Hadn't thought of that, but it makes sense. :)
One other thing - rather than using virBuffer, with a single
virBufferVSprintf before you reset the buffer, it is faster to just use
virAsprintf (no need for an intermediate buffer unless you are
constructing a string in mutiple chunks).
--
Eric Blake eblake@xxxxxxxxxx +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list