From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46679) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XNP4H-0007M1-GA for qemu-devel@nongnu.org; Fri, 29 Aug 2014 12:30:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XNP49-0002jv-6W for qemu-devel@nongnu.org; Fri, 29 Aug 2014 12:30:29 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38337) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XNP48-0002jp-Uy for qemu-devel@nongnu.org; Fri, 29 Aug 2014 12:30:21 -0400 From: Stefan Hajnoczi Date: Fri, 29 Aug 2014 17:29:33 +0100 Message-Id: <1409329803-20744-6-git-send-email-stefanha@redhat.com> In-Reply-To: <1409329803-20744-1-git-send-email-stefanha@redhat.com> References: <1409329803-20744-1-git-send-email-stefanha@redhat.com> Subject: [Qemu-devel] [PULL 05/35] 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: Peter Maydell , Stefan Hajnoczi The out label has the qemu_progress_end() and other cleanup calls. Always goto out in error paths so the cleanup happens. These error paths now return 1 instead of -1. 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 Reviewed-by: Max Reitz --- qemu-img.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/qemu-img.c b/qemu-img.c index a761c36..ff29ed1 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -2304,7 +2304,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; @@ -2376,14 +2376,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; } /* @@ -2394,7 +2394,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 */ @@ -2420,11 +2421,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