2010/3/30 Eduardo Otubo <otubo@linux.vnet.ibm.com>:
I am really sorry. I sent an older version of the patch, here is the correct one.
Eric Blake wrote:
On 03/24/2010 11:19 AM, Eduardo Otubo wrote:
/* exit early if there are no domains */ - if (nids == 0) + if (nids_numdomains == 0 || nids_listdomains == 0 + || nids_numdomains != nids_listdomains) return 0;
Should we be reporting an error here...
- if (VIR_ALLOC_N(ids, nids) < 0) { + if (VIR_ALLOC_N(ids, nids_listdomains) < 0) { virReportOOMError(); goto err; } - if ((nids = phypListDomainsGeneric(conn, ids, nids, 1)) < 0) - goto err; - - /* exit early if there are no domains */ - /* FIXME: phypNumDomainsGeneric() returned > 0 but phypListDomainsGeneric() - * returned 0. indicates this an error condition? - * an even stricter check would be to treat - * - * phypNumDomainsGeneric() != phypListDomainsGeneric() - * - * as an error */
...given this comment? You treated it as an early exit of 0, even if both values were non-zero.
Sorry for the late review...
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 4f7efdb..eb6be32 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -1849,41 +1849,32 @@ phypUUIDTable_Init(virConnectPtr conn) { uuid_tablePtr uuid_table; phyp_driverPtr phyp_driver; - int nids = 0; + int nids_numdomains = 0; + int nids_listdomains = 0; int *ids = NULL; unsigned int i = 0;
- if ((nids = phypNumDomainsGeneric(conn, 2)) < 0) + if ((nids_numdomains = phypNumDomainsGeneric(conn, 2)) < 0) + goto err; + + if ((nids_listdomains = + phypListDomainsGeneric(conn, ids, nids_listdomains, 1)) < 0)
I think you want to call phypListDomainsGeneric(conn, ids, nids_numdomains, 1) instead of phypListDomainsGeneric(conn, ids, nids_listdomains, 1) Also, you have to move the allocation of ids before the call to phypListDomainsGeneric. Otherwise you're calling phypListDomainsGeneric with ids = NULL resulting in a segfault. Because you currently pass nids_listdomains (which is always 0 at this point) to phypListDomainsGeneric you don't trigger the segfault.
goto err;
/* exit early if there are no domains */ - if (nids == 0) + if (nids_numdomains != nids_listdomains)
You should report an error here, otherwise an "unknown error" will be reported by libvirt.
+ goto err; + else if (nids_numdomains == 0 && nids_listdomains == 0) return 0;
Matthias