[libvirt] [PATCH 5/6] xenapi: Resolve Coverity REVERSE_INULL

[Peter Krempa]

On Tue, Mar 10, 2015 at 19:20:28 -0400, John Ferlan wrote:
> Coverity complains that "net_set" is compared to NULL before calling
> xen_network_set_free, but used rather liberally before that.  While
> I was looking at the code I also noted that if the virAsprintfQuiet
> fails, then we leak our structures - so I added those too.
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  src/xenapi/xenapi_utils.c | 25 +++++++++++++++----------
>  1 file changed, 15 insertions(+), 10 deletions(-)
> 
> diff --git a/src/xenapi/xenapi_utils.c b/src/xenapi/xenapi_utils.c
> index ce09dfe..0c13745 100644
> --- a/src/xenapi/xenapi_utils.c
> +++ b/src/xenapi/xenapi_utils.c
> @@ -399,14 +399,14 @@ createVifNetwork(virConnectPtr conn, xen_vm vm, int device,
>      xen_network_set *net_set = NULL;
>      xen_network_record *net_rec = NULL;
>      int cnt = 0;
> -    if (xen_network_get_all(session, &net_set)) {
> -        for (cnt = 0; cnt < net_set->size; cnt++) {
> -            if (xen_network_get_record(session, &net_rec, net_set->contents[cnt])) {
> -                if (STREQ(net_rec->bridge, bridge)) {
> -                    break;
> -                } else {
> -                    xen_network_record_free(net_rec);
> -                }
> +    if (!xen_network_get_all(session, &net_set))
> +        return -1

missing semicolon?

Also @vm_opt is allocated in the declarations section and would be
leaked here. Pre-existing though, so I won't insist on fixing that too.

> +    for (cnt = 0; cnt < net_set->size; cnt++) {
> +        if (xen_network_get_record(session, &net_rec, net_set->contents[cnt])) {
> +            if (STREQ(net_rec->bridge, bridge)) {
> +                break;
> +            } else {
> +                xen_network_record_free(net_rec);
>              }
>          }
>      }
> @@ -425,8 +425,13 @@ createVifNetwork(virConnectPtr conn, xen_vm vm, int device,
>          vif_record->other_config = xen_string_string_map_alloc(0);
>          vif_record->runtime_properties = xen_string_string_map_alloc(0);
>          vif_record->qos_algorithm_params = xen_string_string_map_alloc(0);
> -        if (virAsprintfQuiet(&vif_record->device, "%d", device) < 0)
> +        if (virAsprintfQuiet(&vif_record->device, "%d", device) < 0) {
> +            xen_vif_free(vif);

@vif is allocated few lines below so no need to free it yet.

> +            xen_vif_record_free(vif_record);
> +            xen_network_record_free(net_rec);
> +            xen_network_set_free(net_set);
>              return -1;
> +        }
>          xen_vif_create(session, &vif, vif_record);
>          if (!vif) {
>              xen_vif_free(vif);
> @@ -438,7 +443,7 @@ createVifNetwork(virConnectPtr conn, xen_vm vm, int device,
>          xen_vif_record_free(vif_record);
>          xen_network_record_free(net_rec);
>      }
> -    if (net_set != NULL) xen_network_set_free(net_set);
> +    xen_network_set_free(net_set);
>      return -1;
>  }

Looks good, ACK if you compile test this patch before pushing, with or
without fixing of the other existing leak.

Peter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150311/710fa040/attachment-0001.sig>