[libvirt] [PATCH 09/12] esx: avoid dead code

[Matthias Bolte]

2011/6/6 Eric Blake <eblake at redhat.com>:
> Detected by Coverity.  The beginning of the function already filtered
> out NULL objectContentList as invalid; more likely, the code was
> trying to see if esxVI_RetrieveProperties modified *objectContentList.
>
> * src/esx/esx_vi.c (esxVI_LookupObjectContentByType): Check
> correct pointer.
> ---
>
> Matthias, I'm not sure if this is the correct fix, because I don't
> know how escVI_RetrieveProperties is supposed to work.

Sometime one got to love static code analysis tools. Yes, your patch
is correct and necessary :)

esxVI_RetrieveProperties is generated and returns a list of objects
that match the given propertyFilterSpec.
esxVI_LookupObjectContentByType then tests whether the result
corresponds to the expected occurrence and reports an error otherwise.
This simplifies the callers of  esxVI_LookupObjectContentByType, but
due to the missing dereference the check was never performed because
the code thought that at least one item was obtained. NULL represents
an empty list. Your patch now fixes this and is also a potential
segfault fix because callers of esxVI_LookupObjectContentByType that
specified "required" occurrence assume *objectContentList to be
non-NULL when esxVI_LookupObjectContentByType succeeds.

>  src/esx/esx_vi.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
> index c5c38ca..64e5b73 100644
> --- a/src/esx/esx_vi.c
> +++ b/src/esx/esx_vi.c
> @@ -2,7 +2,7 @@
>  /*
>  * esx_vi.c: client for the VMware VI API 2.5 to manage ESX hosts
>  *
> - * Copyright (C) 2010 Red Hat, Inc.
> + * Copyright (C) 2010-2011 Red Hat, Inc.
>  * Copyright (C) 2009-2011 Matthias Bolte <matthias.bolte at googlemail.com>
>  *
>  * This library is free software; you can redistribute it and/or
> @@ -1737,7 +1737,7 @@ esxVI_LookupObjectContentByType(esxVI_Context *ctx,
>         goto cleanup;
>     }
>
> -    if (objectContentList == NULL) {
> +    if (*objectContentList == NULL) {
>         switch (occurrence) {
>           case esxVI_Occurrence_OptionalItem:
>           case esxVI_Occurrence_OptionalList:
>

ACK.

Matthias