2011/5/4 Daniel Veillard <veillard@xxxxxxxxxx>:
> 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
>
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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list