On 02/08/14 11:00, Kevin Wolf wrote: > Reported-by: Laszlo Ersek <[email protected]> > Signed-off-by: Kevin Wolf <[email protected]> > --- > block/blkdebug.c | 14 +++++++++----- > 1 file changed, 9 insertions(+), 5 deletions(-) > > diff --git a/block/blkdebug.c b/block/blkdebug.c > index 56c4cd0..8eb0db0 100644 > --- a/block/blkdebug.c > +++ b/block/blkdebug.c > @@ -396,14 +396,14 @@ static int blkdebug_open(BlockDriverState *bs, QDict > *options, int flags, > if (error_is_set(&local_err)) { > error_propagate(errp, local_err); > ret = -EINVAL; > - goto fail; > + goto out; > } > > /* Read rules from config file or command line options */ > config = qemu_opt_get(opts, "config"); > ret = read_config(s, config, options, errp); > if (ret) { > - goto fail; > + goto out; > } > > /* Set initial state */ > @@ -414,7 +414,7 @@ static int blkdebug_open(BlockDriverState *bs, QDict > *options, int flags, > flags, true, false, &local_err); > if (ret < 0) { > error_propagate(errp, local_err); > - goto fail; > + goto out; > } > > /* Set request alignment */ > @@ -424,11 +424,15 @@ static int blkdebug_open(BlockDriverState *bs, QDict > *options, int flags, > } else { > error_setg(errp, "Invalid alignment"); > ret = -EINVAL; > - goto fail; > + goto fail_unref; > } > > ret = 0; > -fail: > + goto out; > + > +fail_unref: > + bdrv_unref(bs->file); > +out: > qemu_opts_del(opts); > return ret; > } >
I think this is correct, but the standalone "goto out" looks awful :) In such cases, when I have to combine unconditional teardown with release-on-error-only, I apply a "nested ifs" pattern, where the release-on-error-only bits (on the way out) depend on the function exit status. But "nested ifs" is foreign for qemu, so I think these goto's actually match the qemu style. Reviewed-by: Laszlo Ersek <[email protected]> Thanks! Laszlo
