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

2017-03-22 Thread Max Reitz
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 
> ---
> 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, §or_size) >= 0) {
> -*sector_size_p = sector_size;
> -success = true;
> -}
> +BLKSSZGET,
>  #endif
>  #ifdef DKIOCGETBLOCKSIZE
> -if (ioctl(fd, DKIOCGETBLOCKSIZE, §or_size) >= 0) {
> -*sector_size_p = sector_size;
> -success = true;
> -}
> +DKIOCGETBLOCKSIZE,
>  #endif
>  #ifdef DIOCGSECTORSIZE
> -if (ioctl(fd, DIOCGSECTORSIZE, §or_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], §or_size) >= 0) {
> +*sector_size_p = sector_size;
> +success = true;
> +}
> +}
>  
>  return success ? 0 : -errno;
>  }
> 




signature.asc
Description: OpenPGP digital signature


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

2017-03-20 Thread Peter Maydell
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 
---
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, §or_size) >= 0) {
-*sector_size_p = sector_size;
-success = true;
-}
+BLKSSZGET,
 #endif
 #ifdef DKIOCGETBLOCKSIZE
-if (ioctl(fd, DKIOCGETBLOCKSIZE, §or_size) >= 0) {
-*sector_size_p = sector_size;
-success = true;
-}
+DKIOCGETBLOCKSIZE,
 #endif
 #ifdef DIOCGSECTORSIZE
-if (ioctl(fd, DIOCGSECTORSIZE, §or_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], §or_size) >= 0) {
+*sector_size_p = sector_size;
+success = true;
+}
+}
 
 return success ? 0 : -errno;
 }
-- 
2.7.4