| Message ID | 1406035177-221890-4-git-send-email-den@openvz.org |
|---|---|
| State | New |
| Headers | show |
On Tue, Jul 22, 2014 at 05:19:36PM +0400, Denis V. Lunev wrote: > and rework error path a bit. There is no difference at the moment, but > the code will be definitely shorter when additional processing will > be required for WithouFreSpacExt > > Signed-off-by: Denis V. Lunev <den@openvz.org> > CC: Kevin Wolf <kwolf@redhat.com> > CC: Stefan Hajnoczi <stefanha@redhat.com> > --- > block/parallels.c | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/block/parallels.c b/block/parallels.c > index 8f9ec8a..02739cf 100644 > --- a/block/parallels.c > +++ b/block/parallels.c > @@ -85,12 +85,10 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, > goto fail; > } > > - if (memcmp(ph.magic, HEADER_MAGIC, 16) || > - (le32_to_cpu(ph.version) != HEADER_VERSION)) { > - error_setg(errp, "Image not in Parallels format"); > - ret = -EINVAL; > - goto fail; > - } > + if (le32_to_cpu(ph.version) != HEADER_VERSION) > + goto fail_format; > + if (memcmp(ph.magic, HEADER_MAGIC, 16)) > + goto fail_format; QEMU coding style dictates these statements have curly braces, even though they are just one liners. (If you run your patches against scripts/checkpatch.pl, it should catch most style issues). I think this patch could also just be squashed into patch 4, if desired. > > bs->total_sectors = (uint32_t)le64_to_cpu(ph.nb_sectors); > > @@ -120,6 +118,9 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, > qemu_co_mutex_init(&s->lock); > return 0; > > +fail_format: > + error_setg(errp, "Image not in Parallels format"); > + ret = -EINVAL; > fail: > g_free(s->catalog_bitmap); > return ret; > -- > 1.9.1 > >
On 24/07/14 22:50, Jeff Cody wrote: > On Tue, Jul 22, 2014 at 05:19:36PM +0400, Denis V. Lunev wrote: >> and rework error path a bit. There is no difference at the moment, but >> the code will be definitely shorter when additional processing will >> be required for WithouFreSpacExt >> >> Signed-off-by: Denis V. Lunev <den@openvz.org> >> CC: Kevin Wolf <kwolf@redhat.com> >> CC: Stefan Hajnoczi <stefanha@redhat.com> >> --- >> block/parallels.c | 13 +++++++------ >> 1 file changed, 7 insertions(+), 6 deletions(-) >> >> diff --git a/block/parallels.c b/block/parallels.c >> index 8f9ec8a..02739cf 100644 >> --- a/block/parallels.c >> +++ b/block/parallels.c >> @@ -85,12 +85,10 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, >> goto fail; >> } >> >> - if (memcmp(ph.magic, HEADER_MAGIC, 16) || >> - (le32_to_cpu(ph.version) != HEADER_VERSION)) { >> - error_setg(errp, "Image not in Parallels format"); >> - ret = -EINVAL; >> - goto fail; >> - } >> + if (le32_to_cpu(ph.version) != HEADER_VERSION) >> + goto fail_format; >> + if (memcmp(ph.magic, HEADER_MAGIC, 16)) >> + goto fail_format; > QEMU coding style dictates these statements have curly braces, even > though they are just one liners. (If you run your patches against > scripts/checkpatch.pl, it should catch most style issues). ok, I just used a kernel convention. Will redo this here and in the next path. This is not a problem :) Thank you for pointing this out. > I think this patch could also just be squashed into patch 4, if > desired. I'd prefer not to do this to have behavior changing and behavior non-changing stuff separated. >> >> bs->total_sectors = (uint32_t)le64_to_cpu(ph.nb_sectors); >> >> @@ -120,6 +118,9 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, >> qemu_co_mutex_init(&s->lock); >> return 0; >> >> +fail_format: >> + error_setg(errp, "Image not in Parallels format"); >> + ret = -EINVAL; >> fail: >> g_free(s->catalog_bitmap); >> return ret; >> -- >> 1.9.1 >> >>
diff --git a/block/parallels.c b/block/parallels.c index 8f9ec8a..02739cf 100644 --- a/block/parallels.c +++ b/block/parallels.c @@ -85,12 +85,10 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, goto fail; } - if (memcmp(ph.magic, HEADER_MAGIC, 16) || - (le32_to_cpu(ph.version) != HEADER_VERSION)) { - error_setg(errp, "Image not in Parallels format"); - ret = -EINVAL; - goto fail; - } + if (le32_to_cpu(ph.version) != HEADER_VERSION) + goto fail_format; + if (memcmp(ph.magic, HEADER_MAGIC, 16)) + goto fail_format; bs->total_sectors = (uint32_t)le64_to_cpu(ph.nb_sectors); @@ -120,6 +118,9 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, qemu_co_mutex_init(&s->lock); return 0; +fail_format: + error_setg(errp, "Image not in Parallels format"); + ret = -EINVAL; fail: g_free(s->catalog_bitmap); return ret;
and rework error path a bit. There is no difference at the moment, but the code will be definitely shorter when additional processing will be required for WithouFreSpacExt Signed-off-by: Denis V. Lunev <den@openvz.org> CC: Kevin Wolf <kwolf@redhat.com> CC: Stefan Hajnoczi <stefanha@redhat.com> --- block/parallels.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)