[libvirt] [PATCHv2] virsh: Report error when taking a snapshot with empty --memspec argument

[Martin Kletzander]

On 11/22/2012 02:41 PM, Peter Krempa wrote:
> When the value of memspec was empty taking of a snapshot failed without
> reporting an error.
> ---
>  tools/virsh-snapshot.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
> index 398730c..8ec6456 100644
> --- a/tools/virsh-snapshot.c
> +++ b/tools/virsh-snapshot.c
> @@ -358,18 +358,19 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd)
>      if (desc)
>          virBufferEscapeString(&buf, "  <description>%s</description>\n", desc);
> 
> -    if (vshCommandOptString(cmd, "memspec", &memspec) < 0 ||
> -        vshParseSnapshotMemspec(ctl, &buf, memspec) < 0) {
> -        virBufferFreeAndReset(&buf);
> +    if (vshCommandOptString(cmd, "memspec", &memspec) < 0) {
> +        vshError(ctl, _("memspec argument must not be empty"));
>          goto cleanup;
>      }
> +
> +    if (memspec && vshParseSnapshotMemspec(ctl, &buf, memspec) < 0)
> +        goto cleanup;
> +
>      if (vshCommandOptBool(cmd, "diskspec")) {
>          virBufferAddLit(&buf, "  <disks>\n");
>          while ((opt = vshCommandOptArgv(cmd, opt))) {
> -            if (vshParseSnapshotDiskspec(ctl, &buf, opt->data) < 0) {
> -                virBufferFreeAndReset(&buf);
> +            if (vshParseSnapshotDiskspec(ctl, &buf, opt->data) < 0)
>                  goto cleanup;
> -            }
>          }
>          virBufferAddLit(&buf, "  </disks>\n");
>      }
> @@ -390,6 +391,7 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd)
>      ret = vshSnapshotCreate(ctl, dom, buffer, flags, NULL);
> 
>  cleanup:
> +    virBufferFreeAndReset(&buf);
>      VIR_FREE(buffer);
>      if (dom)
>          virDomainFree(dom);
> 

I've missed the leak in the first version, thanks for finding that out.
 I double-checked this one and it seems alright, so ACK.

Martin