[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 10/13] block: Simplify bdrv_append_temp_snaps
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH v3 10/13] block: Simplify bdrv_append_temp_snapshot() logic |
Date: |
Thu, 6 Apr 2017 15:27:38 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 06.04.2017 um 14:52 hat Eric Blake geschrieben:
> On 04/06/2017 04:00 AM, Kevin Wolf wrote:
> > Am 05.04.2017 um 21:47 hat Eric Blake geschrieben:
> >> Noticed while checking Coccinelle results. Naming a label 'out:'
> >> when it is only used on error paths is weird; meanwhile we know
> >> that snapshot_options is NULL on success and that QDECREF(NULL)
> >> is safe. So merge the two exit paths into one.
> >>
> >> Signed-off-by: Eric Blake <address@hidden>
> >> ---
> >> block.c | 7 ++-----
> >> 1 file changed, 2 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/block.c b/block.c
> >> index 9b87bf6..a13625f 100644
> >> --- a/block.c
> >> +++ b/block.c
> >> @@ -2209,13 +2209,10 @@ static BlockDriverState
> >> *bdrv_append_temp_snapshot(BlockDriverState *bs,
> >> goto out;
> >> }
> >>
> >> +out:
> >> + QDECREF(snapshot_options);
> >> g_free(tmp_filename);
> >> return bs_snapshot;
> >
> > bs_snapshot is uninitialised or at least the wrong return value in error
> > cases. (Hm... Shouldn't the compiler catch the uninitialised part?)
>
> Odd, and I agree that I recall the compiler generally able to catch that
> (maybe it's a matter of compiling with -g vs. -O2). I'm surprised the
> autobuilder didn't flag it. (I think I missed it due to rebase churn on
> my end). The obvious fix is to:
>
> - BlockDriverState *bs_snapshot;
> + BlockDriverState *bs_snapshot = NULL;
That's just half of the fix. It doesn't fix the cases where bs_snapshot
is already set. We still want to return NULL on errors there.
Kevin
pgpi2OF5TosXJ.pgp
Description: PGP signature
- Re: [Qemu-devel] [PATCH v3 06/13] qobject: Add helper macros for common scalar insertions, (continued)
[Qemu-devel] [PATCH v3 07/13] block: Use simpler QDict/QList scalar insertion macros, Eric Blake, 2017/04/05
[Qemu-devel] [PATCH v3 13/13] test-qga: Actually test 0xff sync bytes, Eric Blake, 2017/04/05
[Qemu-devel] [PATCH v3 12/13] fdc-test: Avoid deprecated 'change' command, Eric Blake, 2017/04/05
[Qemu-devel] [PATCH v3 09/13] qobject: Use simpler QDict/QList scalar insertion macros, Eric Blake, 2017/04/05