[Qemu-devel] [PATCH 00/29] coccinelle script to enforce qemu/osdep.h macros usage

[Qemu-devel] [PATCH 00/29] coccinelle script to enforce qemu/osdep.h macros usage

[Philippe Mathieu-Daudé]

Hi,

Another cocci script... Here the idea is to have the same script with all the
osdep.h macros related rules altogether.

First run with QEMU_IS_ALIGNED() and ARRAY_SIZE().

Regards,

Phil.

PD: I used the "enforce" word because I plan to have some continuous integration
job running such scripts (also the clang-tidy good stuff).

Philippe Mathieu-Daudé (29):
  coccinelle: add a script to enforce qemu/osdep.h macros usage
  pci: remove superfluous parenthesis
  qemu-img: use QEMU_IS_ALIGNED macro
  scsi-disk: use QEMU_IS_ALIGNED macro
  block: use QEMU_IS_ALIGNED macro
  migration/block: use QEMU_IS_ALIGNED macro
  ds1338: use QEMU_IS_ALIGNED macro
  ui: use QEMU_IS_ALIGNED macro
  net: use QEMU_IS_ALIGNED macro
  net/rocker: use QEMU_IS_ALIGNED macro
  s390x/sclp: use QEMU_IS_ALIGNED macro
  sm501: use QEMU_IS_ALIGNED macro
  tcg: use QEMU_IS_ALIGNED macro
  cris: use QEMU_IS_ALIGNED macro
  microblaze: use QEMU_IS_ALIGNED macro
  lm32: use QEMU_IS_ALIGNED macro
  sparc: use QEMU_IS_ALIGNED macro
  nios2: use QEMU_IS_ALIGNED macro
  disas: use QEMU_IS_ALIGNED macro
  disas: use ARRAY_SIZE macro
  qga: use ARRAY_SIZE macro
  vmsvga: use ARRAY_SIZE macro
  async: use ARRAY_SIZE macro
  tests/acpi: use ARRAY_SIZE macro
  tests/qos: use ARRAY_SIZE macro
  tests/tcg: use ARRAY_SIZE macro
  tests/hbitmap: use ARRAY_SIZE macro
  tests/qapi: use QEMU_IS_ALIGNED macro
  tests/qapi: use ARRAY_SIZE macro

 accel/tcg/translate-all.c           |  2 +-
 block/io.c                          |  4 +--
 block/qcow2-cache.c                 |  2 +-
 block/vhdx-log.c                    |  2 +-
 block/vvfat.c                       |  2 +-
 disas.c                             |  2 +-
 disas/ia64.c                        |  4 +--
 disas/m68k.c                        |  5 ++--
 disas/ppc.c                         | 12 +++-----
 disas/s390.c                        |  3 +-
 hw/display/sm501_template.h         |  2 +-
 hw/display/vmware_vga.c             |  5 ++--
 hw/misc/pci-testdev.c               |  4 +--
 hw/net/rocker/rocker.c              |  2 +-
 hw/s390x/sclp.c                     |  4 +--
 hw/scsi/scsi-disk.c                 |  4 +--
 hw/timer/ds1338.c                   |  4 +--
 migration/block.c                   |  2 +-
 net/eth.c                           |  2 +-
 qemu-img.c                          |  2 +-
 qga/commands-posix.c                |  2 +-
 scripts/coccinelle/use_osdep.cocci  | 60 +++++++++++++++++++++++++++++++++++++
 target/cris/translate.c             |  6 ++--
 target/lm32/translate.c             |  2 +-
 target/microblaze/op_helper.c       |  3 +-
 target/microblaze/translate.c       |  3 +-
 target/nios2/translate.c            |  2 +-
 target/sparc/cpu.c                  |  4 +--
 tests/acpi-utils.h                  |  2 +-
 tests/libqos/libqos.c               |  2 +-
 tests/tcg/alpha/test-cond.c         |  2 +-
 tests/tcg/alpha/test-ovf.c          |  2 +-
 tests/test-hbitmap.c                |  6 ++--
 tests/test-qobject-output-visitor.c |  2 +-
 tests/test-string-output-visitor.c  |  2 +-
 ui/console-gl.c                     |  2 +-
 util/aio-posix.c                    |  2 +-
 37 files changed, 114 insertions(+), 59 deletions(-)
 create mode 100644 scripts/coccinelle/use_osdep.cocci

-- 
2.13.2

[Qemu-devel] [PATCH 01/29] coccinelle: add a script to enforce qemu/osdep.h macros usage

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 scripts/coccinelle/use_osdep.cocci | 60 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)
 create mode 100644 scripts/coccinelle/use_osdep.cocci

diff --git a/scripts/coccinelle/use_osdep.cocci b/scripts/coccinelle/use_osdep.cocci
new file mode 100644
index 0000000000..356b36e358
--- /dev/null
+++ b/scripts/coccinelle/use_osdep.cocci
@@ -0,0 +1,60 @@
+// Use macros from qemu/osdep.h
+//
+// Copyright: (C) 2017 Philippe Mathieu-Daudé. GPLv2+.
+// Confidence: High
+// Options: --macro-file scripts/cocci-macro-file.h
+//
+// docker run --rm -v `pwd`:`pwd` -w `pwd` philmd/coccinelle \
+//     --macro-file scripts/cocci-macro-file.h \
+//     --sp-file scripts/coccinelle/add_osdep.cocci \
+//     --keep-comments --in-place \
+//     --use-gitgrep --dir .
+
+// Use QEMU_IS_ALIGNED()
+@@
+typedef uintptr_t;
+uintptr_t ptr;
+expression n, m;
+@@
+(
+-ptr % m == 0
++QEMU_PTR_IS_ALIGNED(ptr, m)
+|
+-n % m == 0
++QEMU_IS_ALIGNED(n, m)
+)
+
+// Use QEMU_ALIGN_DOWN()
+@@
+expression n, m;
+@@
+-n / m * m
++QEMU_ALIGN_DOWN(n, m)
+
+// Use ARRAY_SIZE()
+@@
+expression x;
+@@
+-sizeof(x) / sizeof((x)[0])
++ARRAY_SIZE(x)
+
+// Drop superfluous parenthesis
+@@
+expression n, m;
+@@
+(
+-(QEMU_IS_ALIGNED(n, m))
++QEMU_IS_ALIGNED(n, m)
+|
+-QEMU_IS_ALIGNED((n), m)
++QEMU_IS_ALIGNED(n, m)
+|
+-(QEMU_PTR_IS_ALIGNED(n, m))
++QEMU_PTR_IS_ALIGNED(n, m)
+|
+-QEMU_ALIGN_DOWN((n), m)
++QEMU_ALIGN_DOWN(n, m)
+|
+-(ARRAY_SIZE(n))
++ARRAY_SIZE(n)
+)
-- 
2.13.2

Re: [Qemu-devel] [PATCH 01/29] coccinelle: add a script to enforce qemu/osdep.h macros usage

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1589 bytes --]

On 07/18/2017 01:09 AM, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Might be worth mentioning which macros in the commit message body, so
that a grep of 'git log' spots this commit easier.

> ---
>  scripts/coccinelle/use_osdep.cocci | 60 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 60 insertions(+)
>  create mode 100644 scripts/coccinelle/use_osdep.cocci
> 
> +// docker run --rm -v `pwd`:`pwd` -w `pwd` philmd/coccinelle \
> +//     --macro-file scripts/cocci-macro-file.h \
> +//     --sp-file scripts/coccinelle/add_osdep.cocci \
> +//     --keep-comments --in-place \
> +//     --use-gitgrep --dir .

Is '--use-gitgrep' a relatively new option to spatch (not present in my
F26 coccinelle-1.0.6-7), or is it a directive specific to the docker
coccinelle package you are using?  It's somewhat nice if there is a
clean separation between which arguments are for spatch and which are
for docker.

> +
> +// Use QEMU_IS_ALIGNED()
> +@@
> +typedef uintptr_t;
> +uintptr_t ptr;
> +expression n, m;
> +@@
> +(
> +-ptr % m == 0
> ++QEMU_PTR_IS_ALIGNED(ptr, m)
> +|
> +-n % m == 0
> ++QEMU_IS_ALIGNED(n, m)
> +)

Is it worth also trying to flag '(n & (m - 1)) == 0' (for when m is a
power of 2)?

> +
> +// Use QEMU_ALIGN_DOWN()
> +@@
> +expression n, m;
> +@@
> +-n / m * m
> ++QEMU_ALIGN_DOWN(n, m)

Is it worth also trying to catch (n >> m) << m?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

[Qemu-devel] [PATCH 02/29] pci: remove superfluous parenthesis

