[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 3/7] virsh-domain-monitor: Resolve Coverity issues



On Thu, Jul 11, 2013 at 08:34:03AM -0400, John Ferlan wrote:
> Recent changes uncovered a pair of NEGATIVE_RETURNS when processing the
> 'nnames' in 'vshDomainListCollect' in the for loop due to possible -1 value.
> ---
>  tools/virsh-domain-monitor.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
> index 7af765e..bd853ba 100644
> --- a/tools/virsh-domain-monitor.c
> +++ b/tools/virsh-domain-monitor.c
> @@ -1399,6 +1399,7 @@ vshDomainListCollect(vshControl *ctl, unsigned int flags)
>      vshDomainListPtr list = vshMalloc(ctl, sizeof(*list));
>      size_t i;
>      int ret;
> +    int rc;
>      int *ids = NULL;
>      int nids = 0;
>      char **names = NULL;
> @@ -1467,16 +1468,17 @@ fallback:
>  
>      if (!VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_ACTIVE) ||
>          VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_INACTIVE)) {
> -        if ((nnames = virConnectNumOfDefinedDomains(ctl->conn)) < 0) {
> +        if ((rc = virConnectNumOfDefinedDomains(ctl->conn)) < 0) {
>              vshError(ctl, "%s", _("Failed to list inactive domains"));
>              goto cleanup;
>          }
>  
> -        if (nnames) {
> +        if (rc) {
> +            nnames = rc;
>              names = vshMalloc(ctl, sizeof(char *) * nnames);
>  
> -            if ((nnames = virConnectListDefinedDomains(ctl->conn, names,
> -                                                      nnames)) < 0) {
> +            if ((rc = virConnectListDefinedDomains(ctl->conn, names,
> +                                                   nnames)) < 0) {
>                  vshError(ctl, "%s", _("Failed to list inactive domains"));
>                  goto cleanup;
>              }

I think is is nicer to change the code in the 'cleanup' label to do

  cleanup:
    for (i = 0; nnames != -1 && i < nnames; i++)
        VIR_FREE(names[i]);


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]