On Wed, May 04, 2011 at 08:38:43AM +0200, Matthias Bolte wrote: > 2011/5/4 Laine Stump <laine@xxxxxxxxx>: > > On 05/03/2011 03:10 PM, Eric Blake wrote: > >> > >> Detected by clang. > >> > >> * src/esx/esx_driver.c (esxDomainGetInfo): Fail early on error. > >> --- > >> src/esx/esx_driver.c | 1 + > >> 1 files changed, 1 insertions(+), 0 deletions(-) > >> > >> diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c > >> index 1f8f90b..e929208 100644 > >> --- a/src/esx/esx_driver.c > >> +++ b/src/esx/esx_driver.c > >> @@ -2372,8 +2372,9 @@ esxDomainGetInfo(virDomainPtr domain, > >> virDomainInfoPtr info) > >> > >> if (perfEntityMetric == NULL) { > >> VIR_ERROR(_("QueryPerf returned object with > >> unexpected type '%s'"), > >> > >> esxVI_Type_ToString(perfEntityMetricBase->_type)); > >> + goto cleanup; > >> } > >> > >> perfMetricIntSeries = > >> > >> esxVI_PerfMetricIntSeries_DynamicCast(perfEntityMetric->value); > > > > I would just say ACK, since this obviously eliminates a null dereference, > > but I notice that the following check for perfMetricIntSeries == NULL also > > calls VIR_ERROR and then doesn't goto cleanup, so I'm wondering if maybe the > > intent is that if either of these is NULL, result should still get set to 0. > > Mathias? > > > > NACK, as written. > > There is a potential NULL dereference in there, but just going to > cleanup results in freeing static strings here. Patch 1 fixes it > correctly. > > Actually this code has been there for a while now, but didn't do > anything useful with the queried values because of the format mismatch > between libvirt and ESX. Therefore, patch 2 disables that code but > keeps it as a reference for how to query performance counters. If code is not used, then I would ACK disabling for 0.9.1 and then we can figure out if this really should eb provided in some other ways. My only worry is a possible regression, you decide :-) 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