On Thu, Dec 10, 2009 at 04:51:12PM -0500, Dave Allan wrote: > > > >ACK aside from the minor logging fixes& attribute whitespace > > > >Daniel > > Ok, thanks. Final version attached. > > Dave > [...] > + if ((address == NULL) || (logStrToLong_ui(address, > + &p, > + 16, > + &bdf->domain) == -1)) { > + goto out; > + } > + > + if ((p == NULL) || (logStrToLong_ui(p+1, > + &p, > + 16, > + &bdf->bus) == -1)) { > + goto out; > + } > + > + if ((p == NULL) || (logStrToLong_ui(p+1, > + &p, > + 16, > + &bdf->slot) == -1)) { > + goto out; > + } > + > + if ((p == NULL) || (logStrToLong_ui(p+1, > + &p, > + 16, > + &bdf->function) == -1)) { > + goto out; > + } I reformatted this a bit, no need to have one arg per line > +static int get_sriov_function(const char *device_link, > + struct pci_config_address **bdf) > +{ > + char *device_path = NULL, *config_address = NULL; > + char errbuf[64]; > + int ret = SRIOV_ERROR; [...] > + device_path = realpath(device_link, device_path); > + if (device_path == NULL) { > + memset(errbuf, '\0', sizeof(errbuf)); > + VIR_ERROR("Failed to resolve device link '%s': '%s'\n", device_link, > + virStrerror(errno, errbuf, sizeof(errbuf))); Hum I wonder if we don't have a wrapper error macro for this to use instead of allocating a small buffer on the stack, but it's minor. > + goto out; > + } [...] > + VIR_DEBUG("Number of virtual functions: %d\n", *num_funcs); > + if (VIR_REALLOC_N(d->pci_dev.virtual_functions, (*num_funcs) + 1) != 0) { I reformatted that line too > + virReportOOMError(NULL); > + goto out; > + } > + > + if (get_sriov_function(device_link, > + &d->pci_dev.virtual_functions[*num_funcs]) != > + SRIOV_FOUND) { there as well > + > + /* We should not get back SRIOV_NOT_FOUND in this > + * case, so if we do, it's an error. */ > + VIR_ERROR("Failed to get SR IOV function from device link '%s'\n", unneeded \n, fit in a line as a result > + device_link); > + goto out; > + } else { > + (*num_funcs)++; > + d->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION; > + } > + > + VIR_FREE(device_link); > + } > + } > + > + closedir(dir); > + > + ret = 0; > + > +out: > + VIR_FREE(device_link); > + return 0; > +} > + > #endif /* __linux__ */ With those tiny fixes, I phsed the patch, thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list