1. Patchew
  2. QEMU
  3. View series

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

Peter Maydell posted 1 patch 8 years, 7 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1490034784-28177-1-git-send-email-peter.maydell@linaro.org
Test s390x passed
There is a newer version of this series
block/file-posix.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
[Qemu-devel] [PATCH for-2.9?] block/file-posix.c: Fix unused variable warning on OpenBSD
Posted by Peter Maydell 8 years, 7 months ago 
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 <peter.maydell@linaro.org>
---
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[] = {
 #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
Re: [Qemu-devel] [Qemu-block] [PATCH for-2.9?] block/file-posix.c: Fix unused variable warning on OpenBSD
Posted by Jeff Cody 8 years, 7 months ago 
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 <peter.maydell@linaro.org>
> ---
> 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[] = {
>  #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 <jcody@redhat.com>
Re: [Qemu-devel] [Qemu-block] [PATCH for-2.9?] block/file-posix.c: Fix unused variable warning on OpenBSD
Posted by Philippe Mathieu-Daudé 8 years, 7 months ago 
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 <peter.maydell@linaro.org>
>> ---
>> 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 <jcody@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Re: [Qemu-devel] [Qemu-block] [PATCH for-2.9?] block/file-posix.c: Fix unused variable warning on OpenBSD
Posted by Philippe Mathieu-Daudé 8 years, 7 months ago 
On 03/22/2017 01:23 PM, Philippe Mathieu-Daudé wrote:
> 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 <peter.maydell@linaro.org>
>>> ---
>>> 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.

I mean `static const`, sorry.

>
>>>  #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 <jcody@redhat.com>
>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Re: [Qemu-devel] [PATCH for-2.9?] block/file-posix.c: Fix unused variable warning on OpenBSD
Posted by Max Reitz 8 years, 7 months ago 
On 20.03.2017 19:33, 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 <peter.maydell@linaro.org>
> ---
> 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'm fine with targeting 2.9.

> 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[] = {
>  #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++) {

On 32-bit, though:

./block/file-posix.c: In function ‘probe_logical_blocksize’:
./qemu/block/file-posix.c:229:19: error: comparison of unsigned
expression < 0 is always false [-Werror=type-limits]

A cast (int)ARRAY_SIZE() would fix that (I had the same problem once...).

Max

> +        if (ioctl(fd, ioctl_list[i], &sector_size) >= 0) {
> +            *sector_size_p = sector_size;
> +            success = true;
> +        }
> +    }
>  
>      return success ? 0 : -errno;
>  }
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.