2011/5/4 Daniel Veillard <veill...@redhat.com>: > On Wed, May 04, 2011 at 08:38:43AM +0200, Matthias Bolte wrote: >> 2011/5/4 Laine Stump <la...@laine.org>: >> > 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 >
Okay, I've tested this two patches now that I have access to an ESX server again and everything still works as expected. So I decide to push them for 0.9.1 :) Matthias -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list