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

Re: [libvirt] [PATCH] fix xenDaemonListDefinedDomains



2009/11/25 Jim Fehlig <jfehlig novell com>:
> Commit 790f0b3057787bb64da8c46c111ff8d3eff7b2af causes contents of names
> array to be freed even on success, resulting in no listing of defined
> but inactive Xen domains. Patch below fixes it.
>
> Regards,
> Jim
>

Good catch, I just reviewed this commit to see if I've caused similar
bugs elsewhere, but this seems to be the only one.

> Index: libvirt-0.7.4/src/xen/xend_internal.c
> ===================================================================
> --- libvirt-0.7.4.orig/src/xen/xend_internal.c
> +++ libvirt-0.7.4/src/xen/xend_internal.c
> @@ -4693,13 +4693,14 @@ xenDaemonListDefinedDomains(virConnectPt
>          }
>
>          if (ret >= maxnames)
> -            break;
> +            goto out;
>      }
>
>  error:
>      for (i = 0; i < ret; ++i)
>          VIR_FREE(names[i]);
>
> +out:
>      sexpr_free(root);
>      return(ret);
>  }

Your patch doesn't fix the problem in all situations. If maxnames is
larger than the actual number of domains then goto out is never
executed.

I also forgot to set ret to -1 after freeing the names, this needs to
be fixed too.

I propose the attached patch to solve this issues.

Matthias
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index d61e9e6..a0c7d77 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -4696,12 +4696,17 @@ xenDaemonListDefinedDomains(virConnectPtr conn, char **const names, int maxnames
             break;
     }
 
+cleanup:
+    sexpr_free(root);
+    return(ret);
+
 error:
     for (i = 0; i < ret; ++i)
         VIR_FREE(names[i]);
 
-    sexpr_free(root);
-    return(ret);
+    ret = -1;
+
+    goto cleanup;
 }
 
 /**

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