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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list