[libvirt] [PATCHv2 2/2] virsh: add new --details option to vol-list

[Eric Blake]

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 at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20100618/5942770e/attachment-0001.sig>