Re: [Qemu-devel] [PATCH v3 01/10] raw-posix: Fix raw_getlength() to always return -errno on error

[Benoît Canet]

The Friday 30 May 2014 à 20:13:42 (+0200), Markus Armbruster wrote :
> We got a merry mix of -1 and -errno here.
> 
> Signed-off-by: Markus Armbruster <address@hidden>
> Reviewed-by: Eric Blake <address@hidden>
> ---
>  block/raw-posix.c | 28 ++++++++++++++++++++++------
>  1 file changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index 6586a0c..9221de5 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -1097,12 +1097,12 @@ static int64_t raw_getlength(BlockDriverState *bs)
>      struct stat st;
>  
>      if (fstat(fd, &st))
> -        return -1;
> +        return -errno;
>      if (S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode)) {
>          struct disklabel dl;
>  
>          if (ioctl(fd, DIOCGDINFO, &dl))
> -            return -1;
> +            return -errno;
>          return (uint64_t)dl.d_secsize *
>              dl.d_partitions[DISKPART(st.st_rdev)].p_size;
>      } else
> @@ -1116,7 +1116,7 @@ static int64_t raw_getlength(BlockDriverState *bs)
>      struct stat st;
>  
>      if (fstat(fd, &st))
> -        return -1;
> +        return -errno;
>      if (S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode)) {
>          struct dkwedge_info dkw;
>  
> @@ -1126,7 +1126,7 @@ static int64_t raw_getlength(BlockDriverState *bs)
>              struct disklabel dl;
>  
>              if (ioctl(fd, DIOCGDINFO, &dl))
> -                return -1;
> +                return -errno;
>              return (uint64_t)dl.d_secsize *
>                  dl.d_partitions[DISKPART(st.st_rdev)].p_size;
>          }
> @@ -1139,6 +1139,7 @@ static int64_t raw_getlength(BlockDriverState *bs)
>      BDRVRawState *s = bs->opaque;
>      struct dk_minfo minfo;
>      int ret;
> +    int64_t size;
>  
>      ret = fd_open(bs);
>      if (ret < 0) {
> @@ -1157,7 +1158,11 @@ static int64_t raw_getlength(BlockDriverState *bs)
>       * There are reports that lseek on some devices fails, but
>       * irc discussion said that contingency on contingency was overkill.
>       */
> -    return lseek(s->fd, 0, SEEK_END);
> +    size = lseek(s->fd, 0, SEEK_END);
> +    if (size < 0) {
> +        return -errno;
> +    }
> +    return size;
>  }
>  #elif defined(CONFIG_BSD)
>  static int64_t raw_getlength(BlockDriverState *bs)
> @@ -1195,6 +1200,9 @@ again:
>          size = LONG_LONG_MAX;
>  #else
>          size = lseek(fd, 0LL, SEEK_END);
> +        if (size < 0) {
> +            return -errno;
> +        }
>  #endif
>  #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
>          switch(s->type) {
> @@ -1211,6 +1219,9 @@ again:
>  #endif
>      } else {
>          size = lseek(fd, 0, SEEK_END);
> +        if (size < 0) {
> +            return -errno;
> +        }
>      }
>      return size;
>  }
> @@ -1219,13 +1230,18 @@ static int64_t raw_getlength(BlockDriverState *bs)
>  {
>      BDRVRawState *s = bs->opaque;
>      int ret;
> +    int64_t size;
>  
>      ret = fd_open(bs);
>      if (ret < 0) {
>          return ret;
>      }
>  
> -    return lseek(s->fd, 0, SEEK_END);
> +    size = lseek(s->fd, 0, SEEK_END);
> +    if (size < 0) {
> +        return -errno;
> +    }
> +    return size;
>  }
>  #endif
>  
> -- 
> 1.9.3
> 
> 
Reviewed-by: Benoit Canet <address@hidden>