Re: [libvirt] [PATCH] esx: avoid null dereference on error

2011/5/4 Laine Stump <laine(a)laine.org&gt;: > 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.