[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 7/7] list: Use virConnectListAllNodeDevices in virsh



On 09/06/2012 04:05 PM, Peter Krempa wrote:

>> +    if (cap_str) {
>> +        ncaps = vshStringToArray((char *)cap_str, &caps);
> 
> You shouldn't modify const strings.

Indeed; that argues that your earlier patch for vshStringToArray should
be modified to take 'const char *' as the string to split, and that
internally, it should strdup() if it is going to modify in-place, rather
than requiring the caller to pass in a modifiable string.  It would also
be possible to write vshStringToArray without modifying the input string
in place, but probably with a bit more effort (you'd have to use
strndup() of the individual array elements, and if you teach it how to
do ',,' escaped comma parsing, it gets even trickier).


>> virNodeDeviceLookupByName(ctl->conn, devices[i]);
>> -            if (dev && STRNEQ(devices[i], "computer")) {
>> +        for (i = 0; i < list->ndevices; i++) {
>> +            virNodeDevicePtr dev = list->devices[i];
>> +            if (STRNEQ(names[i], "computer")) {
> 
> You need to modify this condition to fix the bug with missing output
> when you specify filters. You will have to abuse
> virNodeDeviceGetParent() that returns NULL if it has no parent. This
> will probably induce a situation where multiple parents will be printed
> but that's right in this situation.

Or, if you take my hint from 6/7 and merely reject --tree and --caps as
incompatible options, then you don't have to worry about this part of
the code changing.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]