diff mbox

[2/5] coccinelle: use DIV_ROUND_UP

Message ID 20170607074632.13162-3-marcandre.lureau@redhat.com
State New
Headers show

Commit Message

Marc-André Lureau June 7, 2017, 7:46 a.m. UTC
The coccinelle/round.cocci script doesn't catch hard coded values.

I used the following script over qemu code base:

(
- ((e1) + 3) / (4)
+ DIV_ROUND_UP(e1,4)
|
- ((e1) + (3)) / (4)
+ DIV_ROUND_UP(e1,4)
|
- ((e1) + 7) / (8)
+ DIV_ROUND_UP(e1,8)
|
- ((e1) + (7)) / (8)
+ DIV_ROUND_UP(e1,8)
|
- ((e1) + 15) / (16)
+ DIV_ROUND_UP(e1,16)
|
- ((e1) + (15)) / (16)
+ DIV_ROUND_UP(e1,16)
|
- ((e1) + 31) / (32)
+ DIV_ROUND_UP(e1,32)
|
- ((e1) + (31)) / (32)
+ DIV_ROUND_UP(e1,32)
)

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 block/qed-check.c           |  3 ++-
 disas/ia64.c                |  4 ++--
 hw/char/virtio-serial-bus.c | 11 ++++++-----
 hw/display/vga.c            |  2 +-
 hw/display/virtio-gpu.c     |  4 ++--
 hw/pci/msix.c               |  4 ++--
 hw/usb/dev-hub.c            |  8 ++++----
 libdecnumber/decNumber.c    |  2 +-
 target/i386/arch_dump.c     | 25 +++++++++++++++----------
 target/ppc/kvm.c            |  4 ++--
 target/ppc/mem_helper.c     |  2 +-
 target/ppc/translate.c      |  2 +-
 ui/cursor.c                 |  2 +-
 ui/vnc-enc-tight.c          |  2 +-
 ui/vnc.c                    |  3 ++-
 15 files changed, 43 insertions(+), 35 deletions(-)

Comments

Peter Maydell June 7, 2017, 9:13 a.m. UTC | #1
On 7 June 2017 at 08:46, Marc-André Lureau <marcandre.lureau@redhat.com> wrote:
> The coccinelle/round.cocci script doesn't catch hard coded values.
>
> I used the following script over qemu code base:
>
> (
> - ((e1) + 3) / (4)
> + DIV_ROUND_UP(e1,4)
> |
> - ((e1) + (3)) / (4)
> + DIV_ROUND_UP(e1,4)

Why do we need both of these? Is it just "coccinelle is weird" ? :-)

> |
> - ((e1) + 7) / (8)
> + DIV_ROUND_UP(e1,8)
> |
> - ((e1) + (7)) / (8)
> + DIV_ROUND_UP(e1,8)
> |
> - ((e1) + 15) / (16)
> + DIV_ROUND_UP(e1,16)
> |
> - ((e1) + (15)) / (16)
> + DIV_ROUND_UP(e1,16)
> |
> - ((e1) + 31) / (32)
> + DIV_ROUND_UP(e1,32)
> |
> - ((e1) + (31)) / (32)
> + DIV_ROUND_UP(e1,32)
> )

> -                     next_op = op_pointer + ((oplen + 7) / 8);
> +                     next_op = op_pointer + (DIV_ROUND_UP(oplen, 8));

I think there's a coccinelle trick for making it drop
now-unnecessary brackets in substitutions like this, but I forget
what it is. Maybe it's as simple as having substitutions for

> - (((e1) + 7) / (8))
> + DIV_ROUND_UP(e1,8)

as well?

thanks
-- PMM
Marc-André Lureau June 7, 2017, 9:27 a.m. UTC | #2
Hi

