Re: [Qemu-devel] [PATCH v11 1/9] qemu-io: Improve alignment checks

["Philippe Mathieu-Daudé" <f4bug@amsat.org>]

On 04/29/2017 04:14 PM, Eric Blake wrote:
> Several copy-and-pasted alignment checks exist in qemu-io, which
> could use some minor improvements:
>
> - Manual comparison against 0x1ff is not as clean as using our
> alignment macros (QEMU_IS_ALIGNED) from osdep.h.
>
> - The error messages aren't quite grammatically correct.
>
> Suggested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Suggested-by: Max Reitz <mreitz@redhat.com>
> Signed-off-by: Eric Blake <eblake@redhat.com>
>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
> v11: retitle [was "qemu-io: Don't open-code QEMU_IS_ALIGNED"], improve
> error messages
> v10: new patch
> ---
>  qemu-io-cmds.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
> index 21af9e6..6a0024b 100644
> --- a/qemu-io-cmds.c
> +++ b/qemu-io-cmds.c
> @@ -740,13 +740,13 @@ static int read_f(BlockBackend *blk, int argc, char **argv)
>      }
>
>      if (bflag) {
> -        if (offset & 0x1ff) {
> -            printf("offset %" PRId64 " is not sector aligned\n",
> +        if (!QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE)) {
> +            printf("%" PRId64 " is not a sector-aligned value for 'offset'\n",
>                     offset);
>              return 0;
>          }
> -        if (count & 0x1ff) {
> -            printf("count %"PRId64" is not sector aligned\n",
> +        if (!QEMU_IS_ALIGNED(count, BDRV_SECTOR_SIZE)) {
> +            printf("%"PRId64" is not a sector-aligned value for 'count'\n",
>                     count);
>              return 0;
>          }
> @@ -1050,14 +1050,14 @@ static int write_f(BlockBackend *blk, int argc, char **argv)
>      }
>
>      if (bflag || cflag) {
> -        if (offset & 0x1ff) {
> -            printf("offset %" PRId64 " is not sector aligned\n",
> +        if (!QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE)) {
> +            printf("%" PRId64 " is not a sector-aligned value for 'offset'\n",
>                     offset);
>              return 0;
>          }
>
> -        if (count & 0x1ff) {
> -            printf("count %"PRId64" is not sector aligned\n",
> +        if (!QEMU_IS_ALIGNED(count, BDRV_SECTOR_SIZE)) {
> +            printf("%"PRId64" is not a sector-aligned value for 'count'\n",
>                     count);
>              return 0;
>          }
> @@ -1769,8 +1769,8 @@ static int alloc_f(BlockBackend *blk, int argc, char **argv)
>      if (offset < 0) {
>          print_cvtnum_err(offset, argv[1]);
>          return 0;
> -    } else if (offset & 0x1ff) {
> -        printf("offset %" PRId64 " is not sector aligned\n",
> +    } else if (!QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE)) {
> +        printf("%" PRId64 " is not a sector-aligned value for 'offset'\n",
>                 offset);
>          return 0;
>      }
>