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 <ebl...@redhat.com> > >> --- > >> 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
pgpR4qhucpPqZ.pgp
Description: PGP signature
