Dan Smith wrote:
KR> ret = _get_dominfo(xml, *dominfo); KR> + if (!ret) { KR> + free(*dominfo); KR> + *dominfo = NULL; KR> + return 0; KR> + } KR> + KR> free(xml);
You've just introduced a memory leak here, since xml won't get free()'d. Just remove the "return 0" and let it fall through and it should be fine.
Yep, good call. Although, if we fall through here, we run into problems with the (*dominfo)->dev_mem_ct = get_mem_devices(dom, &(*dominfo)->dev_mem); related pieces since dominfo is NULL. Instead, I think it should be: ret = _get_dominfo(xml, *dominfo); free(xml); if (!ret) { free(*dominfo); *dominfo = NULL; return 0; }
KR> - if (!get_dominfo(dom, &dominfo)) KR> + ret = get_dominfo(dom, &dominfo); KR> + if (!ret) KR> goto out;
Maybe my eyes are tired at the end of the day, but does this fix anything or is it just a style change?
We need to capture ret from get_domain(). ret is initialized to 1. If we don't capture the ret from get_domain(), then we return 1 to get_vssd_instance(). This causes the if statement to pass, and we return an empty instance. If get_domain() fails, we need to return NULL. -- Kaitlin Rupert IBM Linux Technology Center karupert@us.ibm.com