[Philippe Mathieu-Daudé]

Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/misc/pci-testdev.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/misc/pci-testdev.c b/hw/misc/pci-testdev.c
index 7d5990213e..b928ce7e84 100644
--- a/hw/misc/pci-testdev.c
+++ b/hw/misc/pci-testdev.c
@@ -63,8 +63,8 @@ static const char *iotest_type[] = {
 
 #define IOTEST_TEST(i) (iotest_test[((i) % ARRAY_SIZE(iotest_test))])
 #define IOTEST_TYPE(i) (iotest_type[((i) / ARRAY_SIZE(iotest_test))])
-#define IOTEST_MAX_TEST (ARRAY_SIZE(iotest_test))
-#define IOTEST_MAX_TYPE (ARRAY_SIZE(iotest_type))
+#define IOTEST_MAX_TEST ARRAY_SIZE(iotest_test)
+#define IOTEST_MAX_TYPE ARRAY_SIZE(iotest_type)
 #define IOTEST_MAX (IOTEST_MAX_TEST * IOTEST_MAX_TYPE)
 
 enum {
-- 
2.13.2

Re: [Qemu-devel] [PATCH 02/29] pci: remove superfluous parenthesis

[Richard Henderson]

On 07/17/2017 08:09 PM, Philippe Mathieu-Daudé wrote:
>   #define IOTEST_TEST(i) (iotest_test[((i) % ARRAY_SIZE(iotest_test))])
>   #define IOTEST_TYPE(i) (iotest_type[((i) / ARRAY_SIZE(iotest_test))])

Looks like  array[(x)] is another candidate.

> -#define IOTEST_MAX_TEST (ARRAY_SIZE(iotest_test))
> -#define IOTEST_MAX_TYPE (ARRAY_SIZE(iotest_type))
> +#define IOTEST_MAX_TEST ARRAY_SIZE(iotest_test)
> +#define IOTEST_MAX_TYPE ARRAY_SIZE(iotest_type)


r~

[Qemu-devel] [PATCH 03/29] qemu-img: use QEMU_IS_ALIGNED macro

[Philippe Mathieu-Daudé]

Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 qemu-img.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qemu-img.c b/qemu-img.c
index 182e697f81..6f5f956302 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3804,7 +3804,7 @@ static void bench_cb(void *opaque, int ret)
         b->in_flight--;
 
         /* Time for flush? Drain queue if requested, then flush */
-        if (b->flush_interval && remaining % b->flush_interval == 0) {
+        if (b->flush_interval && QEMU_IS_ALIGNED(remaining, b->flush_interval)) {
             if (!b->in_flight || !b->drain_on_flush) {
                 BlockCompletionFunc *cb;
 
-- 
2.13.2

Re: [Qemu-devel] [PATCH 03/29] qemu-img: use QEMU_IS_ALIGNED macro

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1032 bytes --]

On 07/18/2017 01:09 AM, Philippe Mathieu-Daudé wrote:
> Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  qemu-img.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

> 
> diff --git a/qemu-img.c b/qemu-img.c
> index 182e697f81..6f5f956302 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -3804,7 +3804,7 @@ static void bench_cb(void *opaque, int ret)
>          b->in_flight--;
>  
>          /* Time for flush? Drain queue if requested, then flush */
> -        if (b->flush_interval && remaining % b->flush_interval == 0) {
> +        if (b->flush_interval && QEMU_IS_ALIGNED(remaining, b->flush_interval)) {
>              if (!b->in_flight || !b->drain_on_flush) {
>                  BlockCompletionFunc *cb;
>  
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

[Qemu-devel] [PATCH 04/29] scsi-disk: use QEMU_IS_ALIGNED macro

[Philippe Mathieu-Daudé]

Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/scsi/scsi-disk.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index a53f058621..660d068390 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2620,8 +2620,8 @@ static BlockAIOCB *scsi_block_do_sgio(SCSIBlockReq *req,
      * reads and writes that are not aligned to one logical sectors
      * _and_ cover multiple MemoryRegions.
      */
-    assert(offset % s->qdev.blocksize == 0);
-    assert(iov->size % s->qdev.blocksize == 0);
+    assert(QEMU_IS_ALIGNED(offset, s->qdev.blocksize));
+    assert(QEMU_IS_ALIGNED(iov->size, s->qdev.blocksize));
 
     io_header->interface_id = 'S';
 
-- 
2.13.2

[Qemu-devel] [PATCH 05/29] block: use QEMU_IS_ALIGNED macro

[Philippe Mathieu-Daudé]

Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 block/io.c          | 4 ++--
 block/qcow2-cache.c | 2 +-
 block/vhdx-log.c    | 2 +-
 block/vvfat.c       | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/block/io.c b/block/io.c
index aece54c015..2db571c310 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1232,7 +1232,7 @@ static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
     int max_transfer = MIN_NON_ZERO(bs->bl.max_transfer,
                                     MAX_WRITE_ZEROES_BOUNCE_BUFFER);
 
-    assert(alignment % bs->bl.request_alignment == 0);
+    assert(QEMU_IS_ALIGNED(alignment, bs->bl.request_alignment));
     head = offset % alignment;
     tail = (offset + bytes) % alignment;
     max_write_zeroes = QEMU_ALIGN_DOWN(max_write_zeroes, alignment);
@@ -2342,7 +2342,7 @@ int coroutine_fn bdrv_co_pdiscard(BlockDriverState *bs, int64_t offset,
      * unaligned requests (by returning -ENOTSUP), so we must fragment
      * the request accordingly.  */
     align = MAX(bs->bl.pdiscard_alignment, bs->bl.request_alignment);
-    assert(align % bs->bl.request_alignment == 0);
+    assert(QEMU_IS_ALIGNED(align, bs->bl.request_alignment));
     head = offset % align;
     tail = (offset + bytes) % align;
 
diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c
index 1d25147392..a4886c581e 100644
--- a/block/qcow2-cache.c
+++ b/block/qcow2-cache.c
@@ -58,7 +58,7 @@ static inline int qcow2_cache_get_table_idx(BlockDriverState *bs,
     BDRVQcow2State *s = bs->opaque;
     ptrdiff_t table_offset = (uint8_t *) table - (uint8_t *) c->table_array;
     int idx = table_offset / s->cluster_size;
-    assert(idx >= 0 && idx < c->size && table_offset % s->cluster_size == 0);
+    assert(idx >= 0 && idx < c->size && QEMU_IS_ALIGNED(table_offset, s->cluster_size));
     return idx;
 }
 
diff --git a/block/vhdx-log.c b/block/vhdx-log.c
index 01278f3fc9..bdcaa4cc16 100644
--- a/block/vhdx-log.c
+++ b/block/vhdx-log.c
@@ -282,7 +282,7 @@ static bool vhdx_log_desc_is_valid(VHDXLogDescriptor *desc,
     }
 
     if (desc->signature == VHDX_LOG_ZERO_SIGNATURE) {
-        if (desc->zero_length % VHDX_LOG_SECTOR_SIZE == 0) {
+        if (QEMU_IS_ALIGNED(desc->zero_length, VHDX_LOG_SECTOR_SIZE)) {
             /* valid */
             ret = true;
         }
diff --git a/block/vvfat.c b/block/vvfat.c
index 4dae790203..5eb827fbd6 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -454,7 +454,7 @@ static direntry_t *create_long_filename(BDRVVVFATState *s, const char *filename)
         entry=array_get(&(s->directory),s->directory.next-1-(i/26));
         if (i >= 2 * length + 2) {
             entry->name[offset] = 0xff;
-        } else if (i % 2 == 0) {
+        } else if (QEMU_IS_ALIGNED(i, 2)) {
             entry->name[offset] = longname[i / 2] & 0xff;
         } else {
             entry->name[offset] = longname[i / 2] >> 8;
-- 
2.13.2

Re: [Qemu-devel] [PATCH 05/29] block: use QEMU_IS_ALIGNED macro

[Juan Quintela]

Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Juan Quintela <quintela@redhat.com>

Re: [Qemu-devel] [PATCH 05/29] block: use QEMU_IS_ALIGNED macro

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1243 bytes --]

On 07/18/2017 01:09 AM, Philippe Mathieu-Daudé wrote:
> Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  block/io.c          | 4 ++--
>  block/qcow2-cache.c | 2 +-
>  block/vhdx-log.c    | 2 +-
>  block/vvfat.c       | 2 +-
>  4 files changed, 5 insertions(+), 5 deletions(-)

The first three look okay,

> --- a/block/vvfat.c
> +++ b/block/vvfat.c
> @@ -454,7 +454,7 @@ static direntry_t *create_long_filename(BDRVVVFATState *s, const char *filename)
>          entry=array_get(&(s->directory),s->directory.next-1-(i/26));
>          if (i >= 2 * length + 2) {
>              entry->name[offset] = 0xff;
> -        } else if (i % 2 == 0) {
> +        } else if (QEMU_IS_ALIGNED(i, 2)) {
>              entry->name[offset] = longname[i / 2] & 0xff;
>          } else {
>              entry->name[offset] = longname[i / 2] >> 8;

but this one looks a bit odd.  Code-wise, it's identical, but
semantically, this doesn't feel like an alignment check, so much as an
even-or-odd check.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

[Qemu-devel] [PATCH 06/29] migration/block: use QEMU_IS_ALIGNED macro

[Philippe Mathieu-Daudé]

Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 migration/block.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/migration/block.c b/migration/block.c
index 9171f60028..498c72ad59 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -933,7 +933,7 @@ static int block_load(QEMUFile *f, void *opaque, int version_id)
                 ret = bdrv_get_info(blk_bs(blk), &bdi);
                 if (ret == 0 && bdi.cluster_size > 0 &&
                     bdi.cluster_size <= BLOCK_SIZE &&
-                    BLOCK_SIZE % bdi.cluster_size == 0) {
+                    QEMU_IS_ALIGNED(BLOCK_SIZE, bdi.cluster_size)) {
                     cluster_size = bdi.cluster_size;
                 } else {
                     cluster_size = BLOCK_SIZE;
-- 
2.13.2

Re: [Qemu-devel] [PATCH 06/29] migration/block: use QEMU_IS_ALIGNED macro

[Juan Quintela]

Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  migration/block.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/migration/block.c b/migration/block.c
> index 9171f60028..498c72ad59 100644
> --- a/migration/block.c
> +++ b/migration/block.c
> @@ -933,7 +933,7 @@ static int block_load(QEMUFile *f, void *opaque, int version_id)
>                  ret = bdrv_get_info(blk_bs(blk), &bdi);
>                  if (ret == 0 && bdi.cluster_size > 0 &&
>                      bdi.cluster_size <= BLOCK_SIZE &&
> -                    BLOCK_SIZE % bdi.cluster_size == 0) {
> +                    QEMU_IS_ALIGNED(BLOCK_SIZE, bdi.cluster_size)) {
>                      cluster_size = bdi.cluster_size;
>                  } else {
>                      cluster_size = BLOCK_SIZE;

Reviewed-by: Juan Quintela <quintela@redhat.com>

Should I take this patch, or does it go through the block layer?  Your
call.

Later, Juan.

Re: [Qemu-devel] [PATCH 06/29] migration/block: use QEMU_IS_ALIGNED macro

[Alex Bennée]

Juan Quintela <quintela@redhat.com> writes:

> Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>> Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  migration/block.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/migration/block.c b/migration/block.c
>> index 9171f60028..498c72ad59 100644
>> --- a/migration/block.c
>> +++ b/migration/block.c
>> @@ -933,7 +933,7 @@ static int block_load(QEMUFile *f, void *opaque, int version_id)
>>                  ret = bdrv_get_info(blk_bs(blk), &bdi);
>>                  if (ret == 0 && bdi.cluster_size > 0 &&
>>                      bdi.cluster_size <= BLOCK_SIZE &&
>> -                    BLOCK_SIZE % bdi.cluster_size == 0) {
>> +                    QEMU_IS_ALIGNED(BLOCK_SIZE, bdi.cluster_size)) {
>>                      cluster_size = bdi.cluster_size;
>>                  } else {
>>                      cluster_size = BLOCK_SIZE;
>
> Reviewed-by: Juan Quintela <quintela@redhat.com>
>
> Should I take this patch, or does it go through the block layer?  Your
> call.

Given it is part of a set would it not make more sense going through
-trivial with the rest of them?

--
Alex Bennée

[Qemu-devel] [PATCH 07/29] ds1338: use QEMU_IS_ALIGNED macro

[Philippe Mathieu-Daudé]

Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/timer/ds1338.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/timer/ds1338.c b/hw/timer/ds1338.c
index 3849b74a68..a77fe14514 100644
--- a/hw/timer/ds1338.c
+++ b/hw/timer/ds1338.c
@@ -65,7 +65,7 @@ static void capture_current_time(DS1338State *s)
     s->nvram[1] = to_bcd(now.tm_min);
     if (s->nvram[2] & HOURS_12) {
         int tmp = now.tm_hour;
-        if (tmp % 12 == 0) {
+        if (QEMU_IS_ALIGNED(tmp, 12)) {
             tmp += 12;
         }
         if (tmp <= 12) {
@@ -154,7 +154,7 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
                 if (data & HOURS_PM) {
                     tmp += 12;
                 }
-                if (tmp % 12 == 0) {
+                if (QEMU_IS_ALIGNED(tmp, 12)) {
                     tmp -= 12;
                 }
                 now.tm_hour = tmp;
-- 
2.13.2

Re: [Qemu-devel] [PATCH 07/29] ds1338: use QEMU_IS_ALIGNED macro

[Alastair D'Silva]

I'm not sure this makes sense - we're dealing with time here.

-- 
Alastair D'Silva           mob: 0423 762 819
skype: alastair_dsilva     msn: alastair@d-silva.org
blog: http://alastair.d-silva.org    Twitter: @EvilDeece



> -----Original Message-----
> From: Philippe Mathieu-Daudé [mailto:philippe.mathieu.daude@gmail.com]
> On Behalf Of Philippe Mathieu-Daudé
> Sent: Tuesday, 18 July 2017 4:10 PM
> To: qemu-trivial@nongnu.org; Peter Maydell <peter.maydell@linaro.org>;
> Alastair D'Silva <alastair@d-silva.org>; Corey Minyard
> <cminyard@mvista.com>
> Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>; qemu-devel@nongnu.org;
> Eric Blake <eblake@redhat.com>
> Subject: [PATCH 07/29] ds1338: use QEMU_IS_ALIGNED macro
> 
> Applied using the Coccinelle semantic patch
> scripts/coccinelle/use_osdep.cocci
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/timer/ds1338.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/timer/ds1338.c b/hw/timer/ds1338.c index
> 3849b74a68..a77fe14514 100644
> --- a/hw/timer/ds1338.c
> +++ b/hw/timer/ds1338.c
> @@ -65,7 +65,7 @@ static void capture_current_time(DS1338State *s)
>      s->nvram[1] = to_bcd(now.tm_min);
>      if (s->nvram[2] & HOURS_12) {
>          int tmp = now.tm_hour;
> -        if (tmp % 12 == 0) {
> +        if (QEMU_IS_ALIGNED(tmp, 12)) {
>              tmp += 12;
>          }
>          if (tmp <= 12) {
> @@ -154,7 +154,7 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
>                  if (data & HOURS_PM) {
>                      tmp += 12;
>                  }
> -                if (tmp % 12 == 0) {
> +                if (QEMU_IS_ALIGNED(tmp, 12)) {
>                      tmp -= 12;
>                  }
>                  now.tm_hour = tmp;
> --
> 2.13.2
> 
> 
> ---
> This email has been checked for viruses by AVG.
> http://www.avg.com

Re: [Qemu-devel] [PATCH 07/29] ds1338: use QEMU_IS_ALIGNED macro

[Philippe Mathieu-Daudé]

On Tue, Jul 18, 2017 at 7:23 AM, Alastair D'Silva <alastair@d-silva.org> wrote:
> I'm not sure this makes sense - we're dealing with time here.

Indeed, thanks for your review!

>> --- a/hw/timer/ds1338.c
>> +++ b/hw/timer/ds1338.c
>> @@ -65,7 +65,7 @@ static void capture_current_time(DS1338State *s)
>>      s->nvram[1] = to_bcd(now.tm_min);
>>      if (s->nvram[2] & HOURS_12) {
>>          int tmp = now.tm_hour;
>> -        if (tmp % 12 == 0) {
>> +        if (QEMU_IS_ALIGNED(tmp, 12)) {
>>              tmp += 12;
>>          }
>>          if (tmp <= 12) {
>> @@ -154,7 +154,7 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
>>                  if (data & HOURS_PM) {
>>                      tmp += 12;
>>                  }
>> -                if (tmp % 12 == 0) {
>> +                if (QEMU_IS_ALIGNED(tmp, 12)) {
>>                      tmp -= 12;
>>                  }
>>                  now.tm_hour = tmp;

[Qemu-devel] [PATCH 08/29] ui: use QEMU_IS_ALIGNED macro

[Philippe Mathieu-Daudé]

Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 ui/console-gl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui/console-gl.c b/ui/console-gl.c
index 5165e21646..83ad31d223 100644
--- a/ui/console-gl.c
+++ b/ui/console-gl.c
@@ -80,7 +80,7 @@ void surface_gl_create_texture(ConsoleGLState *gls,
                                DisplaySurface *surface)
 {
     assert(gls);
-    assert(surface_stride(surface) % surface_bytes_per_pixel(surface) == 0);
+    assert(QEMU_IS_ALIGNED(surface_stride(surface), surface_bytes_per_pixel(surface)));
 
     switch (surface->format) {
     case PIXMAN_BE_b8g8r8x8:
-- 
2.13.2

[Qemu-devel] [PATCH 09/29] net: use QEMU_IS_ALIGNED macro

[Philippe Mathieu-Daudé]

Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 net/eth.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/eth.c b/net/eth.c
index 5b9ba26a56..187a6812f6 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -332,7 +332,7 @@ eth_setup_ip4_fragmentation(const void *l2hdr, size_t l2hdr_len,
         uint16_t frag_off_units = frag_offset / IP_FRAG_UNIT_SIZE;
         uint16_t new_ip_off;
 
-        assert(frag_offset % IP_FRAG_UNIT_SIZE == 0);
+        assert(QEMU_IS_ALIGNED(frag_offset, IP_FRAG_UNIT_SIZE));
         assert((frag_off_units & ~IP_OFFMASK) == 0);
 
         orig_flags = be16_to_cpu(iphdr->ip_off) & ~(IP_OFFMASK|IP_MF);
-- 
2.13.2

Re: [Qemu-devel] [PATCH 09/29] net: use QEMU_IS_ALIGNED macro

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1171 bytes --]

On 07/18/2017 01:09 AM, Philippe Mathieu-Daudé wrote:
> Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  net/eth.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/eth.c b/net/eth.c
> index 5b9ba26a56..187a6812f6 100644
> --- a/net/eth.c
> +++ b/net/eth.c
> @@ -332,7 +332,7 @@ eth_setup_ip4_fragmentation(const void *l2hdr, size_t l2hdr_len,
>          uint16_t frag_off_units = frag_offset / IP_FRAG_UNIT_SIZE;
>          uint16_t new_ip_off;
>  
> -        assert(frag_offset % IP_FRAG_UNIT_SIZE == 0);
> +        assert(QEMU_IS_ALIGNED(frag_offset, IP_FRAG_UNIT_SIZE));
>          assert((frag_off_units & ~IP_OFFMASK) == 0);

This line is also a candidate for using the macro, albeit in the form
QEMU_IS_ALIGNED(frag_off_units, IP_OFFMASK + 1), which might not make as
much sense (but it DOES re-visit my question of whether the coccinelle
script should be taught more patterns)

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

[Qemu-devel] [PATCH 10/29] net/rocker: use QEMU_IS_ALIGNED macro

[Philippe Mathieu-Daudé]

Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/net/rocker/rocker.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c
index 4f0f6d71e5..55228f2f52 100644
--- a/hw/net/rocker/rocker.c
+++ b/hw/net/rocker/rocker.c
@@ -1419,7 +1419,7 @@ static int pci_rocker_init(PCIDevice *dev)
             desc_ring_set_consume(ring, cmd_consume, ROCKER_MSIX_VEC_CMD);
         } else if (i == ROCKER_RING_EVENT) {
             desc_ring_set_consume(ring, NULL, ROCKER_MSIX_VEC_EVENT);
-        } else if (i % 2 == 0) {
+        } else if (QEMU_IS_ALIGNED(i, 2)) {
             desc_ring_set_consume(ring, tx_consume,
                                   ROCKER_MSIX_VEC_TX((i - 2) / 2));
         } else if (i % 2 == 1) {
-- 
2.13.2

Re: [Qemu-devel] [PATCH 10/29] net/rocker: use QEMU_IS_ALIGNED macro

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1178 bytes --]

On 07/18/2017 01:09 AM, Philippe Mathieu-Daudé wrote:
> Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/net/rocker/rocker.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c
> index 4f0f6d71e5..55228f2f52 100644
> --- a/hw/net/rocker/rocker.c
> +++ b/hw/net/rocker/rocker.c
> @@ -1419,7 +1419,7 @@ static int pci_rocker_init(PCIDevice *dev)
>              desc_ring_set_consume(ring, cmd_consume, ROCKER_MSIX_VEC_CMD);
>          } else if (i == ROCKER_RING_EVENT) {
>              desc_ring_set_consume(ring, NULL, ROCKER_MSIX_VEC_EVENT);
> -        } else if (i % 2 == 0) {
> +        } else if (QEMU_IS_ALIGNED(i, 2)) {
>              desc_ring_set_consume(ring, tx_consume,
>                                    ROCKER_MSIX_VEC_TX((i - 2) / 2));
>          } else if (i % 2 == 1) {
> 

Given the if chain, I think you don't want this one.


-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [Qemu-devel] [PATCH 10/29] net/rocker: use QEMU_IS_ALIGNED macro

[Philippe Mathieu-Daudé]

On 07/18/2017 02:51 PM, Eric Blake wrote:
> On 07/18/2017 01:09 AM, Philippe Mathieu-Daudé wrote:
>> Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>   hw/net/rocker/rocker.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c
>> index 4f0f6d71e5..55228f2f52 100644
>> --- a/hw/net/rocker/rocker.c
>> +++ b/hw/net/rocker/rocker.c
>> @@ -1419,7 +1419,7 @@ static int pci_rocker_init(PCIDevice *dev)
>>               desc_ring_set_consume(ring, cmd_consume, ROCKER_MSIX_VEC_CMD);
>>           } else if (i == ROCKER_RING_EVENT) {
>>               desc_ring_set_consume(ring, NULL, ROCKER_MSIX_VEC_EVENT);
>> -        } else if (i % 2 == 0) {
>> +        } else if (QEMU_IS_ALIGNED(i, 2)) {
>>               desc_ring_set_consume(ring, tx_consume,
>>                                     ROCKER_MSIX_VEC_TX((i - 2) / 2));
>>           } else if (i % 2 == 1) {
>>
> 
> Given the if chain, I think you don't want this one.

Indeed, dropped. Thanks for your review!

[Qemu-devel] [PATCH 11/29] s390x/sclp: use QEMU_IS_ALIGNED macro

[Philippe Mathieu-Daudé]

Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/s390x/sclp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index 9253dbbc64..0a207c7345 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -237,7 +237,7 @@ static void assign_storage(SCLPDevice *sclp, SCCB *sccb)
     }
     assign_addr = (assign_info->rn - 1) * mhd->rzm;
 
-    if ((assign_addr % MEM_SECTION_SIZE == 0) &&
+    if (QEMU_IS_ALIGNED(assign_addr, MEM_SECTION_SIZE) &&
         (assign_addr >= mhd->padded_ram_size)) {
         /* Re-use existing memory region if found */
         mr = memory_region_find(sysmem, assign_addr, 1).mr;
@@ -297,7 +297,7 @@ static void unassign_storage(SCLPDevice *sclp, SCCB *sccb)
     unassign_addr = (assign_info->rn - 1) * mhd->rzm;
 
     /* if the addr is a multiple of 256 MB */
-    if ((unassign_addr % MEM_SECTION_SIZE == 0) &&
+    if (QEMU_IS_ALIGNED(unassign_addr, MEM_SECTION_SIZE) &&
         (unassign_addr >= mhd->padded_ram_size)) {
         mhd->standby_state_map[(unassign_addr -
                            mhd->padded_ram_size) / MEM_SECTION_SIZE] = 0;
-- 
2.13.2

Re: [Qemu-devel] [PATCH 11/29] s390x/sclp: use QEMU_IS_ALIGNED macro

[Cornelia Huck]

On Tue, 18 Jul 2017 03:09:47 -0300
Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:

> Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/s390x/sclp.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> index 9253dbbc64..0a207c7345 100644
> --- a/hw/s390x/sclp.c
> +++ b/hw/s390x/sclp.c
> @@ -237,7 +237,7 @@ static void assign_storage(SCLPDevice *sclp, SCCB *sccb)
>      }
>      assign_addr = (assign_info->rn - 1) * mhd->rzm;
>  
> -    if ((assign_addr % MEM_SECTION_SIZE == 0) &&
> +    if (QEMU_IS_ALIGNED(assign_addr, MEM_SECTION_SIZE) &&
>          (assign_addr >= mhd->padded_ram_size)) {
>          /* Re-use existing memory region if found */
>          mr = memory_region_find(sysmem, assign_addr, 1).mr;
> @@ -297,7 +297,7 @@ static void unassign_storage(SCLPDevice *sclp, SCCB *sccb)
>      unassign_addr = (assign_info->rn - 1) * mhd->rzm;
>  
>      /* if the addr is a multiple of 256 MB */
> -    if ((unassign_addr % MEM_SECTION_SIZE == 0) &&
> +    if (QEMU_IS_ALIGNED(unassign_addr, MEM_SECTION_SIZE) &&
>          (unassign_addr >= mhd->padded_ram_size)) {
>          mhd->standby_state_map[(unassign_addr -
>                             mhd->padded_ram_size) / MEM_SECTION_SIZE] = 0;

Acked-by: Cornelia Huck <cohuck@redhat.com>

Re: [Qemu-devel] [PATCH 11/29] s390x/sclp: use QEMU_IS_ALIGNED macro

[Cornelia Huck]

On Tue, 18 Jul 2017 11:58:40 +0200
Cornelia Huck <cohuck@redhat.com> wrote:

> On Tue, 18 Jul 2017 03:09:47 -0300
> Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> 
> > Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci
> > 
> > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > ---
> >  hw/s390x/sclp.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> > index 9253dbbc64..0a207c7345 100644
> > --- a/hw/s390x/sclp.c
> > +++ b/hw/s390x/sclp.c
> > @@ -237,7 +237,7 @@ static void assign_storage(SCLPDevice *sclp, SCCB *sccb)
> >      }
> >      assign_addr = (assign_info->rn - 1) * mhd->rzm;
> >  
> > -    if ((assign_addr % MEM_SECTION_SIZE == 0) &&
> > +    if (QEMU_IS_ALIGNED(assign_addr, MEM_SECTION_SIZE) &&
> >          (assign_addr >= mhd->padded_ram_size)) {
> >          /* Re-use existing memory region if found */
> >          mr = memory_region_find(sysmem, assign_addr, 1).mr;
> > @@ -297,7 +297,7 @@ static void unassign_storage(SCLPDevice *sclp, SCCB *sccb)
> >      unassign_addr = (assign_info->rn - 1) * mhd->rzm;
> >  
> >      /* if the addr is a multiple of 256 MB */
> > -    if ((unassign_addr % MEM_SECTION_SIZE == 0) &&
> > +    if (QEMU_IS_ALIGNED(unassign_addr, MEM_SECTION_SIZE) &&
> >          (unassign_addr >= mhd->padded_ram_size)) {
> >          mhd->standby_state_map[(unassign_addr -
> >                             mhd->padded_ram_size) / MEM_SECTION_SIZE] = 0;  
> 
> Acked-by: Cornelia Huck <cohuck@redhat.com>

Oh, and I assume this will go through trivial, right?

[Qemu-devel] [PATCH 12/29] sm501: use QEMU_IS_ALIGNED macro

[Philippe Mathieu-Daudé]

Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/display/sm501_template.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/display/sm501_template.h b/hw/display/sm501_template.h
index a60abad019..6b6386bc72 100644
--- a/hw/display/sm501_template.h
+++ b/hw/display/sm501_template.h
@@ -105,7 +105,7 @@ static void glue(draw_hwc_line_, PIXEL_NAME)(uint8_t *d, const uint8_t *s,
 
     for (i = 0; i < SM501_HWC_WIDTH && c_x + i < width; i++) {
         /* get pixel value */
-        if (i % 4 == 0) {
+        if (QEMU_IS_ALIGNED(i, 4)) {
             bitset = ldub_p(s);
             s++;
         }
-- 
2.13.2

[Qemu-devel] [PATCH 13/29] tcg: use QEMU_IS_ALIGNED macro

[Philippe Mathieu-Daudé]

Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 accel/tcg/translate-all.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 4e1831cbb9..9011618157 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -151,7 +151,7 @@ static void page_table_config_init(void)
     v_l2_levels = v_l1_shift / V_L2_BITS - 1;
 
     assert(v_l1_bits <= V_L1_MAX_BITS);
-    assert(v_l1_shift % V_L2_BITS == 0);
+    assert(QEMU_IS_ALIGNED(v_l1_shift, V_L2_BITS));
     assert(v_l2_levels >= 0);
 }
 
-- 
2.13.2

[Qemu-devel] [PATCH 14/29] cris: use QEMU_IS_ALIGNED macro

[Philippe Mathieu-Daudé]

Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/cris/translate.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/cris/translate.c b/target/cris/translate.c
index 0ee05ca02d..46afdff145 100644
--- a/target/cris/translate.c
+++ b/target/cris/translate.c
@@ -3332,14 +3332,14 @@ void cris_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
 
     for (i = 0; i < 16; i++) {
         cpu_fprintf(f, "%s=%8.8x ", regnames[i], env->regs[i]);
-        if ((i + 1) % 4 == 0) {
+        if (QEMU_IS_ALIGNED(i + 1, 4)) {
             cpu_fprintf(f, "\n");
         }
     }
     cpu_fprintf(f, "\nspecial regs:\n");
     for (i = 0; i < 16; i++) {
         cpu_fprintf(f, "%s=%8.8x ", pregnames[i], env->pregs[i]);
-        if ((i + 1) % 4 == 0) {
+        if (QEMU_IS_ALIGNED(i + 1, 4)) {
             cpu_fprintf(f, "\n");
         }
     }
@@ -3350,7 +3350,7 @@ void cris_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
             for (i = 0; i < 16; i++) {
                 cpu_fprintf(f, "s%2.2d=%8.8x ",
                         i, env->sregs[srs][i]);
-                if ((i + 1) % 4 == 0) {
+                if (QEMU_IS_ALIGNED(i + 1, 4)) {
                     cpu_fprintf(f, "\n");
                 }
             }
-- 
2.13.2

[Qemu-devel] [PATCH 15/29] microblaze: use QEMU_IS_ALIGNED macro

[Philippe Mathieu-Daudé]

Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci
(also added braces to satisfy checkpatch)

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/microblaze/op_helper.c | 3 ++-
 target/microblaze/translate.c | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/target/microblaze/op_helper.c b/target/microblaze/op_helper.c
index 1e07e21c1c..41ab52fb7a 100644
--- a/target/microblaze/op_helper.c
+++ b/target/microblaze/op_helper.c
@@ -108,8 +108,9 @@ void helper_debug(CPUMBState *env)
              (env->sregs[SR_MSR] & MSR_IE));
     for (i = 0; i < 32; i++) {
         qemu_log("r%2.2d=%8.8x ", i, env->regs[i]);
-        if ((i + 1) % 4 == 0)
+        if (QEMU_IS_ALIGNED(i + 1, 4)) {
             qemu_log("\n");
+        }
     }
     qemu_log("\n\n");
 }
diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index cb65d1e129..ae7b3c71b0 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -1839,9 +1839,10 @@ void mb_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
 
     for (i = 0; i < 32; i++) {
         cpu_fprintf(f, "r%2.2d=%8.8x ", i, env->regs[i]);
-        if ((i + 1) % 4 == 0)
+        if (QEMU_IS_ALIGNED(i + 1, 4)) {
             cpu_fprintf(f, "\n");
         }
+    }
     cpu_fprintf(f, "\n\n");
 }
 
-- 
2.13.2

Re: [Qemu-devel] [PATCH 15/29] microblaze: use QEMU_IS_ALIGNED macro

[Marc-André Lureau]

On Mon, Jul 17, 2017 at 11:09 PM, Philippe Mathieu-Daudé
<f4bug@amsat.org> wrote:
> Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci
> (also added braces to satisfy checkpatch)
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  target/microblaze/op_helper.c | 3 ++-
>  target/microblaze/translate.c | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/target/microblaze/op_helper.c b/target/microblaze/op_helper.c
> index 1e07e21c1c..41ab52fb7a 100644
> --- a/target/microblaze/op_helper.c
> +++ b/target/microblaze/op_helper.c
> @@ -108,8 +108,9 @@ void helper_debug(CPUMBState *env)
>               (env->sregs[SR_MSR] & MSR_IE));
>      for (i = 0; i < 32; i++) {
>          qemu_log("r%2.2d=%8.8x ", i, env->regs[i]);
> -        if ((i + 1) % 4 == 0)
> +        if (QEMU_IS_ALIGNED(i + 1, 4)) {
>              qemu_log("\n");
> +        }
>      }
>      qemu_log("\n\n");
>  }
> diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
> index cb65d1e129..ae7b3c71b0 100644
> --- a/target/microblaze/translate.c
> +++ b/target/microblaze/translate.c
> @@ -1839,9 +1839,10 @@ void mb_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
>
>      for (i = 0; i < 32; i++) {
>          cpu_fprintf(f, "r%2.2d=%8.8x ", i, env->regs[i]);
> -        if ((i + 1) % 4 == 0)
> +        if (QEMU_IS_ALIGNED(i + 1, 4)) {
>              cpu_fprintf(f, "\n");
>          }
> +    }
>      cpu_fprintf(f, "\n\n");
>  }
>
> --
> 2.13.2
>
>



-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH 15/29] microblaze: use QEMU_IS_ALIGNED macro

[Thomas Huth]

On 18.07.2017 13:08, Marc-André Lureau wrote:
> On Mon, Jul 17, 2017 at 11:09 PM, Philippe Mathieu-Daudé
> <f4bug@amsat.org> wrote:
>> Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci
>> (also added braces to satisfy checkpatch)
[...]
>> diff --git a/target/microblaze/op_helper.c b/target/microblaze/op_helper.c
>> index 1e07e21c1c..41ab52fb7a 100644
>> --- a/target/microblaze/op_helper.c
>> +++ b/target/microblaze/op_helper.c
>> @@ -108,8 +108,9 @@ void helper_debug(CPUMBState *env)
>>               (env->sregs[SR_MSR] & MSR_IE));
>>      for (i = 0; i < 32; i++) {
>>          qemu_log("r%2.2d=%8.8x ", i, env->regs[i]);
>> -        if ((i + 1) % 4 == 0)
>> +        if (QEMU_IS_ALIGNED(i + 1, 4)) {
>>              qemu_log("\n");
>> +        }
>>      }
>>      qemu_log("\n\n");
>>  }
>> diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
>> index cb65d1e129..ae7b3c71b0 100644
>> --- a/target/microblaze/translate.c
>> +++ b/target/microblaze/translate.c
>> @@ -1839,9 +1839,10 @@ void mb_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
>>
>>      for (i = 0; i < 32; i++) {
>>          cpu_fprintf(f, "r%2.2d=%8.8x ", i, env->regs[i]);
>> -        if ((i + 1) % 4 == 0)
>> +        if (QEMU_IS_ALIGNED(i + 1, 4)) {
>>              cpu_fprintf(f, "\n");
>>          }
>> +    }
>>      cpu_fprintf(f, "\n\n");
>>  }

I'd also vote to keep the original code here ... it's about pretty
printing, not about checking the alignment of addresses.

 Thomas

[Qemu-devel] [PATCH 16/29] lm32: use QEMU_IS_ALIGNED macro

[Philippe Mathieu-Daudé]

Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/lm32/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/lm32/translate.c b/target/lm32/translate.c
index 692882f447..a9b159a6e3 100644
--- a/target/lm32/translate.c
+++ b/target/lm32/translate.c
@@ -1185,7 +1185,7 @@ void lm32_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
 
     for (i = 0; i < 32; i++) {
         cpu_fprintf(f, "r%2.2d=%8.8x ", i, env->regs[i]);
-        if ((i + 1) % 4 == 0) {
+        if (QEMU_IS_ALIGNED(i + 1, 4)) {
             cpu_fprintf(f, "\n");
         }
     }
-- 
2.13.2

Re: [Qemu-devel] [PATCH 16/29] lm32: use QEMU_IS_ALIGNED macro

[Michael Walle]

Am 2017-07-18 08:09, schrieb Philippe Mathieu-Daudé:
> Applied using the Coccinelle semantic patch 
> scripts/coccinelle/use_osdep.cocci
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

QEMU_IS_ALIGNED() sounds like it is used to check if a memory access is 
aligned. Although it does the same, the line in question is used for 
formatted output. I'm not sure if this macro should be used here.

-michael

> ---
>  target/lm32/translate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/lm32/translate.c b/target/lm32/translate.c
> index 692882f447..a9b159a6e3 100644
> --- a/target/lm32/translate.c
> +++ b/target/lm32/translate.c
> @@ -1185,7 +1185,7 @@ void lm32_cpu_dump_state(CPUState *cs, FILE *f,
> fprintf_function cpu_fprintf,
> 
>      for (i = 0; i < 32; i++) {
>          cpu_fprintf(f, "r%2.2d=%8.8x ", i, env->regs[i]);
> -        if ((i + 1) % 4 == 0) {
> +        if (QEMU_IS_ALIGNED(i + 1, 4)) {
>              cpu_fprintf(f, "\n");
>          }
>      }

Re: [Qemu-devel] [PATCH 16/29] lm32: use QEMU_IS_ALIGNED macro

[Thomas Huth]

On 18.07.2017 13:42, Michael Walle wrote:
> Am 2017-07-18 08:09, schrieb Philippe Mathieu-Daudé:
>> Applied using the Coccinelle semantic patch
>> scripts/coccinelle/use_osdep.cocci
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
> QEMU_IS_ALIGNED() sounds like it is used to check if a memory access is
> aligned. Although it does the same, the line in question is used for
> formatted output. I'm not sure if this macro should be used here.

+1

I think we should not replace every usage of % blindly. It does really
look wrong in this case here.

 Thomas

Re: [Qemu-devel] [PATCH 16/29] lm32: use QEMU_IS_ALIGNED macro

[Philippe Mathieu-Daudé]

On 07/18/2017 11:37 AM, Thomas Huth wrote:
> On 18.07.2017 13:42, Michael Walle wrote:
>> Am 2017-07-18 08:09, schrieb Philippe Mathieu-Daudé:
>>> Applied using the Coccinelle semantic patch
>>> scripts/coccinelle/use_osdep.cocci
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>
>> QEMU_IS_ALIGNED() sounds like it is used to check if a memory access is
>> aligned. Although it does the same, the line in question is used for
>> formatted output. I'm not sure if this macro should be used here.
> 
> +1
> 
> I think we should not replace every usage of % blindly. It does really
> look wrong in this case here.

Dropped, will wear my glasses next time ;)

[Qemu-devel] [PATCH 18/29] nios2: use QEMU_IS_ALIGNED macro

[Philippe Mathieu-Daudé]

Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/nios2/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/nios2/translate.c b/target/nios2/translate.c
index 2f3c2e5dfb..250ac9cce5 100644
--- a/target/nios2/translate.c
+++ b/target/nios2/translate.c
@@ -925,7 +925,7 @@ void nios2_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
 
     for (i = 0; i < NUM_CORE_REGS; i++) {
         cpu_fprintf(f, "%9s=%8.8x ", regnames[i], env->regs[i]);
-        if ((i + 1) % 4 == 0) {
+        if (QEMU_IS_ALIGNED(i + 1, 4)) {
             cpu_fprintf(f, "\n");
         }
     }
-- 
2.13.2

[Qemu-devel] [PATCH 19/29] disas: use QEMU_IS_ALIGNED macro

[Philippe Mathieu-Daudé]

Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci

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

There is no entry for this file in MAINTAINERS, should it go under TCG/Overall?

 disas.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/disas.c b/disas.c
index d335c55bbf..8b59448286 100644
--- a/disas.c
+++ b/disas.c
@@ -151,7 +151,7 @@ static int print_insn_objdump(bfd_vma pc, disassemble_info *info,
     info->read_memory_func(pc, buf, n, info);
 
     for (i = 0; i < n; ++i) {
-        if (i % 32 == 0) {
+        if (QEMU_IS_ALIGNED(i, 32)) {
             info->fprintf_func(info->stream, "\n%s: ", prefix);
         }
         info->fprintf_func(info->stream, "%02x", buf[i]);
-- 
2.13.2

Re: [Qemu-devel] [PATCH 19/29] disas: use QEMU_IS_ALIGNED macro

[Marc-André Lureau]

Hi

On Mon, Jul 17, 2017 at 11:09 PM, Philippe Mathieu-Daudé
<f4bug@amsat.org> wrote:
> Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>
> There is no entry for this file in MAINTAINERS, should it go under TCG/Overall?

That would make sense to me.

>
>  disas.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/disas.c b/disas.c
> index d335c55bbf..8b59448286 100644
> --- a/disas.c
> +++ b/disas.c
> @@ -151,7 +151,7 @@ static int print_insn_objdump(bfd_vma pc, disassemble_info *info,
>      info->read_memory_func(pc, buf, n, info);
>
>      for (i = 0; i < n; ++i) {
> -        if (i % 32 == 0) {
> +        if (QEMU_IS_ALIGNED(i, 32)) {
>              info->fprintf_func(info->stream, "\n%s: ", prefix);
>          }
>          info->fprintf_func(info->stream, "%02x", buf[i]);
> --
> 2.13.2
>
>



-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH 19/29] disas: use QEMU_IS_ALIGNED macro

[Thomas Huth]

On 18.07.2017 13:07, Marc-André Lureau wrote:
> Hi
> 
> On Mon, Jul 17, 2017 at 11:09 PM, Philippe Mathieu-Daudé
> <f4bug@amsat.org> wrote:
>> Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci
[...]
>> diff --git a/disas.c b/disas.c
>> index d335c55bbf..8b59448286 100644
>> --- a/disas.c
>> +++ b/disas.c
>> @@ -151,7 +151,7 @@ static int print_insn_objdump(bfd_vma pc, disassemble_info *info,
>>      info->read_memory_func(pc, buf, n, info);
>>
>>      for (i = 0; i < n; ++i) {
>> -        if (i % 32 == 0) {
>> +        if (QEMU_IS_ALIGNED(i, 32)) {
>>              info->fprintf_func(info->stream, "\n%s: ", prefix);
>>          }
>>          info->fprintf_func(info->stream, "%02x", buf[i]);

This looks wrong to me. QEMU_IS_ALIGNED should be used for addresses and
similar things. This part here is about pretty printing a hex dump.

The code should likely be converted to use qemu_hexdump() instead, I guess.

 Thomas

Re: [Qemu-devel] [PATCH 19/29] disas: use QEMU_IS_ALIGNED macro

[Paolo Bonzini]

On 18/07/2017 16:43, Thomas Huth wrote:
> On 18.07.2017 13:07, Marc-André Lureau wrote:
>> Hi
>>
>> On Mon, Jul 17, 2017 at 11:09 PM, Philippe Mathieu-Daudé
>> <f4bug@amsat.org> wrote:
>>> Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci
> [...]
>>> diff --git a/disas.c b/disas.c
>>> index d335c55bbf..8b59448286 100644
>>> --- a/disas.c
>>> +++ b/disas.c
>>> @@ -151,7 +151,7 @@ static int print_insn_objdump(bfd_vma pc, disassemble_info *info,
>>>      info->read_memory_func(pc, buf, n, info);
>>>
>>>      for (i = 0; i < n; ++i) {
>>> -        if (i % 32 == 0) {
>>> +        if (QEMU_IS_ALIGNED(i, 32)) {
>>>              info->fprintf_func(info->stream, "\n%s: ", prefix);
>>>          }
>>>          info->fprintf_func(info->stream, "%02x", buf[i]);
> 
> This looks wrong to me. QEMU_IS_ALIGNED should be used for addresses and
> similar things. This part here is about pretty printing a hex dump.
> 
> The code should likely be converted to use qemu_hexdump() instead, I guess.

qemu_hexdump only works with FILE*.

But I agree that QEMU_IS_ALIGNED looks weird here.  I think it should
mostly be used when the argument is a pointer, to hide the cast.  Uses
for non-pointer arguments should be decided on a one-by-one basis; "is
the first argument an address" is definitely a good thing to consider.
Another might be "is the second argument a variable", because "i % n ==
0" is less efficient than "(i & (n - 1)) == 0".

Paolo


Paolo

Re: [Qemu-devel] [PATCH 19/29] disas: use QEMU_IS_ALIGNED macro

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1082 bytes --]

On 07/23/2017 09:52 AM, Paolo Bonzini wrote:

> 
> But I agree that QEMU_IS_ALIGNED looks weird here.  I think it should
> mostly be used when the argument is a pointer, to hide the cast.  Uses
> for non-pointer arguments should be decided on a one-by-one basis; "is
> the first argument an address" is definitely a good thing to consider.
> Another might be "is the second argument a variable", because "i % n ==
> 0" is less efficient than "(i & (n - 1)) == 0".

How often is QEMU_IS_ALIGNED used on non-power-of-2?  Would it be worth
rewriting it into bit-wise ops, at the expense of declaring that any
code aligning to other values must open-code their own %?  Can we have
the compiler help us ensure that the second argument is a power-of-2?
Conversely, if the compiler is given a constant for the second argument,
and is not optimizing modulo power of two into bitwise ops (at least at
CFLAGS=-O2), then that's a compiler bug.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [Qemu-devel] [PATCH 19/29] disas: use QEMU_IS_ALIGNED macro

[Michael Tokarev]

18.07.2017 09:09, Philippe Mathieu-Daudé wrote:

> @@ -151,7 +151,7 @@ static int print_insn_objdump(bfd_vma pc, disassemble_info *info,
>      info->read_memory_func(pc, buf, n, info);
>  
>      for (i = 0; i < n; ++i) {
> -        if (i % 32 == 0) {
> +        if (QEMU_IS_ALIGNED(i, 32)) {
>              info->fprintf_func(info->stream, "\n%s: ", prefix);
>          }
>          info->fprintf_func(info->stream, "%02x", buf[i]);

This does not seem to be related to _alignment_ per se, it is
just formatting, 32 entries per line, so that output looks
more-or less compact and stays readable.  To me it is a
strange move, not so much logical.

It doesn't matter much either way, but with IS_ALIGNED, to me
at least, it looks a bit less logical than now.

Thanks,

/mjt

[Qemu-devel] [PATCH 20/29] disas: use ARRAY_SIZE macro

[Philippe Mathieu-Daudé]

Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 disas/ia64.c |  4 ++--
 disas/m68k.c |  5 ++---
 disas/ppc.c  | 12 ++++--------
 disas/s390.c |  3 +--
 4 files changed, 9 insertions(+), 15 deletions(-)

diff --git a/disas/ia64.c b/disas/ia64.c
index 140754c944..a668a49f69 100644
--- a/disas/ia64.c
+++ b/disas/ia64.c
@@ -385,7 +385,7 @@ struct ia64_operand
 /* Disassemble as unsigned decimal (instead of hex): */
 #define IA64_OPND_FLAG_DECIMAL_UNSIGNED	(1<<1)
 
-#define NELEMS(a)	((int) (sizeof (a) / sizeof (a[0])))
+#define NELEMS(a) ((int) ARRAY_SIZE(a))
 
 static const char*
 ins_rsvd (const struct ia64_operand *self ATTRIBUTE_UNUSED,
@@ -2857,7 +2857,7 @@ static const unsigned short dep257[] = {
   2170, 2173, 2274, 2284, 2327, 4135, 20616, 28866, 29018,
 };
 
-#define NELS(X) (sizeof(X)/sizeof(X[0]))
+#define NELS(X) ARRAY_SIZE(X)
 static const struct ia64_opcode_dependency
 op_dependencies[] = {
   { NELS(dep1), dep1, NELS(dep0), dep0, },
diff --git a/disas/m68k.c b/disas/m68k.c
index 61b689ef3e..9989989e02 100644
--- a/disas/m68k.c
+++ b/disas/m68k.c
@@ -4197,7 +4197,7 @@ TBL("tblunb", "tblunw", "tblunl", 0, 0),
 {"wdebug", 4,	two(0175750, 03),	two(0177770, 0xffff), "ds", mcfisa_a },
 };
 
-const int m68k_numopcodes = sizeof m68k_opcodes / sizeof m68k_opcodes[0];
+const int m68k_numopcodes = ARRAY_SIZE(m68k_opcodes);
 
 /* These aliases used to be in the above table, each one duplicating
    all of the entries for its primary exactly.  This table was
@@ -4450,8 +4450,7 @@ const struct m68k_opcode_alias m68k_opcode_aliases[] =
 
 };
 
-const int m68k_numaliases =
-  sizeof m68k_opcode_aliases / sizeof m68k_opcode_aliases[0];
+const int m68k_numaliases = ARRAY_SIZE(m68k_opcode_aliases);
 /* **** End of m68k-opc.c */
 /* **** floatformat.c from sourceware.org CVS 2005-08-14.  */
 /* IEEE floating point support routines, for GDB, the GNU Debugger.
diff --git a/disas/ppc.c b/disas/ppc.c
index 5ab9c35a84..337415b887 100644
--- a/disas/ppc.c
+++ b/disas/ppc.c
@@ -893,8 +893,7 @@ const struct powerpc_operand powerpc_operands[] =
   { 0x1, 25, NULL, NULL, PPC_OPERAND_OPTIONAL},
 };
 
-const unsigned int num_powerpc_operands = (sizeof (powerpc_operands)
-					   / sizeof (powerpc_operands[0]));
+const unsigned int num_powerpc_operands = ARRAY_SIZE(powerpc_operands);
 
 /* The functions used to insert and extract complicated operands.  */
 
@@ -5029,10 +5028,8 @@ const struct powerpc_opcode powerpc_opcodes[] = {
 
 };
 
-const int powerpc_num_opcodes =
-  sizeof (powerpc_opcodes) / sizeof (powerpc_opcodes[0]);
-\f

-/* The macro table.  This is only used by the assembler.  */
+const int powerpc_num_opcodes = ARRAY_SIZE(powerpc_opcodes);
+/* The macro table.  This is only used by the assembler. */
 
 /* The expressions of the form (-x ! 31) & (x | 31) have the value 0
    when x=0; 32-x when x is between 1 and 31; are negative if x is
@@ -5086,8 +5083,7 @@ const struct powerpc_macro powerpc_macros[] = {
 { "clrlslwi.",4,  PPCCOM,	"rlwinm. %0,%1,%3,(%2)-(%3),31-(%3)" },
 };
 
-const int powerpc_num_macros =
-  sizeof (powerpc_macros) / sizeof (powerpc_macros[0]);
+const int powerpc_num_macros = ARRAY_SIZE(powerpc_macros);
 
 
 /* This file provides several disassembler functions, all of which use
diff --git a/disas/s390.c b/disas/s390.c
index 1f167d2eaa..ffa1519622 100644
--- a/disas/s390.c
+++ b/disas/s390.c
@@ -1893,5 +1893,4 @@ static const struct s390_opcode s390_opcodes[] =
   { "pr", OP16(0x0101LL), MASK_E, INSTR_E, 3, 0},
 };
 
-static const int s390_num_opcodes =
-  sizeof (s390_opcodes) / sizeof (s390_opcodes[0]);
+static const int s390_num_opcodes = ARRAY_SIZE(s390_opcodes);
-- 
2.13.2

Re: [Qemu-devel] [PATCH 20/29] disas: use ARRAY_SIZE macro

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 4538 bytes --]

On Tue, Jul 18, 2017 at 03:09:56AM -0300, Philippe Mathieu-Daudé wrote:
> Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

ppc parts,

Acked-by: David Gibson <david@gibson.dropbear.id.au>

Although.. I note that this disas stuff is pulled in from binutils /
gas.  Are local changes going to make it harder to update from there
again?


> ---
>  disas/ia64.c |  4 ++--
>  disas/m68k.c |  5 ++---
>  disas/ppc.c  | 12 ++++--------
>  disas/s390.c |  3 +--
>  4 files changed, 9 insertions(+), 15 deletions(-)
> 
> diff --git a/disas/ia64.c b/disas/ia64.c
> index 140754c944..a668a49f69 100644
> --- a/disas/ia64.c
> +++ b/disas/ia64.c
> @@ -385,7 +385,7 @@ struct ia64_operand
>  /* Disassemble as unsigned decimal (instead of hex): */
>  #define IA64_OPND_FLAG_DECIMAL_UNSIGNED	(1<<1)
>  
> -#define NELEMS(a)	((int) (sizeof (a) / sizeof (a[0])))
> +#define NELEMS(a) ((int) ARRAY_SIZE(a))
>  
>  static const char*
>  ins_rsvd (const struct ia64_operand *self ATTRIBUTE_UNUSED,
> @@ -2857,7 +2857,7 @@ static const unsigned short dep257[] = {
>    2170, 2173, 2274, 2284, 2327, 4135, 20616, 28866, 29018,
>  };
>  
> -#define NELS(X) (sizeof(X)/sizeof(X[0]))
> +#define NELS(X) ARRAY_SIZE(X)
>  static const struct ia64_opcode_dependency
>  op_dependencies[] = {
>    { NELS(dep1), dep1, NELS(dep0), dep0, },
> diff --git a/disas/m68k.c b/disas/m68k.c
> index 61b689ef3e..9989989e02 100644
> --- a/disas/m68k.c
> +++ b/disas/m68k.c
> @@ -4197,7 +4197,7 @@ TBL("tblunb", "tblunw", "tblunl", 0, 0),
>  {"wdebug", 4,	two(0175750, 03),	two(0177770, 0xffff), "ds", mcfisa_a },
>  };
>  
> -const int m68k_numopcodes = sizeof m68k_opcodes / sizeof m68k_opcodes[0];
> +const int m68k_numopcodes = ARRAY_SIZE(m68k_opcodes);
>  
>  /* These aliases used to be in the above table, each one duplicating
>     all of the entries for its primary exactly.  This table was
> @@ -4450,8 +4450,7 @@ const struct m68k_opcode_alias m68k_opcode_aliases[] =
>  
>  };
>  
> -const int m68k_numaliases =
> -  sizeof m68k_opcode_aliases / sizeof m68k_opcode_aliases[0];
> +const int m68k_numaliases = ARRAY_SIZE(m68k_opcode_aliases);
>  /* **** End of m68k-opc.c */
>  /* **** floatformat.c from sourceware.org CVS 2005-08-14.  */
>  /* IEEE floating point support routines, for GDB, the GNU Debugger.
> diff --git a/disas/ppc.c b/disas/ppc.c
> index 5ab9c35a84..337415b887 100644
> --- a/disas/ppc.c
> +++ b/disas/ppc.c
> @@ -893,8 +893,7 @@ const struct powerpc_operand powerpc_operands[] =
>    { 0x1, 25, NULL, NULL, PPC_OPERAND_OPTIONAL},
>  };
>  
> -const unsigned int num_powerpc_operands = (sizeof (powerpc_operands)
> -					   / sizeof (powerpc_operands[0]));
> +const unsigned int num_powerpc_operands = ARRAY_SIZE(powerpc_operands);
>  
>  /* The functions used to insert and extract complicated operands.  */
>  
> @@ -5029,10 +5028,8 @@ const struct powerpc_opcode powerpc_opcodes[] = {
>  
>  };
>  
> -const int powerpc_num_opcodes =
> -  sizeof (powerpc_opcodes) / sizeof (powerpc_opcodes[0]);
> -\f
> -/* The macro table.  This is only used by the assembler.  */
> +const int powerpc_num_opcodes = ARRAY_SIZE(powerpc_opcodes);
> +/* The macro table.  This is only used by the assembler. */
>  
>  /* The expressions of the form (-x ! 31) & (x | 31) have the value 0
>     when x=0; 32-x when x is between 1 and 31; are negative if x is
> @@ -5086,8 +5083,7 @@ const struct powerpc_macro powerpc_macros[] = {
>  { "clrlslwi.",4,  PPCCOM,	"rlwinm. %0,%1,%3,(%2)-(%3),31-(%3)" },
>  };
>  
> -const int powerpc_num_macros =
> -  sizeof (powerpc_macros) / sizeof (powerpc_macros[0]);
> +const int powerpc_num_macros = ARRAY_SIZE(powerpc_macros);
>  
>  
>  /* This file provides several disassembler functions, all of which use
> diff --git a/disas/s390.c b/disas/s390.c
> index 1f167d2eaa..ffa1519622 100644
> --- a/disas/s390.c
> +++ b/disas/s390.c
> @@ -1893,5 +1893,4 @@ static const struct s390_opcode s390_opcodes[] =
>    { "pr", OP16(0x0101LL), MASK_E, INSTR_E, 3, 0},
>  };
>  
> -static const int s390_num_opcodes =
> -  sizeof (s390_opcodes) / sizeof (s390_opcodes[0]);
> +static const int s390_num_opcodes = ARRAY_SIZE(s390_opcodes);

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [Qemu-devel] [PATCH 20/29] disas: use ARRAY_SIZE macro

[Marc-André Lureau]

On Mon, Jul 17, 2017 at 11:09 PM, Philippe Mathieu-Daudé
<f4bug@amsat.org> wrote:
> Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  disas/ia64.c |  4 ++--
>  disas/m68k.c |  5 ++---
>  disas/ppc.c  | 12 ++++--------
>  disas/s390.c |  3 +--
>  4 files changed, 9 insertions(+), 15 deletions(-)
>
> diff --git a/disas/ia64.c b/disas/ia64.c
> index 140754c944..a668a49f69 100644
> --- a/disas/ia64.c
> +++ b/disas/ia64.c
> @@ -385,7 +385,7 @@ struct ia64_operand
>  /* Disassemble as unsigned decimal (instead of hex): */
>  #define IA64_OPND_FLAG_DECIMAL_UNSIGNED        (1<<1)
>
> -#define NELEMS(a)      ((int) (sizeof (a) / sizeof (a[0])))
> +#define NELEMS(a) ((int) ARRAY_SIZE(a))
>
>  static const char*
>  ins_rsvd (const struct ia64_operand *self ATTRIBUTE_UNUSED,
> @@ -2857,7 +2857,7 @@ static const unsigned short dep257[] = {
>    2170, 2173, 2274, 2284, 2327, 4135, 20616, 28866, 29018,
>  };
>
> -#define NELS(X) (sizeof(X)/sizeof(X[0]))
> +#define NELS(X) ARRAY_SIZE(X)

You may want to replace NELEMS & NELS with ARRAY_SIZE instead.

>  static const struct ia64_opcode_dependency
>  op_dependencies[] = {
>    { NELS(dep1), dep1, NELS(dep0), dep0, },
> diff --git a/disas/m68k.c b/disas/m68k.c
> index 61b689ef3e..9989989e02 100644
> --- a/disas/m68k.c
> +++ b/disas/m68k.c
> @@ -4197,7 +4197,7 @@ TBL("tblunb", "tblunw", "tblunl", 0, 0),
>  {"wdebug", 4,  two(0175750, 03),       two(0177770, 0xffff), "ds", mcfisa_a },
>  };
>
> -const int m68k_numopcodes = sizeof m68k_opcodes / sizeof m68k_opcodes[0];
> +const int m68k_numopcodes = ARRAY_SIZE(m68k_opcodes);
>
>  /* These aliases used to be in the above table, each one duplicating
>     all of the entries for its primary exactly.  This table was
> @@ -4450,8 +4450,7 @@ const struct m68k_opcode_alias m68k_opcode_aliases[] =
>
>  };
>
> -const int m68k_numaliases =
> -  sizeof m68k_opcode_aliases / sizeof m68k_opcode_aliases[0];
> +const int m68k_numaliases = ARRAY_SIZE(m68k_opcode_aliases);
>  /* **** End of m68k-opc.c */
>  /* **** floatformat.c from sourceware.org CVS 2005-08-14.  */
>  /* IEEE floating point support routines, for GDB, the GNU Debugger.
> diff --git a/disas/ppc.c b/disas/ppc.c
> index 5ab9c35a84..337415b887 100644
> --- a/disas/ppc.c
> +++ b/disas/ppc.c
> @@ -893,8 +893,7 @@ const struct powerpc_operand powerpc_operands[] =
>    { 0x1, 25, NULL, NULL, PPC_OPERAND_OPTIONAL},
>  };
>
> -const unsigned int num_powerpc_operands = (sizeof (powerpc_operands)
> -                                          / sizeof (powerpc_operands[0]));
> +const unsigned int num_powerpc_operands = ARRAY_SIZE(powerpc_operands);
>
>  /* The functions used to insert and extract complicated operands.  */
>
> @@ -5029,10 +5028,8 @@ const struct powerpc_opcode powerpc_opcodes[] = {
>
>  };
>
> -const int powerpc_num_opcodes =
> -  sizeof (powerpc_opcodes) / sizeof (powerpc_opcodes[0]);
> -
> -/* The macro table.  This is only used by the assembler.  */
> +const int powerpc_num_opcodes = ARRAY_SIZE(powerpc_opcodes);
> +/* The macro table.  This is only used by the assembler. */
>
>  /* The expressions of the form (-x ! 31) & (x | 31) have the value 0
>     when x=0; 32-x when x is between 1 and 31; are negative if x is
> @@ -5086,8 +5083,7 @@ const struct powerpc_macro powerpc_macros[] = {
>  { "clrlslwi.",4,  PPCCOM,      "rlwinm. %0,%1,%3,(%2)-(%3),31-(%3)" },
>  };
>
> -const int powerpc_num_macros =
> -  sizeof (powerpc_macros) / sizeof (powerpc_macros[0]);
> +const int powerpc_num_macros = ARRAY_SIZE(powerpc_macros);
>
>
>  /* This file provides several disassembler functions, all of which use
> diff --git a/disas/s390.c b/disas/s390.c
> index 1f167d2eaa..ffa1519622 100644
> --- a/disas/s390.c
> +++ b/disas/s390.c
> @@ -1893,5 +1893,4 @@ static const struct s390_opcode s390_opcodes[] =
>    { "pr", OP16(0x0101LL), MASK_E, INSTR_E, 3, 0},
>  };
>
> -static const int s390_num_opcodes =
> -  sizeof (s390_opcodes) / sizeof (s390_opcodes[0]);
> +static const int s390_num_opcodes = ARRAY_SIZE(s390_opcodes);
> --
> 2.13.2
>
>



-- 
Marc-André Lureau

[Qemu-devel] [PATCH 21/29] qga: use ARRAY_SIZE macro

[Philippe Mathieu-Daudé]

Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 qga/commands-posix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index d8e412275e..2a5d5cb4ba 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -896,7 +896,7 @@ static void build_guest_fsinfo_for_real_device(char const *syspath,
     if (p && sscanf(q, "%u", &host) == 1) {
         has_host = true;
         nhosts = build_hosts(syspath, p, has_ata, hosts,
-                             sizeof(hosts) / sizeof(hosts[0]), errp);
+                             ARRAY_SIZE(hosts), errp);
         if (nhosts < 0) {
             goto cleanup;
         }
-- 
2.13.2

Re: [Qemu-devel] [PATCH 21/29] qga: use ARRAY_SIZE macro

[Marc-André Lureau]

On Mon, Jul 17, 2017 at 11:09 PM, Philippe Mathieu-Daudé
<f4bug@amsat.org> wrote:
> Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  qga/commands-posix.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index d8e412275e..2a5d5cb4ba 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -896,7 +896,7 @@ static void build_guest_fsinfo_for_real_device(char const *syspath,
>      if (p && sscanf(q, "%u", &host) == 1) {
>          has_host = true;
>          nhosts = build_hosts(syspath, p, has_ata, hosts,
> -                             sizeof(hosts) / sizeof(hosts[0]), errp);
> +                             ARRAY_SIZE(hosts), errp);
>          if (nhosts < 0) {
>              goto cleanup;
>          }
> --
> 2.13.2
>
>



-- 
Marc-André Lureau

[Qemu-devel] [PATCH 22/29] vmsvga: use ARRAY_SIZE macro

[Philippe Mathieu-Daudé]

Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci

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

No entry in MAINTAINERS, should it go under Graphics or Vmware?

 hw/display/vmware_vga.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
index 4a64b41259..ff8130ff70 100644
--- a/hw/display/vmware_vga.c
+++ b/hw/display/vmware_vga.c
@@ -679,10 +679,9 @@ static void vmsvga_fifo_run(struct vmsvga_state_s *s)
             if (cursor.width > 256
                 || cursor.height > 256
                 || cursor.bpp > 32
-                || SVGA_BITMAP_SIZE(x, y)
-                    > sizeof(cursor.mask) / sizeof(cursor.mask[0])
+                || SVGA_BITMAP_SIZE(x, y) > ARRAY_SIZE(cursor.mask)
                 || SVGA_PIXMAP_SIZE(x, y, cursor.bpp)
-                    > sizeof(cursor.image) / sizeof(cursor.image[0])) {
+                    > ARRAY_SIZE(cursor.image)) {
                     goto badcmd;
             }
 
-- 
2.13.2

Re: [Qemu-devel] [PATCH 22/29] vmsvga: use ARRAY_SIZE macro

[Marc-André Lureau]

On Mon, Jul 17, 2017 at 11:09 PM, Philippe Mathieu-Daudé
<f4bug@amsat.org> wrote:
> Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>
> No entry in MAINTAINERS, should it go under Graphics or Vmware?
>
>  hw/display/vmware_vga.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
> index 4a64b41259..ff8130ff70 100644
> --- a/hw/display/vmware_vga.c
> +++ b/hw/display/vmware_vga.c
> @@ -679,10 +679,9 @@ static void vmsvga_fifo_run(struct vmsvga_state_s *s)
>              if (cursor.width > 256
>                  || cursor.height > 256
>                  || cursor.bpp > 32
> -                || SVGA_BITMAP_SIZE(x, y)
> -                    > sizeof(cursor.mask) / sizeof(cursor.mask[0])
> +                || SVGA_BITMAP_SIZE(x, y) > ARRAY_SIZE(cursor.mask)
>                  || SVGA_PIXMAP_SIZE(x, y, cursor.bpp)
> -                    > sizeof(cursor.image) / sizeof(cursor.image[0])) {
> +                    > ARRAY_SIZE(cursor.image)) {
>                      goto badcmd;
>              }
>
> --
> 2.13.2
>
>



-- 
Marc-André Lureau

[Qemu-devel] [PATCH 23/29] async: use ARRAY_SIZE macro

[Philippe Mathieu-Daudé]

Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 util/aio-posix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util/aio-posix.c b/util/aio-posix.c
index 2d51239ec6..e89fd7024a 100644
--- a/util/aio-posix.c
+++ b/util/aio-posix.c
@@ -119,7 +119,7 @@ static int aio_epoll(AioContext *ctx, GPollFD *pfds,
     }
     if (timeout <= 0 || ret > 0) {
         ret = epoll_wait(ctx->epollfd, events,
-                         sizeof(events) / sizeof(events[0]),
+                         ARRAY_SIZE(events),
                          timeout);
         if (ret <= 0) {
             goto out;
-- 
2.13.2

Re: [Qemu-devel] [PATCH 23/29] async: use ARRAY_SIZE macro

[Marc-André Lureau]

On Mon, Jul 17, 2017 at 11:09 PM, Philippe Mathieu-Daudé
<f4bug@amsat.org> wrote:
> Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  util/aio-posix.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/util/aio-posix.c b/util/aio-posix.c
> index 2d51239ec6..e89fd7024a 100644
> --- a/util/aio-posix.c
> +++ b/util/aio-posix.c
> @@ -119,7 +119,7 @@ static int aio_epoll(AioContext *ctx, GPollFD *pfds,
>      }
>      if (timeout <= 0 || ret > 0) {
>          ret = epoll_wait(ctx->epollfd, events,
> -                         sizeof(events) / sizeof(events[0]),
> +                         ARRAY_SIZE(events),
>                           timeout);
>          if (ret <= 0) {
>              goto out;
> --
> 2.13.2
>
>



-- 
Marc-André Lureau

[Qemu-devel] [PATCH 24/29] tests/acpi: use ARRAY_SIZE macro

[Philippe Mathieu-Daudé]

Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 tests/acpi-utils.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/acpi-utils.h b/tests/acpi-utils.h
index f8d87236c6..91ee0a273d 100644
--- a/tests/acpi-utils.h
+++ b/tests/acpi-utils.h
@@ -58,7 +58,7 @@ typedef struct {
     } while (0);
 
 #define ACPI_READ_ARRAY(arr, addr)                               \
-    ACPI_READ_ARRAY_PTR(arr, sizeof(arr) / sizeof(arr[0]), addr)
+    ACPI_READ_ARRAY_PTR(arr, ARRAY_SIZE(arr), addr)
 
 #define ACPI_READ_TABLE_HEADER(table, addr)                      \
     do {                                                         \
-- 
2.13.2

[Qemu-devel] [PATCH 25/29] libqos: use ARRAY_SIZE macro

[Philippe Mathieu-Daudé]

Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci

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

No entry in MAINTAINERS for tests/libqos/* does it deserve it's own entry?

 tests/libqos/libqos.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/libqos/libqos.c b/tests/libqos/libqos.c
index 6226546c28..0edb7e99ba 100644
--- a/tests/libqos/libqos.c
+++ b/tests/libqos/libqos.c
@@ -258,7 +258,7 @@ void generate_pattern(void *buffer, size_t len, size_t cycle_len)
     p = rand() % 256;
     for (i = 0; i < len; i++) {
         tx[i] = p++ % 256;
-        if (i % cycle_len == 0) {
+        if (QEMU_IS_ALIGNED(i, cycle_len)) {
             p = rand() % 256;
         }
     }
-- 
2.13.2

Re: [Qemu-devel] [PATCH 25/29] libqos: use ARRAY_SIZE macro

[Marc-André Lureau]

On Mon, Jul 17, 2017 at 11:10 PM, Philippe Mathieu-Daudé
<f4bug@amsat.org> wrote:
> Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


>
> No entry in MAINTAINERS for tests/libqos/* does it deserve it's own entry?
>

It would, do you want to candidate? :)

>  tests/libqos/libqos.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/libqos/libqos.c b/tests/libqos/libqos.c
> index 6226546c28..0edb7e99ba 100644
> --- a/tests/libqos/libqos.c
> +++ b/tests/libqos/libqos.c
> @@ -258,7 +258,7 @@ void generate_pattern(void *buffer, size_t len, size_t cycle_len)
>      p = rand() % 256;
>      for (i = 0; i < len; i++) {
>          tx[i] = p++ % 256;
> -        if (i % cycle_len == 0) {
> +        if (QEMU_IS_ALIGNED(i, cycle_len)) {
>              p = rand() % 256;
>          }
>      }
> --
> 2.13.2
>
>



-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH 25/29] libqos: use ARRAY_SIZE macro

[Laurent Vivier]

On 18/07/2017 08:10, Philippe Mathieu-Daudé wrote:
> Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> 
> No entry in MAINTAINERS for tests/libqos/* does it deserve it's own entry?
> 
>  tests/libqos/libqos.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/libqos/libqos.c b/tests/libqos/libqos.c
> index 6226546c28..0edb7e99ba 100644
> --- a/tests/libqos/libqos.c
> +++ b/tests/libqos/libqos.c
> @@ -258,7 +258,7 @@ void generate_pattern(void *buffer, size_t len, size_t cycle_len)
>      p = rand() % 256;
>      for (i = 0; i < len; i++) {
>          tx[i] = p++ % 256;
> -        if (i % cycle_len == 0) {
> +        if (QEMU_IS_ALIGNED(i, cycle_len)) {
>              p = rand() % 256;
>          }
>      }
> 

The title doesn't reflect the real change.

otherwise:

Reviewed-by: Laurent Vivier <lvivier@redhat.com>

Laurent

Re: [Qemu-devel] [PATCH 25/29] libqos: use ARRAY_SIZE macro

[Philippe Mathieu-Daudé]

On Tue, Jul 18, 2017 at 8:58 AM, Laurent Vivier <lvivier@redhat.com> wrote:
> On 18/07/2017 08:10, Philippe Mathieu-Daudé wrote:
[...]
>
> The title doesn't reflect the real change.

Good catch ;) too much rebase...

>
> otherwise:
>
> Reviewed-by: Laurent Vivier <lvivier@redhat.com>

Thanks!

>
> Laurent

[Qemu-devel] [PATCH 26/29] tests/tcg: use ARRAY_SIZE macro

[Philippe Mathieu-Daudé]

Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 tests/tcg/alpha/test-cond.c | 2 +-
 tests/tcg/alpha/test-ovf.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/tcg/alpha/test-cond.c b/tests/tcg/alpha/test-cond.c
index 74adffaa69..e5596f10b8 100644
--- a/tests/tcg/alpha/test-cond.c
+++ b/tests/tcg/alpha/test-cond.c
@@ -77,7 +77,7 @@ int main (void)
 {
   int i;
 
-  for (i = 0; i < sizeof (vectors)/sizeof(vectors[0]); i++)
+  for (i = 0; i < ARRAY_SIZE(vectors); i++)
     if ((*vectors[i].func)(vectors[i].v) != vectors[i].r) {
       write(1, "Failed\n", 7);
       return 1;
diff --git a/tests/tcg/alpha/test-ovf.c b/tests/tcg/alpha/test-ovf.c
index 01c80e7525..43fc6ebb2e 100644
--- a/tests/tcg/alpha/test-ovf.c
+++ b/tests/tcg/alpha/test-ovf.c
@@ -20,7 +20,7 @@ int main (void)
 {
   int i;
 
-  for (i = 0; i < sizeof (vectors)/sizeof(vectors[0]); i++)
+  for (i = 0; i < ARRAY_SIZE(vectors); i++)
     if ((*vectors[i].func)(vectors[i].a, vectors[i].b) != vectors[i].r) {
       write(1, "Failed\n", 7);
     }
-- 
2.13.2

Re: [Qemu-devel] [PATCH 26/29] tests/tcg: use ARRAY_SIZE macro

[Marc-André Lureau]

Hi

On Mon, Jul 17, 2017 at 11:10 PM, Philippe Mathieu-Daudé
<f4bug@amsat.org> wrote:
> Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  tests/tcg/alpha/test-cond.c | 2 +-
>  tests/tcg/alpha/test-ovf.c  | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>

I think those do not include osdep.h

(sigh, tests/tcg would need some love ;)

> diff --git a/tests/tcg/alpha/test-cond.c b/tests/tcg/alpha/test-cond.c
> index 74adffaa69..e5596f10b8 100644
> --- a/tests/tcg/alpha/test-cond.c
> +++ b/tests/tcg/alpha/test-cond.c
> @@ -77,7 +77,7 @@ int main (void)
>  {
>    int i;
>
> -  for (i = 0; i < sizeof (vectors)/sizeof(vectors[0]); i++)
> +  for (i = 0; i < ARRAY_SIZE(vectors); i++)
>      if ((*vectors[i].func)(vectors[i].v) != vectors[i].r) {
>        write(1, "Failed\n", 7);
>        return 1;
> diff --git a/tests/tcg/alpha/test-ovf.c b/tests/tcg/alpha/test-ovf.c
> index 01c80e7525..43fc6ebb2e 100644
> --- a/tests/tcg/alpha/test-ovf.c
> +++ b/tests/tcg/alpha/test-ovf.c
> @@ -20,7 +20,7 @@ int main (void)
>  {
>    int i;
>
> -  for (i = 0; i < sizeof (vectors)/sizeof(vectors[0]); i++)
> +  for (i = 0; i < ARRAY_SIZE(vectors); i++)
>      if ((*vectors[i].func)(vectors[i].a, vectors[i].b) != vectors[i].r) {
>        write(1, "Failed\n", 7);
>      }
> --
> 2.13.2
>
>



-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH 26/29] tests/tcg: use ARRAY_SIZE macro

[Alex Bennée]

Marc-André Lureau <marcandre.lureau@gmail.com> writes:

> Hi
>
> On Mon, Jul 17, 2017 at 11:10 PM, Philippe Mathieu-Daudé
> <f4bug@amsat.org> wrote:
>> Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  tests/tcg/alpha/test-cond.c | 2 +-
>>  tests/tcg/alpha/test-ovf.c  | 2 +-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>
>
> I think those do not include osdep.h
>
> (sigh, tests/tcg would need some love ;)

Pranith has had a couple of revs at this:

  Subject: [PATCH v5 00/19] Cleanup of TCG tests
  Date: Thu,  1 Dec 2016 00:14:14 -0500
  Message-Id: <20161201051433.17168-1-bobby.prani@gmail.com>

We have the cross-compilers available as a fallback via docker although
not every one can seamlessly use it for building stuff. Is getting a
working cross-compiler setup too much of a hurdle fore those that want
to build cross-arch tests?

>
>> diff --git a/tests/tcg/alpha/test-cond.c b/tests/tcg/alpha/test-cond.c
>> index 74adffaa69..e5596f10b8 100644
>> --- a/tests/tcg/alpha/test-cond.c
>> +++ b/tests/tcg/alpha/test-cond.c
>> @@ -77,7 +77,7 @@ int main (void)
>>  {
>>    int i;
>>
>> -  for (i = 0; i < sizeof (vectors)/sizeof(vectors[0]); i++)
>> +  for (i = 0; i < ARRAY_SIZE(vectors); i++)
>>      if ((*vectors[i].func)(vectors[i].v) != vectors[i].r) {
>>        write(1, "Failed\n", 7);
>>        return 1;
>> diff --git a/tests/tcg/alpha/test-ovf.c b/tests/tcg/alpha/test-ovf.c
>> index 01c80e7525..43fc6ebb2e 100644
>> --- a/tests/tcg/alpha/test-ovf.c
>> +++ b/tests/tcg/alpha/test-ovf.c
>> @@ -20,7 +20,7 @@ int main (void)
>>  {
>>    int i;
>>
>> -  for (i = 0; i < sizeof (vectors)/sizeof(vectors[0]); i++)
>> +  for (i = 0; i < ARRAY_SIZE(vectors); i++)
>>      if ((*vectors[i].func)(vectors[i].a, vectors[i].b) != vectors[i].r) {
>>        write(1, "Failed\n", 7);
>>      }
>> --
>> 2.13.2
>>
>>


--
Alex Bennée

Re: [Qemu-devel] [PATCH 26/29] tests/tcg: use ARRAY_SIZE macro

[Marc-André Lureau]

Hi

On Tue, Jul 18, 2017 at 4:16 AM, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Marc-André Lureau <marcandre.lureau@gmail.com> writes:
>
>> Hi
>>
>> On Mon, Jul 17, 2017 at 11:10 PM, Philippe Mathieu-Daudé
>> <f4bug@amsat.org> wrote:
>>> Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> ---
>>>  tests/tcg/alpha/test-cond.c | 2 +-
>>>  tests/tcg/alpha/test-ovf.c  | 2 +-
>>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>
>> I think those do not include osdep.h
>>
>> (sigh, tests/tcg would need some love ;)
>
> Pranith has had a couple of revs at this:
>
>   Subject: [PATCH v5 00/19] Cleanup of TCG tests
>   Date: Thu,  1 Dec 2016 00:14:14 -0500
>   Message-Id: <20161201051433.17168-1-bobby.prani@gmail.com>
>
> We have the cross-compilers available as a fallback via docker although
> not every one can seamlessly use it for building stuff. Is getting a
> working cross-compiler setup too much of a hurdle fore those that want
> to build cross-arch tests?

Yes, I also did some experiments in cleaning up tests/tcg
(https://github.com/elmarco/qemu/tree/tcg-tests)

The trouble is I don't see an official/automated way to run tests/tcg
tests. Which cross-compiler should I use? When I played with this,
they all had various quirks, on Fedora, Debian or crosstool-ng. I
haven't looked at the docker tests, is this running tests/tcg test
somehow?

Re: [Qemu-devel] [PATCH 26/29] tests/tcg: use ARRAY_SIZE macro

[Alex Bennée]

Marc-André Lureau <marcandre.lureau@gmail.com> writes:

> Hi
>
> On Tue, Jul 18, 2017 at 4:16 AM, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> Marc-André Lureau <marcandre.lureau@gmail.com> writes:
>>
>>> Hi
>>>
>>> On Mon, Jul 17, 2017 at 11:10 PM, Philippe Mathieu-Daudé
>>> <f4bug@amsat.org> wrote:
>>>> Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci
>>>>
>>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>> ---
>>>>  tests/tcg/alpha/test-cond.c | 2 +-
>>>>  tests/tcg/alpha/test-ovf.c  | 2 +-
>>>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>
>>> I think those do not include osdep.h
>>>
>>> (sigh, tests/tcg would need some love ;)
>>
>> Pranith has had a couple of revs at this:
>>
>>   Subject: [PATCH v5 00/19] Cleanup of TCG tests
>>   Date: Thu,  1 Dec 2016 00:14:14 -0500
>>   Message-Id: <20161201051433.17168-1-bobby.prani@gmail.com>
>>
>> We have the cross-compilers available as a fallback via docker although
>> not every one can seamlessly use it for building stuff. Is getting a
>> working cross-compiler setup too much of a hurdle fore those that want
>> to build cross-arch tests?
>
> Yes, I also did some experiments in cleaning up tests/tcg
> (https://github.com/elmarco/qemu/tree/tcg-tests)
>
> The trouble is I don't see an official/automated way to run tests/tcg
> tests. Which cross-compiler should I use? When I played with this,
> they all had various quirks, on Fedora, Debian or crosstool-ng.

Debian at least have put a lot of work into getting cross-compilers
installed as part of the native distro with good multi-arch support. We
now use plain Debian 9 for most of the QEMU cross compiles - a few older
architectures still need Debian 8 with the emdebian overlay.

> I haven't looked at the docker tests, is this running tests/tcg test
> somehow?

No but as tests/tcg are all pretty simple and have less cross-compile
dependencies than QEMU itself it is fairly trivial to invoke docker to
do a cross-build for you in-place. e.g.:

  docker run --rm -t -u (id -u) -v (pwd):(pwd) -w (pwd) qemu:debian-armhf-cross arm-linux-gnueabihf-gcc tests/tcg/hello-arm.c

This is perhaps a bad example as there is a bit-rot that needs fixing.
The alternative is to mine the users PATH for fairly standard
cross-compile prefixes which work at least on Debian systems. I recently
submitted a patch to RISU that allowed you to use either in its
build-all-arches script:

  https://git.linaro.org/people/peter.maydell/risu.git/tree/build-all-archs

--
Alex Bennée

Re: [Qemu-devel] [PATCH 26/29] tests/tcg: use ARRAY_SIZE macro

[Philippe Mathieu-Daudé]

On Tue, Jul 18, 2017 at 8:16 AM, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Pranith has had a couple of revs at this:
>
>   Subject: [PATCH v5 00/19] Cleanup of TCG tests
>   Date: Thu,  1 Dec 2016 00:14:14 -0500
>   Message-Id: <20161201051433.17168-1-bobby.prani@gmail.com>
>
> We have the cross-compilers available as a fallback via docker although
> not every one can seamlessly use it for building stuff. Is getting a
> working cross-compiler setup too much of a hurdle fore those that want
> to build cross-arch tests?

I have a WiP branch using qemu to cross-run tests it _works_ funny but
when it fails it is not funny to understand what is really buggy...
/to be continued.../

Phil.

[Qemu-devel] [PATCH 27/29] tests/hbitmap: use ARRAY_SIZE macro

[Philippe Mathieu-Daudé]

Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 tests/test-hbitmap.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/test-hbitmap.c b/tests/test-hbitmap.c
index 1acb353889..628e72122b 100644
--- a/tests/test-hbitmap.c
+++ b/tests/test-hbitmap.c
@@ -813,7 +813,7 @@ static void test_hbitmap_serialize_basic(TestHBitmapData *data,
     size_t buf_size;
     uint8_t *buf;
     uint64_t positions[] = { 0, 1, L1 - 1, L1, L2 - 1, L2, L2 + 1, L3 - 1 };
-    int num_positions = sizeof(positions) / sizeof(positions[0]);
+    int num_positions = ARRAY_SIZE(positions);
 
     hbitmap_test_init(data, L3, 0);
     g_assert(hbitmap_is_serializable(data->hb));
@@ -838,7 +838,7 @@ static void test_hbitmap_serialize_part(TestHBitmapData *data,
     size_t buf_size;
     uint8_t *buf;
     uint64_t positions[] = { 0, 1, L1 - 1, L1, L2 - 1, L2, L2 + 1, L3 - 1 };
-    int num_positions = sizeof(positions) / sizeof(positions[0]);
+    int num_positions = ARRAY_SIZE(positions);
 
     hbitmap_test_init(data, L3, 0);
     buf_size = L2;
@@ -880,7 +880,7 @@ static void test_hbitmap_serialize_zeroes(TestHBitmapData *data,
     int64_t next;
     uint64_t min_l1 = MAX(L1, 64);
     uint64_t positions[] = { 0, min_l1, L2, L3 - min_l1};
-    int num_positions = sizeof(positions) / sizeof(positions[0]);
+    int num_positions = ARRAY_SIZE(positions);
 
     hbitmap_test_init(data, L3, 0);
 
-- 
2.13.2

Re: [Qemu-devel] [PATCH 27/29] tests/hbitmap: use ARRAY_SIZE macro

[Marc-André Lureau]

On Mon, Jul 17, 2017 at 11:10 PM, Philippe Mathieu-Daudé
<f4bug@amsat.org> wrote:
> Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  tests/test-hbitmap.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tests/test-hbitmap.c b/tests/test-hbitmap.c
> index 1acb353889..628e72122b 100644
> --- a/tests/test-hbitmap.c
> +++ b/tests/test-hbitmap.c
> @@ -813,7 +813,7 @@ static void test_hbitmap_serialize_basic(TestHBitmapData *data,
>      size_t buf_size;
>      uint8_t *buf;
>      uint64_t positions[] = { 0, 1, L1 - 1, L1, L2 - 1, L2, L2 + 1, L3 - 1 };
> -    int num_positions = sizeof(positions) / sizeof(positions[0]);
> +    int num_positions = ARRAY_SIZE(positions);
>
>      hbitmap_test_init(data, L3, 0);
>      g_assert(hbitmap_is_serializable(data->hb));
> @@ -838,7 +838,7 @@ static void test_hbitmap_serialize_part(TestHBitmapData *data,
>      size_t buf_size;
>      uint8_t *buf;
>      uint64_t positions[] = { 0, 1, L1 - 1, L1, L2 - 1, L2, L2 + 1, L3 - 1 };
> -    int num_positions = sizeof(positions) / sizeof(positions[0]);
> +    int num_positions = ARRAY_SIZE(positions);
>
>      hbitmap_test_init(data, L3, 0);
>      buf_size = L2;
> @@ -880,7 +880,7 @@ static void test_hbitmap_serialize_zeroes(TestHBitmapData *data,
>      int64_t next;
>      uint64_t min_l1 = MAX(L1, 64);
>      uint64_t positions[] = { 0, min_l1, L2, L3 - min_l1};
> -    int num_positions = sizeof(positions) / sizeof(positions[0]);
> +    int num_positions = ARRAY_SIZE(positions);
>
>      hbitmap_test_init(data, L3, 0);
>
> --
> 2.13.2
>
>



-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH 27/29] tests/hbitmap: use ARRAY_SIZE macro

[John Snow]

On 07/18/2017 02:10 AM, Philippe Mathieu-Daudé wrote:
> Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: John Snow <jsnow@redhat.com>

[Qemu-devel] [PATCH 28/29] tests/qapi: use QEMU_IS_ALIGNED macro

[Philippe Mathieu-Daudé]

Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 tests/test-qobject-output-visitor.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/test-qobject-output-visitor.c b/tests/test-qobject-output-visitor.c
index 749c54065f..121ee181a0 100644
--- a/tests/test-qobject-output-visitor.c
+++ b/tests/test-qobject-output-visitor.c
@@ -558,7 +558,7 @@ static void init_native_list(UserDefNativeListUnion *cvalue)
         boolList **list = &cvalue->u.boolean.data;
         for (i = 0; i < 32; i++) {
             *list = g_new0(boolList, 1);
-            (*list)->value = (i % 3 == 0);
+            (*list)->value = QEMU_IS_ALIGNED(i, 3);
             (*list)->next = NULL;
             list = &(*list)->next;
         }
-- 
2.13.2

Re: [Qemu-devel] [PATCH 28/29] tests/qapi: use QEMU_IS_ALIGNED macro

[Marc-André Lureau]

On Mon, Jul 17, 2017 at 11:10 PM, Philippe Mathieu-Daudé
<f4bug@amsat.org> wrote:
> Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  tests/test-qobject-output-visitor.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/test-qobject-output-visitor.c b/tests/test-qobject-output-visitor.c
> index 749c54065f..121ee181a0 100644
> --- a/tests/test-qobject-output-visitor.c
> +++ b/tests/test-qobject-output-visitor.c
> @@ -558,7 +558,7 @@ static void init_native_list(UserDefNativeListUnion *cvalue)
>          boolList **list = &cvalue->u.boolean.data;
>          for (i = 0; i < 32; i++) {
>              *list = g_new0(boolList, 1);
> -            (*list)->value = (i % 3 == 0);
> +            (*list)->value = QEMU_IS_ALIGNED(i, 3);
>              (*list)->next = NULL;
>              list = &(*list)->next;
>          }
> --
> 2.13.2
>
>



-- 
Marc-André Lureau

[Qemu-devel] [PATCH 29/29] tests/qapi: use ARRAY_SIZE macro

[Philippe Mathieu-Daudé]

Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 tests/test-string-output-visitor.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/test-string-output-visitor.c b/tests/test-string-output-visitor.c
index e736db3af8..f0902dd49e 100644
--- a/tests/test-string-output-visitor.c
+++ b/tests/test-string-output-visitor.c
@@ -97,7 +97,7 @@ static void test_visitor_out_intList(TestOutputVisitorData *data,
     Error *err = NULL;
     char *str;
 
-    for (i = 0; i < sizeof(value) / sizeof(value[0]); i++) {
+    for (i = 0; i < ARRAY_SIZE(value); i++) {
         *tmp = g_malloc0(sizeof(**tmp));
         (*tmp)->value = value[i];
         tmp = &(*tmp)->next;
-- 
2.13.2

Re: [Qemu-devel] [PATCH 29/29] tests/qapi: use ARRAY_SIZE macro

[Marc-André Lureau]

On Mon, Jul 17, 2017 at 11:10 PM, Philippe Mathieu-Daudé
<f4bug@amsat.org> wrote:
> Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  tests/test-string-output-visitor.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/test-string-output-visitor.c b/tests/test-string-output-visitor.c
> index e736db3af8..f0902dd49e 100644
> --- a/tests/test-string-output-visitor.c
> +++ b/tests/test-string-output-visitor.c
> @@ -97,7 +97,7 @@ static void test_visitor_out_intList(TestOutputVisitorData *data,
>      Error *err = NULL;
>      char *str;
>
> -    for (i = 0; i < sizeof(value) / sizeof(value[0]); i++) {
> +    for (i = 0; i < ARRAY_SIZE(value); i++) {
>          *tmp = g_malloc0(sizeof(**tmp));
>          (*tmp)->value = value[i];
>          tmp = &(*tmp)->next;
> --
> 2.13.2
>
>



-- 
Marc-André Lureau