On 06/11/2012 07:13 AM, Peter Krempa wrote:
On 06/09/12 06:34, Eric Blake wrote:
Requiring the user to pass in parallel arrays of names and parents is annoying; it means that you can't qsort one of the arrays without invalidating the ordering of the other. By refactoring this function to use callbacks, we isolate the layout to be independent of the printing, and a future patch can exploit that to improve layout.
* tools/virsh.c (vshTreePrintInternal): Use callbacks rather than requiring a char** array. (vshTreeArrayLookup): New helper function. (vshTreePrint, cmdNodeListDevices, cmdSnapshotList): Update callers.
+static const char * +vshTreeArrayLookup(int devid, bool parent, void *opaque) +{ + struct vshTreeArray *arrays = opaque; + if (parent) + return arrays->parents[devid]; + return arrays->names[devid]; +} +
This is used just for listing node devices IIUC. Wouln't it be better to explicitly name the helper and struct that it's meant for node devices? vshNodeDeviceTree ... ?
In _this_ patch, it's used by both node devices and snapshots. Later, in patch 6/6, I switch snapshots over to using a different callback. Maybe I should rename this function as part of that later split into two different callbacks, but for this patch, since it really is used twice, I think it makes sense as-is.
ACK. The name doesn't matter that much, making the tree printer more universal does.
Peter
-- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org