Patchwork raw-posix: Fix build without is_allocated support

login
register
mail settings
Submitter Kevin Wolf
Date June 20, 2012, 8:02 a.m.
Message ID <1340179371-8128-1-git-send-email-kwolf@redhat.com>
Download mbox | patch
Permalink /patch/165910/
State New
Headers show

Comments

Kevin Wolf - June 20, 2012, 8:02 a.m.
Move the declaration of s into the #ifdef sections that actually make
use of it.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/raw-posix.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)
Alexander Graf - June 20, 2012, 9:36 a.m.
On 20.06.2012, at 10:02, Kevin Wolf wrote:

> Move the declaration of s into the #ifdef sections that actually make
> use of it.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Yup, that fixes it for me. Btw, when did we start declaring variables within actual code? Most of the QEMU code follows the "variables have to be declared on the top of a block" methodology.

Tested-by: Alexander Graf <agraf@suse.de>


Alex
Kevin Wolf - June 20, 2012, 9:49 a.m.
Am 20.06.2012 11:36, schrieb Alexander Graf:
> 
> On 20.06.2012, at 10:02, Kevin Wolf wrote:
> 
>> Move the declaration of s into the #ifdef sections that actually make
>> use of it.
>>
>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> 
> Yup, that fixes it for me. Btw, when did we start declaring variables within actual code? Most of the QEMU code follows the "variables have to be declared on the top of a block" methodology.

Yes, and generally I think this is good style because it improves
readability. I see #ifdef blocks as an exception because logically and
visually they are separate blocks, even though they aren't in C. But C99
allows this, so why not use it in this case. (And I think you can find
more examples like this in qemu where it's used with #ifdef)

> Tested-by: Alexander Graf <agraf@suse.de>

Thanks for testing. Just out of curiosity, which host platform did you
even use to get the fallback case?

Kevin
Alexander Graf - June 20, 2012, 9:53 a.m.
On 20.06.2012, at 11:49, Kevin Wolf wrote:

> Am 20.06.2012 11:36, schrieb Alexander Graf:
>> 
>> On 20.06.2012, at 10:02, Kevin Wolf wrote:
>> 
>>> Move the declaration of s into the #ifdef sections that actually make
>>> use of it.
>>> 
>>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>> 
>> Yup, that fixes it for me. Btw, when did we start declaring variables within actual code? Most of the QEMU code follows the "variables have to be declared on the top of a block" methodology.
> 
> Yes, and generally I think this is good style because it improves
> readability. I see #ifdef blocks as an exception because logically and
> visually they are separate blocks, even though they aren't in C. But C99
> allows this, so why not use it in this case. (And I think you can find
> more examples like this in qemu where it's used with #ifdef)

Well, had I known that this is valid coding style wise, I could've simplified a bit of ppc kvm code, where we duplicate the #ifdef - once for the definition and once for the actual code.

> 
>> Tested-by: Alexander Graf <agraf@suse.de>
> 
> Thanks for testing. Just out of curiosity, which host platform did you
> even use to get the fallback case?

This is on my PPC test machine, which is running openSUSE 11.1, which is the last released openSUSE PPC version :).


Alex

Patch

diff --git a/block/raw-posix.c b/block/raw-posix.c
index bf7700a..0dce089 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -606,7 +606,6 @@  static int coroutine_fn raw_co_is_allocated(BlockDriverState *bs,
                                             int64_t sector_num,
                                             int nb_sectors, int *pnum)
 {
-    BDRVRawState *s = bs->opaque;
     off_t start, data, hole;
     int ret;
 
@@ -616,11 +615,15 @@  static int coroutine_fn raw_co_is_allocated(BlockDriverState *bs,
     }
 
     start = sector_num * BDRV_SECTOR_SIZE;
+
 #ifdef CONFIG_FIEMAP
+
+    BDRVRawState *s = bs->opaque;
     struct {
         struct fiemap fm;
         struct fiemap_extent fe;
     } f;
+
     f.fm.fm_start = start;
     f.fm.fm_length = (int64_t)nb_sectors * BDRV_SECTOR_SIZE;
     f.fm.fm_flags = 0;
@@ -643,7 +646,11 @@  static int coroutine_fn raw_co_is_allocated(BlockDriverState *bs,
         data = f.fe.fe_logical;
         hole = f.fe.fe_logical + f.fe.fe_length;
     }
+
 #elif defined SEEK_HOLE && defined SEEK_DATA
+
+    BDRVRawState *s = bs->opaque;
+
     hole = lseek(s->fd, start, SEEK_HOLE);
     if (hole == -1) {
         /* -ENXIO indicates that sector_num was past the end of the file.