On Fri, Jun 22, 2007 at 02:56:58AM +0100, Daniel P. Berrange wrote:
This is the 2nd key patch in the series. It switches the qemud/driver.c file methods to all follow the official libvirt internal driver API. NB, since we're using dummy statically declared virConnect object in the dispatch.c file, we can't use the regular virGetDomain function for constructing virDomainPtr objects at this time. Thus we have a hack to just malloc a virDomain struct & manually fill in the fields. This hack will go away in a later patch.
Okay, just two nitpicks, one about lack of comments, okay those functions are basically described as part of the driver API so it's a bit like cut and paste, but I still think it's nicer to have next to the code, and some lines are really long,
- for (i = 0 ; i < QEMUD_MAX_NUM_NETWORKS ; i++) { - names[i] = &out->qemud_packet_server_data_u.listNetworksReply.networks[i*QEMUD_MAX_NAME_LEN]; - }
There is a lot of place like that where the code manipulates all the dereference chain a lot of time, why not use temporary variables to make the code more readable and help the compiler w.r.t. generating good code ? +1 but maybe we need to revisit a bit that code. Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/