Re: [Qemu-block] [Qemu-devel] [PATCH for-2.9?] block/file-posix.c: Fix unused variable warning on OpenBSD

[Philippe Mathieu-Daudé]

On 03/22/2017 10:58 AM, Jeff Cody wrote:
> On Mon, Mar 20, 2017 at 06:33:04PM +0000, Peter Maydell wrote:
> > On OpenBSD none of the ioctls probe_logical_blocksize() tries
> > exist, so the variable sector_size is unused. Refactor the
> > code to avoid this (and reduce the duplicated code).
> >
> > Signed-off-by: Peter Maydell <address@hidden>
> > ---
> > The alternative would be to move the variable so it was
> > local to a code block inside each #ifdef, but this seemed
> > a bit nicer anyway.
> >
> > Tentatively tagged 'for-2.9' just because this is the only
> > warning in the OpenBSD build, but I don't insist on it.
> >
> > I've opted to retain the existing behaviour of "try every
> > ioctl available and use the last one that works" rather
> > than "stop as soon as something worked".
> >
> > ---
> >  block/file-posix.c | 28 ++++++++++++++--------------
> >  1 file changed, 14 insertions(+), 14 deletions(-)
> >
> > diff --git a/block/file-posix.c b/block/file-posix.c
> > index 53febd3..b980d23 100644
> > --- a/block/file-posix.c
> > +++ b/block/file-posix.c
> > @@ -219,28 +219,28 @@ static int probe_logical_blocksize(int fd, unsigned int 
> > *sector_size_p)
> >  {
> >      unsigned int sector_size;
> >      bool success = false;
> > +    int i;
> >
> >      errno = ENOTSUP;
> > -
> > -    /* Try a few ioctls to get the right size */
> > +    unsigned long ioctl_list[] = {

this can be `const` but not a bit win.

> >  #ifdef BLKSSZGET
> > -    if (ioctl(fd, BLKSSZGET, &sector_size) >= 0) {
> > -        *sector_size_p = sector_size;
> > -        success = true;
> > -    }
> > +        BLKSSZGET,
> >  #endif
> >  #ifdef DKIOCGETBLOCKSIZE
> > -    if (ioctl(fd, DKIOCGETBLOCKSIZE, &sector_size) >= 0) {
> > -        *sector_size_p = sector_size;
> > -        success = true;
> > -    }
> > +        DKIOCGETBLOCKSIZE,
> >  #endif
> >  #ifdef DIOCGSECTORSIZE
> > -    if (ioctl(fd, DIOCGSECTORSIZE, &sector_size) >= 0) {
> > -        *sector_size_p = sector_size;
> > -        success = true;
> > -    }
> > +        DIOCGSECTORSIZE,
> >  #endif
> > +    };
> > +
> > +    /* Try a few ioctls to get the right size */
> > +    for (i = 0; i < ARRAY_SIZE(ioctl_list); i++) {
> > +        if (ioctl(fd, ioctl_list[i], &sector_size) >= 0) {
> > +            *sector_size_p = sector_size;
> > +            success = true;
> > +        }
> > +    }
> >
> >      return success ? 0 : -errno;
> >  }
> > --
> > 2.7.4
>
> Reviewed-by: Jeff Cody <address@hidden>

Reviewed-by: Philippe Mathieu-Daudé <address@hidden>