----- Original Message -----
> On 7 June 2017 at 08:46, Marc-André Lureau <marcandre.lureau@redhat.com>
> wrote:
> > The coccinelle/round.cocci script doesn't catch hard coded values.
> >
> > I used the following script over qemu code base:
> >
> > (
> > - ((e1) + 3) / (4)
> > + DIV_ROUND_UP(e1,4)
> > |
> > - ((e1) + (3)) / (4)
> > + DIV_ROUND_UP(e1,4)
> 
> Why do we need both of these? Is it just "coccinelle is weird" ? :-)

I am total newbie to coccinnelle-land, but I think this one is useless duplication

> 
> > |
> > - ((e1) + 7) / (8)
> > + DIV_ROUND_UP(e1,8)
> > |
> > - ((e1) + (7)) / (8)
> > + DIV_ROUND_UP(e1,8)
> > |
> > - ((e1) + 15) / (16)
> > + DIV_ROUND_UP(e1,16)
> > |
> > - ((e1) + (15)) / (16)
> > + DIV_ROUND_UP(e1,16)
> > |
> > - ((e1) + 31) / (32)
> > + DIV_ROUND_UP(e1,32)
> > |
> > - ((e1) + (31)) / (32)
> > + DIV_ROUND_UP(e1,32)
> > )
> 
> > -                     next_op = op_pointer + ((oplen + 7) / 8);
> > +                     next_op = op_pointer + (DIV_ROUND_UP(oplen, 8));
> 
> I think there's a coccinelle trick for making it drop
> now-unnecessary brackets in substitutions like this, but I forget
> what it is. Maybe it's as simple as having substitutions for
> 

> > - (((e1) + 7) / (8))
> > + DIV_ROUND_UP(e1,8)
> 
> as well?


I think you need a second rule:
@@
expression e1;
expression e2;
@@
-(DIV_ROUND_UP(e1,e2))
+DIV_ROUND_UP(e1,e2)


I will fix it in second version.
Eric Blake June 7, 2017, 9:50 p.m. UTC | #3
On 06/07/2017 02:46 AM, Marc-André Lureau wrote:
> The coccinelle/round.cocci script doesn't catch hard coded values.
> 
> I used the following script over qemu code base:
> 
> (
> - ((e1) + 3) / (4)
> + DIV_ROUND_UP(e1,4)

As in 1/5, can't you also write a search for ((e1) + (e2) - 1) / e2, to
cover non-constant divisions?
Marc-André Lureau June 8, 2017, 7:37 a.m. UTC | #4
Hi

On Thu, Jun 8, 2017 at 1:50 AM Eric Blake <eblake@redhat.com> wrote:

> On 06/07/2017 02:46 AM, Marc-André Lureau wrote:
> > The coccinelle/round.cocci script doesn't catch hard coded values.
> >
> > I used the following script over qemu code base:
> >
> > (
> > - ((e1) + 3) / (4)
> > + DIV_ROUND_UP(e1,4)
>
> As in 1/5, can't you also write a search for ((e1) + (e2) - 1) / e2, to
> cover non-constant divisions?
>

That one is covered by scripts/coccinelle/round.cocci

I can merge my script there, but I would need to spend some time to clean
it up (help welcome).
diff mbox

Patch

diff --git a/block/qed-check.c b/block/qed-check.c
index dcd4f036b8..f447053d24 100644
--- a/block/qed-check.c
+++ b/block/qed-check.c
@@ -228,7 +228,8 @@  int qed_check(BDRVQEDState *s, BdrvCheckResult *result, bool fix)
     };
     int ret;
 
-    check.used_clusters = g_try_new0(uint32_t, (check.nclusters + 31) / 32);
+    check.used_clusters = g_try_new0(uint32_t,
+                                     DIV_ROUND_UP(check.nclusters, 32));
     if (check.nclusters && check.used_clusters == NULL) {
         return -ENOMEM;
     }
diff --git a/disas/ia64.c b/disas/ia64.c
index 140754c944..bf576d3099 100644
--- a/disas/ia64.c
+++ b/disas/ia64.c
@@ -10156,14 +10156,14 @@  locate_opcode_ent (ia64_insn opcode, enum ia64_insn_type type)
 		    }
 		  if (x > count)
 		    {
-		      next_op = op_pointer + ((oplen + 7) / 8);
+		      next_op = op_pointer + (DIV_ROUND_UP(oplen, 8));
 		      currbitnum -= count;
 		      break;
 		    }
 		}
 	      else if (! currbit)
 		{
-		  next_op = op_pointer + ((oplen + 7) / 8);
+		  next_op = op_pointer + (DIV_ROUND_UP(oplen, 8));
 		  break;
 		}
 	    }
diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index f5bc173844..823e1c915c 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -663,7 +663,7 @@  static void virtio_serial_save_device(VirtIODevice *vdev, QEMUFile *f)
 
     /* The ports map */
     max_nr_ports = s->serial.max_virtserial_ports;
-    for (i = 0; i < (max_nr_ports + 31) / 32; i++) {
+    for (i = 0; i < DIV_ROUND_UP(max_nr_ports, 32); i++) {
         qemu_put_be32s(f, &s->ports_map[i]);
     }
 
@@ -798,7 +798,7 @@  static int virtio_serial_load_device(VirtIODevice *vdev, QEMUFile *f,
     qemu_get_be32s(f, &tmp);
 
     max_nr_ports = s->serial.max_virtserial_ports;
-    for (i = 0; i < (max_nr_ports + 31) / 32; i++) {
+    for (i = 0; i < DIV_ROUND_UP(max_nr_ports, 32); i++) {
         qemu_get_be32s(f, &ports_map);
 
         if (ports_map != s->ports_map[i]) {
@@ -863,7 +863,7 @@  static uint32_t find_free_port_id(VirtIOSerial *vser)
     unsigned int i, max_nr_ports;
 
     max_nr_ports = vser->serial.max_virtserial_ports;
-    for (i = 0; i < (max_nr_ports + 31) / 32; i++) {
+    for (i = 0; i < DIV_ROUND_UP(max_nr_ports, 32); i++) {
         uint32_t map, zeroes;
 
         map = vser->ports_map[i];
@@ -1075,8 +1075,9 @@  static void virtio_serial_device_realize(DeviceState *dev, Error **errp)
         vser->ovqs[i] = virtio_add_queue(vdev, 128, handle_output);
     }
 
-    vser->ports_map = g_malloc0(((vser->serial.max_virtserial_ports + 31) / 32)
-        * sizeof(vser->ports_map[0]));
+    vser->ports_map =
+        g_malloc0(DIV_ROUND_UP(vser->serial.max_virtserial_ports, 32)
+                  * sizeof(vser->ports_map[0]));
     /*
      * Reserve location 0 for a console port for backward compat
      * (old kernel, new qemu)
diff --git a/hw/display/vga.c b/hw/display/vga.c
index dcc95f88e2..c2d3e8f54b 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -1621,7 +1621,7 @@  static void vga_draw_graphic(VGACommonState *s, int full_update)
            s->line_compare, sr(s, VGA_SEQ_CLOCK_MODE));
 #endif
     addr1 = (s->start_addr * 4);
-    bwidth = (width * bits + 7) / 8;
+    bwidth = DIV_ROUND_UP(width * bits, 8);
     y_start = -1;
     d = surface_data(surface);
     linesize = surface_stride(surface);
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 58dc0b2737..641f57e7c5 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -408,7 +408,7 @@  static void virtio_gpu_transfer_to_host_2d(VirtIOGPU *g,
     }
 
     format = pixman_image_get_format(res->image);
-    bpp = (PIXMAN_FORMAT_BPP(format) + 7) / 8;
+    bpp = DIV_ROUND_UP(PIXMAN_FORMAT_BPP(format), 8);
     stride = pixman_image_get_stride(res->image);
 
     if (t2d.offset || t2d.r.x || t2d.r.y ||
@@ -570,7 +570,7 @@  static void virtio_gpu_set_scanout(VirtIOGPU *g,
     scanout = &g->scanout[ss.scanout_id];
 
     format = pixman_image_get_format(res->image);
-    bpp = (PIXMAN_FORMAT_BPP(format) + 7) / 8;
+    bpp = DIV_ROUND_UP(PIXMAN_FORMAT_BPP(format), 8);
     offset = (ss.r.x * bpp) + ss.r.y * pixman_image_get_stride(res->image);
     if (!scanout->ds || surface_data(scanout->ds)
         != ((uint8_t *)pixman_image_get_data(res->image) + offset) ||
diff --git a/hw/pci/msix.c b/hw/pci/msix.c
index bb54e8b0ac..4af09afe6b 100644
--- a/hw/pci/msix.c
+++ b/hw/pci/msix.c
@@ -431,7 +431,7 @@  void msix_save(PCIDevice *dev, QEMUFile *f)
     }
 
     qemu_put_buffer(f, dev->msix_table, n * PCI_MSIX_ENTRY_SIZE);
-    qemu_put_buffer(f, dev->msix_pba, (n + 7) / 8);
+    qemu_put_buffer(f, dev->msix_pba, DIV_ROUND_UP(n, 8));
 }
 
 /* Should be called after restoring the config space. */
@@ -446,7 +446,7 @@  void msix_load(PCIDevice *dev, QEMUFile *f)
 
     msix_clear_all_vectors(dev);
     qemu_get_buffer(f, dev->msix_table, n * PCI_MSIX_ENTRY_SIZE);
-    qemu_get_buffer(f, dev->msix_pba, (n + 7) / 8);
+    qemu_get_buffer(f, dev->msix_pba, DIV_ROUND_UP(n, 8));
     msix_update_function_masked(dev);
 
     for (vector = 0; vector < n; vector++) {
diff --git a/hw/usb/dev-hub.c b/hw/usb/dev-hub.c
index e82a6a6c44..c79d7bc030 100644
--- a/hw/usb/dev-hub.c
+++ b/hw/usb/dev-hub.c
@@ -109,7 +109,7 @@  static const USBDescIface desc_iface_hub = {
         {
             .bEndpointAddress      = USB_DIR_IN | 0x01,
             .bmAttributes          = USB_ENDPOINT_XFER_INT,
-            .wMaxPacketSize        = 1 + (NUM_PORTS + 7) / 8,
+            .wMaxPacketSize        = 1 + DIV_ROUND_UP(NUM_PORTS, 8),
             .bInterval             = 0xff,
         },
     }
@@ -442,14 +442,14 @@  static void usb_hub_handle_control(USBDevice *dev, USBPacket *p,
             data[2] = NUM_PORTS;
 
             /* fill DeviceRemovable bits */
-            limit = ((NUM_PORTS + 1 + 7) / 8) + 7;
+            limit = (DIV_ROUND_UP(NUM_PORTS + 1, 8)) + 7;
             for (n = 7; n < limit; n++) {
                 data[n] = 0x00;
                 var_hub_size++;
             }
 
             /* fill PortPwrCtrlMask bits */
-            limit = limit + ((NUM_PORTS + 7) / 8);
+            limit = limit + (DIV_ROUND_UP(NUM_PORTS, 8));
             for (;n < limit; n++) {
                 data[n] = 0xff;
                 var_hub_size++;
@@ -477,7 +477,7 @@  static void usb_hub_handle_data(USBDevice *dev, USBPacket *p)
             unsigned int status;
             uint8_t buf[4];
             int i, n;
-            n = (NUM_PORTS + 1 + 7) / 8;
+            n = DIV_ROUND_UP(NUM_PORTS + 1, 8);
             if (p->iov.size == 1) { /* FreeBSD workaround */
                 n = 1;
             } else if (n > p->iov.size) {
diff --git a/libdecnumber/decNumber.c b/libdecnumber/decNumber.c
index c9e7807f87..9667d27c7c 100644
--- a/libdecnumber/decNumber.c
+++ b/libdecnumber/decNumber.c
@@ -5021,7 +5021,7 @@  static decNumber * decMultiplyOp(decNumber *res, const decNumber *lhs,
       /* to the right to avoid overwrite during the unchunking. */
       needbytes=iacc*sizeof(uLong);
       #if DECDPUN==1
-      zoff=(iacc+7)/8;	      /* items to offset by */
+      zoff=DIV_ROUND_UP(iacc, 8);	      /* items to offset by */
       needbytes+=zoff*8;
       #endif
       if (needbytes>(Int)sizeof(zaccbuff)) {
diff --git a/target/i386/arch_dump.c b/target/i386/arch_dump.c
index e6788250b8..158e056b59 100644
--- a/target/i386/arch_dump.c
+++ b/target/i386/arch_dump.c
@@ -77,8 +77,9 @@  static int x86_64_write_elf64_note(WriteCoreDumpFunction f,
     regs.gs = env->segs[R_GS].selector;
 
     descsz = sizeof(x86_64_elf_prstatus);
-    note_size = ((sizeof(Elf64_Nhdr) + 3) / 4 + (name_size + 3) / 4 +
-                (descsz + 3) / 4) * 4;
+    note_size = (DIV_ROUND_UP(sizeof(Elf64_Nhdr), 4)
+                 + DIV_ROUND_UP(name_size, 4)
+                 + DIV_ROUND_UP(descsz, 4)) * 4;
     note = g_malloc0(note_size);
     note->n_namesz = cpu_to_le32(name_size);
     note->n_descsz = cpu_to_le32(descsz);
@@ -156,8 +157,9 @@  static int x86_write_elf64_note(WriteCoreDumpFunction f, CPUX86State *env,
 
     x86_fill_elf_prstatus(&prstatus, env, id);
     descsz = sizeof(x86_elf_prstatus);
-    note_size = ((sizeof(Elf64_Nhdr) + 3) / 4 + (name_size + 3) / 4 +
-                (descsz + 3) / 4) * 4;
+    note_size = (DIV_ROUND_UP(sizeof(Elf64_Nhdr), 4)
+                 + DIV_ROUND_UP(name_size, 4)
+                 + DIV_ROUND_UP(descsz, 4)) * 4;
     note = g_malloc0(note_size);
     note->n_namesz = cpu_to_le32(name_size);
     note->n_descsz = cpu_to_le32(descsz);
@@ -211,8 +213,9 @@  int x86_cpu_write_elf32_note(WriteCoreDumpFunction f, CPUState *cs,
 
     x86_fill_elf_prstatus(&prstatus, &cpu->env, cpuid);
     descsz = sizeof(x86_elf_prstatus);
-    note_size = ((sizeof(Elf32_Nhdr) + 3) / 4 + (name_size + 3) / 4 +
-                (descsz + 3) / 4) * 4;
+    note_size = (DIV_ROUND_UP(sizeof(Elf32_Nhdr), 4)
+                 + DIV_ROUND_UP(name_size, 4)
+                 + DIV_ROUND_UP(descsz, 4)) * 4;
     note = g_malloc0(note_size);
     note->n_namesz = cpu_to_le32(name_size);
     note->n_descsz = cpu_to_le32(descsz);
@@ -443,10 +446,12 @@  ssize_t cpu_get_note_size(int class, int machine, int nr_cpus)
 #endif
     qemu_desc_size = sizeof(QEMUCPUState);
 
-    elf_note_size = ((note_head_size + 3) / 4 + (name_size + 3) / 4 +
-                     (elf_desc_size + 3) / 4) * 4;
-    qemu_note_size = ((note_head_size + 3) / 4 + (name_size + 3) / 4 +
-                      (qemu_desc_size + 3) / 4) * 4;
+    elf_note_size = (DIV_ROUND_UP(note_head_size, 4)
+                     + DIV_ROUND_UP(name_size, 4)
+                     + DIV_ROUND_UP(elf_desc_size, 4)) * 4;
+    qemu_note_size = (DIV_ROUND_UP(note_head_size, 4)
+                      + DIV_ROUND_UP(name_size, 4)
+                      + DIV_ROUND_UP(qemu_desc_size, 4)) * 4;
 
     return (elf_note_size + qemu_note_size) * nr_cpus;
 }
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 51249ce79e..3b712a0434 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -601,8 +601,8 @@  static void kvm_sw_tlb_put(PowerPCCPU *cpu)
         return;
     }
 
-    bitmap = g_malloc((env->nb_tlb + 7) / 8);
-    memset(bitmap, 0xFF, (env->nb_tlb + 7) / 8);
+    bitmap = g_malloc(DIV_ROUND_UP(env->nb_tlb, 8));
+    memset(bitmap, 0xFF, DIV_ROUND_UP(env->nb_tlb, 8));
 
     dirty_tlb.bitmap = (uintptr_t)bitmap;
     dirty_tlb.num_dirty = env->nb_tlb;
diff --git a/target/ppc/mem_helper.c b/target/ppc/mem_helper.c
index e6383c6bfa..a34e604db3 100644
--- a/target/ppc/mem_helper.c
+++ b/target/ppc/mem_helper.c
@@ -111,7 +111,7 @@  void helper_lswx(CPUPPCState *env, target_ulong addr, uint32_t reg,
                  uint32_t ra, uint32_t rb)
 {
     if (likely(xer_bc != 0)) {
-        int num_used_regs = (xer_bc + 3) / 4;
+        int num_used_regs = DIV_ROUND_UP(xer_bc, 4);
         if (unlikely((ra != 0 && lsw_reg_in_range(reg, num_used_regs, ra)) ||
                      lsw_reg_in_range(reg, num_used_regs, rb))) {
             raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM,
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index c0cd64d927..76f9ccde25 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -2882,7 +2882,7 @@  static void gen_lswi(DisasContext *ctx)
     }
     if (nb == 0)
         nb = 32;
-    nr = (nb + 3) / 4;
+    nr = DIV_ROUND_UP(nb, 4);
     if (unlikely(lsw_reg_in_range(start, nr, ra))) {
         gen_inval_exception(ctx, POWERPC_EXCP_INVAL_LSWX);
         return;
diff --git a/ui/cursor.c b/ui/cursor.c
index 5155b392e8..2e2fe13fa6 100644
--- a/ui/cursor.c
+++ b/ui/cursor.c
@@ -118,7 +118,7 @@  void cursor_put(QEMUCursor *c)
 
 int cursor_get_mono_bpl(QEMUCursor *c)
 {
-    return (c->width + 7) / 8;
+    return DIV_ROUND_UP(c->width, 8);
 }
 
 void cursor_set_mono(QEMUCursor *c,
diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index 1e53b1cf84..15a49ee53d 100644
--- a/ui/vnc-enc-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -980,7 +980,7 @@  static int send_mono_rect(VncState *vs, int x, int y,
     }
 #endif
 
-    bytes = ((w + 7) / 8) * h;
+    bytes = (DIV_ROUND_UP(w, 8)) * h;
 
     vnc_write_u8(vs, (stream | VNC_TIGHT_EXPLICIT_FILTER) << 4);
     vnc_write_u8(vs, VNC_TIGHT_FILTER_PALETTE);
diff --git a/ui/vnc.c b/ui/vnc.c
index 47b49c7318..e0952441fc 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2782,7 +2782,8 @@  static int vnc_refresh_server_surface(VncDisplay *vd)
             PIXMAN_FORMAT_BPP(pixman_image_get_format(vd->guest.fb));
         guest_row0 = (uint8_t *)pixman_image_get_data(vd->guest.fb);
         guest_stride = pixman_image_get_stride(vd->guest.fb);
-        guest_ll = pixman_image_get_width(vd->guest.fb) * ((guest_bpp + 7) / 8);
+        guest_ll = pixman_image_get_width(vd->guest.fb)
+            * DIV_ROUND_UP(guest_bpp, 8);
     }
     line_bytes = MIN(server_stride, guest_ll);