[libvirt] [PATCH] virsh: fix uninitialized variable in cmdSnapshotEdit
If the domain can't be looked up, name is used unitialized after the cleanup label. Found by coverity. --- tools/virsh-snapshot.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index 4281109..0bd9583 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -459,7 +459,7 @@ cmdSnapshotEdit(vshControl *ctl, const vshCmd *cmd) virDomainPtr dom = NULL; virDomainSnapshotPtr snapshot = NULL; virDomainSnapshotPtr edited = NULL; - const char *name; + const char *name = NULL; const char *edited_name; bool ret = false; unsigned int getxml_flags = VIR_DOMAIN_XML_SECURE; @@ -532,7 +532,7 @@ cmdSnapshotEdit(vshControl *ctl, const vshCmd *cmd) ret = true; cleanup: - if (!ret) + if (!ret && name) vshError(ctl, _("Failed to update %s"), name); if (edited) virDomainSnapshotFree(edited); -- 1.7.8.6
On 15.11.2012 11:37, Ján Tomko wrote:
If the domain can't be looked up, name is used unitialized after the cleanup label.
Found by coverity. --- tools/virsh-snapshot.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index 4281109..0bd9583 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -459,7 +459,7 @@ cmdSnapshotEdit(vshControl *ctl, const vshCmd *cmd) virDomainPtr dom = NULL; virDomainSnapshotPtr snapshot = NULL; virDomainSnapshotPtr edited = NULL; - const char *name; + const char *name = NULL; const char *edited_name; bool ret = false; unsigned int getxml_flags = VIR_DOMAIN_XML_SECURE; @@ -532,7 +532,7 @@ cmdSnapshotEdit(vshControl *ctl, const vshCmd *cmd) ret = true;
cleanup: - if (!ret) + if (!ret && name) vshError(ctl, _("Failed to update %s"), name); if (edited) virDomainSnapshotFree(edited);
ACKed & pushed. Thanks. Michal
On Thu, Nov 15, 2012 at 11:37:32AM +0100, Ján Tomko wrote:
If the domain can't be looked up, name is used unitialized after the cleanup label.
Found by coverity. --- tools/virsh-snapshot.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index 4281109..0bd9583 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -459,7 +459,7 @@ cmdSnapshotEdit(vshControl *ctl, const vshCmd *cmd) virDomainPtr dom = NULL; virDomainSnapshotPtr snapshot = NULL; virDomainSnapshotPtr edited = NULL; - const char *name; + const char *name = NULL; const char *edited_name; bool ret = false; unsigned int getxml_flags = VIR_DOMAIN_XML_SECURE; @@ -532,7 +532,7 @@ cmdSnapshotEdit(vshControl *ctl, const vshCmd *cmd) ret = true;
cleanup: - if (!ret) + if (!ret && name) vshError(ctl, _("Failed to update %s"), name);
Shouldn't we call vshError with a different message in the !ret && !name case? Christophe
On 11/16/12 11:58, Christophe Fergeau wrote:
Shouldn't we call vshError with a different message in the !ret && !name case?
Christophe vshCommandOptDomain and vshLookupSnapshot already give reasonable error messages in that case. For non-existent domains:
error: failed to get domain 'bla' error: Domain not found: no domain with matching name 'bla' And for snapshots: error: Domain snapshot not found: no domain snapshot with matching name 'bla'
On Fri, Nov 16, 2012 at 12:10:10PM +0100, Ján Tomko wrote:
On 11/16/12 11:58, Christophe Fergeau wrote:
Shouldn't we call vshError with a different message in the !ret && !name case?
Christophe vshCommandOptDomain and vshLookupSnapshot already give reasonable error messages in that case. For non-existent domains:
error: failed to get domain 'bla' error: Domain not found: no domain with matching name 'bla'
And for snapshots: error: Domain snapshot not found: no domain snapshot with matching name 'bla'
Ah, all is good then, thanks! Christophe
participants (3)
-
Christophe Fergeau -
Ján Tomko -
Michal Privoznik