diff mbox

[2/3] block: Use g_new() & friends to avoid multiplying sizes

Message ID 1401467438-17189-3-git-send-email-armbru@redhat.com
State New
Headers show

Commit Message

Markus Armbruster May 30, 2014, 4:30 p.m. UTC 
g_new(T, n) is safer than g_malloc(sizeof(*v) * n) for two reasons.
One, it catches multiplication overflowing size_t.  Two, it returns
T * rather than void *, which lets the compiler catch more type
errors.

Perhaps a conversion to g_malloc_n() would be neater in places,
especially where we do "v = g_malloc(sizeof(*v) * n)", but it's merely
four years old, and we can't use such newfangled stuff.

This commit does not touch allocations with size arguments that don't
use sizeof.  We can make them safe by converting to g_malloc_n() when
the newfangled stuff becomes available to us in a couple of years.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block/bochs.c            | 2 +-
 block/parallels.c        | 2 +-
 block/qcow2-cache.c      | 2 +-
 block/qed-check.c        | 3 +--
 block/rbd.c              | 2 +-
 block/sheepdog.c         | 2 +-
 hw/block/dataplane/ioq.c | 4 ++--
 hw/block/nvme.c          | 8 ++++----
 8 files changed, 12 insertions(+), 13 deletions(-)

Comments

Max Reitz May 30, 2014, 6:25 p.m. UTC | #1 
On 30.05.2014 18:30, Markus Armbruster wrote:
> g_new(T, n) is safer than g_malloc(sizeof(*v) * n) for two reasons.
> One, it catches multiplication overflowing size_t.  Two, it returns
> T * rather than void *, which lets the compiler catch more type
> errors.
>
> Perhaps a conversion to g_malloc_n() would be neater in places,
> especially where we do "v = g_malloc(sizeof(*v) * n)", but it's merely
> four years old, and we can't use such newfangled stuff.
>
> This commit does not touch allocations with size arguments that don't
> use sizeof.  We can make them safe by converting to g_malloc_n() when
> the newfangled stuff becomes available to us in a couple of years.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   block/bochs.c            | 2 +-
>   block/parallels.c        | 2 +-
>   block/qcow2-cache.c      | 2 +-
>   block/qed-check.c        | 3 +--
>   block/rbd.c              | 2 +-
>   block/sheepdog.c         | 2 +-
>   hw/block/dataplane/ioq.c | 4 ++--
>   hw/block/nvme.c          | 8 ++++----
>   8 files changed, 12 insertions(+), 13 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>
diff mbox

Patch

diff --git a/block/bochs.c b/block/bochs.c
index 6674b27..199ac2b 100644
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -131,7 +131,7 @@  static int bochs_open(BlockDriverState *bs, QDict *options, int flags,
         return -EFBIG;
     }
 
-    s->catalog_bitmap = g_try_malloc(s->catalog_size * 4);
+    s->catalog_bitmap = g_try_new(uint32_t, s->catalog_size);
     if (s->catalog_size && s->catalog_bitmap == NULL) {
         error_setg(errp, "Could not allocate memory for catalog");
         return -ENOMEM;
diff --git a/block/parallels.c b/block/parallels.c
index 7325678..7667a50 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -105,7 +105,7 @@  static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
         ret = -EFBIG;
         goto fail;
     }
