| Message ID | 1409077076-29855-4-git-send-email-stefanha@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 26.08.2014 20:17, Stefan Hajnoczi wrote: > 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 <jsnow@redhat.com> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > --- > qemu-img.c | 15 ++++++--------- > 1 file changed, 6 insertions(+), 9 deletions(-) Reviewed-by: Max Reitz <mreitz@redhat.com>
Stefan Hajnoczi <stefanha@redhat.com> writes: > 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. Please mention here that you fix exit code.from -1 to 1. > > Reported-by: John Snow <jsnow@redhat.com> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> [...]
On Wed, Aug 27, 2014 at 08:35:33AM +0200, Markus Armbruster wrote: > Stefan Hajnoczi <stefanha@redhat.com> writes: > > > 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. > > Please mention here that you fix exit code.from -1 to 1. Fixed up when merging
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);
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 <jsnow@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- qemu-img.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)