2010/3/30 Eduardo Otubo <otubo@xxxxxxxxxxxxxxxxxx>:
>
> 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list