-    s->catalog_bitmap = g_try_malloc(s->catalog_size * 4);
+    s->catalog_bitmap = g_try_new(uint32_t, s->catalog_size);
     if (s->catalog_size && s->catalog_bitmap == NULL) {
         ret = -ENOMEM;
         goto fail;
diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c
index 351bc01..9409faf 100644
--- a/block/qcow2-cache.c
+++ b/block/qcow2-cache.c
@@ -50,7 +50,7 @@  Qcow2Cache *qcow2_cache_create(BlockDriverState *bs, int num_tables)
 
     c = g_malloc0(sizeof(*c));
     c->size = num_tables;
-    c->entries = g_malloc0(sizeof(*c->entries) * num_tables);
+    c->entries = g_new0(Qcow2CachedTable, num_tables);
 
     for (i = 0; i < c->size; i++) {
         c->entries[i].table = qemu_try_blockalign(bs, s->cluster_size);
diff --git a/block/qed-check.c b/block/qed-check.c
index 40a882c..36ecd29 100644
--- a/block/qed-check.c
+++ b/block/qed-check.c
@@ -227,8 +227,7 @@  int qed_check(BDRVQEDState *s, BdrvCheckResult *result, bool fix)
     };
     int ret;
 
-    check.used_clusters = g_try_malloc0(((check.nclusters + 31) / 32) *
-                                        sizeof(check.used_clusters[0]));
+    check.used_clusters = g_try_new0(uint32_t, (check.nclusters + 31) / 32);
     if (check.nclusters && check.used_clusters == NULL) {
         return -ENOMEM;
     }
diff --git a/block/rbd.c b/block/rbd.c
index d6d9955..9a3fbae 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -865,7 +865,7 @@  static int qemu_rbd_snap_list(BlockDriverState *bs,
     int max_snaps = RBD_MAX_SNAPS;
 
     do {
-        snaps = g_malloc(sizeof(*snaps) * max_snaps);
+        snaps = g_new(rbd_snap_info_t, max_snaps);
         snap_count = rbd_snap_list(s->image, snaps, &max_snaps);
         if (snap_count <= 0) {
             g_free(snaps);
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 1eb1fd0..0709fd0 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -2318,7 +2318,7 @@  static int sd_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab)
         goto out;
     }
 
-    sn_tab = g_malloc0(nr * sizeof(*sn_tab));
+    sn_tab = g_new0(QEMUSnapshotInfo, nr);
 
     /* calculate a vdi id with hash function */
     hval = fnv_64a_buf(s->name, strlen(s->name), FNV1A_64_INIT);
diff --git a/hw/block/dataplane/ioq.c b/hw/block/dataplane/ioq.c
index f709f87..626d7a3 100644
--- a/hw/block/dataplane/ioq.c
+++ b/hw/block/dataplane/ioq.c
@@ -34,10 +34,10 @@  void ioq_init(IOQueue *ioq, int fd, unsigned int max_reqs)
         exit(1);
     }
 
-    ioq->freelist = g_malloc0(sizeof ioq->freelist[0] * max_reqs);
+    ioq->freelist = g_new0(struct iocb *, max_reqs);
     ioq->freelist_idx = 0;
 
-    ioq->queue = g_malloc0(sizeof ioq->queue[0] * max_reqs);
+    ioq->queue = g_new0(struct iocb *, max_reqs);
     ioq->queue_idx = 0;
 }
 
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 5fd8f89..47577ec 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -319,7 +319,7 @@  static void nvme_init_sq(NvmeSQueue *sq, NvmeCtrl *n, uint64_t dma_addr,
     sq->size = size;
     sq->cqid = cqid;
     sq->head = sq->tail = 0;
-    sq->io_req = g_malloc(sq->size * sizeof(*sq->io_req));
+    sq->io_req = g_new(NvmeRequest, sq->size);
 
     QTAILQ_INIT(&sq->req_list);
     QTAILQ_INIT(&sq->out_req_list);
@@ -773,9 +773,9 @@  static int nvme_init(PCIDevice *pci_dev)
     n->reg_size = 1 << qemu_fls(0x1004 + 2 * (n->num_queues + 1) * 4);
     n->ns_size = bs_size / (uint64_t)n->num_namespaces;
 
-    n->namespaces = g_malloc0(sizeof(*n->namespaces)*n->num_namespaces);
-    n->sq = g_malloc0(sizeof(*n->sq)*n->num_queues);
-    n->cq = g_malloc0(sizeof(*n->cq)*n->num_queues);
+    n->namespaces = g_new0(NvmeNamespace, n->num_namespaces);
+    n->sq = g_new0(NvmeSQueue *, n->num_queues);
+    n->cq = g_new0(NvmeCQueue *, n->num_queues);
 
     memory_region_init_io(&n->iomem, OBJECT(n), &nvme_mmio_ops, n,
                           "nvme", n->reg_size);