From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44437) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XMLJx-0002sp-TK for qemu-devel@nongnu.org; Tue, 26 Aug 2014 14:18:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XMLJn-0000Ie-Sq for qemu-devel@nongnu.org; Tue, 26 Aug 2014 14:18:17 -0400 Received: from mx1.redhat.com ([209.132.183.28]:17630) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XMLJn-0000IS-KC for qemu-devel@nongnu.org; Tue, 26 Aug 2014 14:18:07 -0400 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s7QII79b000845 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Tue, 26 Aug 2014 14:18:07 -0400 From: Stefan Hajnoczi Date: Tue, 26 Aug 2014 19:17:56 +0100 Message-Id: <1409077076-29855-4-git-send-email-stefanha@redhat.com> In-Reply-To: <1409077076-29855-1-git-send-email-stefanha@redhat.com> References: <1409077076-29855-1-git-send-email-stefanha@redhat.com> Subject: [Qemu-devel] [PATCH 3/3] qemu-img: always goto out in img_snapshot() error paths List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Kevin Wolf , Jeff Cody , jsnow@redhat.com, Markus Armbruster , Stefan Hajnoczi The out label has the qemu_progress_end() and other cleanup calls. Always goto out in error paths so the cleanup happens. Note that bdrv_unref(NULL) is safe. We just need to initialize bs to NULL at the top of the function. We can now remove the obsolete bs_old_backing = NULL and bs_new_backing = NULL for safe mode. Originally it was necessary in commit 3e85c6fd ("qemu-img rebase") but became useless in commit c2abcce ("qemu-img: avoid calling exit(1) to release resources properly") because the variables are already initialized during declaration. Reported-by: John Snow Signed-off-by: Stefan Hajnoczi --- qemu-img.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/qemu-img.c b/qemu-img.c index 01750b7..ac2e21b 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -2323,7 +2323,7 @@ static int img_snapshot(int argc, char **argv) static int img_rebase(int argc, char **argv) { - BlockDriverState *bs, *bs_old_backing = NULL, *bs_new_backing = NULL; + BlockDriverState *bs = NULL, *bs_old_backing = NULL, *bs_new_backing = NULL; BlockDriver *old_backing_drv, *new_backing_drv; char *filename; const char *fmt, *cache, *src_cache, *out_basefmt, *out_baseimg; @@ -2395,14 +2395,14 @@ static int img_rebase(int argc, char **argv) ret = bdrv_parse_cache_flags(cache, &flags); if (ret < 0) { error_report("Invalid cache option: %s", cache); - return -1; + goto out; } src_flags = BDRV_O_FLAGS; ret = bdrv_parse_cache_flags(src_cache, &src_flags); if (ret < 0) { error_report("Invalid source cache option: %s", src_cache); - return -1; + goto out; } /* @@ -2413,7 +2413,8 @@ static int img_rebase(int argc, char **argv) */ bs = bdrv_new_open("image", filename, fmt, flags, true, quiet); if (!bs) { - return 1; + ret = -1; + goto out; } /* Find the right drivers for the backing files */ @@ -2439,11 +2440,7 @@ static int img_rebase(int argc, char **argv) } /* For safe rebasing we need to compare old and new backing file */ - if (unsafe) { - /* Make the compiler happy */ - bs_old_backing = NULL; - bs_new_backing = NULL; - } else { + if (!unsafe) { char backing_name[1024]; bs_old_backing = bdrv_new("old_backing", &error_abort); -- 1.9.3