Small optimizations for code using g_malloc0 + memset/memcpy

g_malloc0 already clears the memory, so no need for the additional memset here.

Signed-off-by Thomas Huth <[email protected]>

---
 hw/dma/pxa2xx_dma.c | 1 -
 1 file changed, 1 deletion(-)

hw/dma/pxa2xx_dma.c +0 -0

index d4501fb..3f0a720 100644

Quoted 14 lines · Thomas Huth

g_malloc0 already clears the memory, so no need for the additional memset here.

Signed-off-by Thomas Huth <[email protected]>

---
 hw/dma/pxa2xx_dma.c | 1 -
 1 file changed, 1 deletion(-)

hw/dma/pxa2xx_dma.c +0 -0

index d4501fb..3f0a720 100644
@@ -461,7 +461,6 @@ static int pxa2xx_dma_init(SysBusDevice *sbd)
 

hw/dma/pxa2xx_dma.c static int pxa2xx_dma_init(SysBusDevice *sbd) · line 462

Quoted 1 line

hw/dma/pxa2xx_dma.c +0 -0

     s->chan = g_malloc0(sizeof(PXA2xxDMAChannel) * s->channels);

As long as we're here, this should probably be switched to g_new0(), in part to avoid any chance that the multiply can overflow.

g_malloc0 already clears the memory, so no need for the additional memset here.

Cc Paolo Bonzini <[email protected]>

Cc David Gibson <[email protected]>

Cc Alexander Graf <[email protected]>

Signed-off-by Thomas Huth <[email protected]>

---
 hw/scsi/spapr_vscsi.c | 1 -
 1 file changed, 1 deletion(-)

g_malloc0 already clears the memory, so no need for the additional memset here.

Cc Paolo Bonzini <[email protected]>

Cc David Gibson <[email protected]>

Cc Alexander Graf <[email protected]>

Signed-off-by Thomas Huth <[email protected]>

g_malloc0 already clears the memory, so no need for additional memsets here. And while we're at it, let's also remove the superfluous typecasts for the return values of g_malloc0.

Signed-off-by Thomas Huth <[email protected]>

---
 hw/input/tsc210x.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

hw/input/tsc210x.c +0 -0

index fae3385..92b076a 100644

Quoted 17 lines · Thomas Huth

g_malloc0 already clears the memory, so no need for additional memsets here. And while we're at it, let's also remove the superfluous typecasts for the return values of g_malloc0.

Signed-off-by Thomas Huth <[email protected]>

---
 hw/input/tsc210x.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

hw/input/tsc210x.c +0 -0

index fae3385..92b076a 100644
@@ -1086,9 +1086,7 @@ uWireSlave *tsc2102_init(qemu_irq pint)
 {
     TSC210xState *s;
 

hw/input/tsc210x.c uWireSlave *tsc2102_init(qemu_irq pint) · line 1089

Quoted 4 lines

hw/input/tsc210x.c +1 -3

-    s = (TSC210xState *)
-            g_malloc0(sizeof(TSC210xState));
-    memset(s, 0, sizeof(TSC210xState));
+    s = g_malloc0(sizeof(TSC210xState));

This should probably be g_new0(TSC210xState, 1), consistent with Markus' recent cleanup patches.

Change a g_malloc0 into g_malloc since the following memset fills the whole buffer anyway.

Cc Laszlo Ersek <[email protected]>

Signed-off-by Thomas Huth <[email protected]>

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

tests/i440fx-test.c +0 -0

index d0bc8de..7fa1709 100644

Quoted 15 lines · Thomas Huth

Change a g_malloc0 into g_malloc since the following memset fills the whole buffer anyway.

Cc Laszlo Ersek <[email protected]>

Signed-off-by Thomas Huth <[email protected]>

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

tests/i440fx-test.c +0 -0

index d0bc8de..7fa1709 100644

tests/i440fx-test.c static void write_area(uint32_t start, uint32_t end, uint8_t value) · line 197

Quoted 8 lines

tests/i440fx-test.c +1 -1

@@ -191,7 +191,7 @@ static void write_area(uint32_t start, uint32_t end, uint8_t value)
     uint32_t size = end - start + 1;
     uint8_t *data;
 
-    data = g_malloc0(size);
+    data = g_malloc(size);
     memset(data, value, size);
     memwrite(start, data, size);
 

Technically you are right of course, but I remember some historical mess around this, in this file.

Plus I vaguely recall g_new[0]() being the most recent preference.

https://developer.gnome.org/glib/stable/glib-Memory-Allocation.html#g-new

See e.g. commit 97f3ad3551. Markus?

Quoted 15 lines · Thomas Huth

Change a g_malloc0 into g_malloc since the following memset fills the whole buffer anyway.

Cc Laszlo Ersek <[email protected]>

Signed-off-by Thomas Huth <[email protected]>

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

tests/i440fx-test.c +0 -0

index d0bc8de..7fa1709 100644

tests/i440fx-test.c static void write_area(uint32_t start, uint32_t end, uint8_t value) · line 197

Quoted 8 lines

tests/i440fx-test.c +1 -1

@@ -191,7 +191,7 @@ static void write_area(uint32_t start, uint32_t end, uint8_t value)
     uint32_t size = end - start + 1;
     uint8_t *data;
 
-    data = g_malloc0(size);
+    data = g_malloc(size);
     memset(data, value, size);
     memwrite(start, data, size);
 

Technically you are right of course, but I remember some historical mess around this, in this file.

Plus I vaguely recall g_new[0]() being the most recent preference.

https://developer.gnome.org/glib/stable/glib-Memory-Allocation.html#g-new

See e.g. commit 97f3ad3551. Markus?

Quoted 28 lines · Laszlo Ersek

Quoted 15 lines · Thomas Huth

Change a g_malloc0 into g_malloc since the following memset fills the whole buffer anyway.

Cc Laszlo Ersek <[email protected]>

Signed-off-by Thomas Huth <[email protected]>

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

tests/i440fx-test.c +0 -0

index d0bc8de..7fa1709 100644

tests/i440fx-test.c static void write_area(uint32_t start, uint32_t end, uint8_t value) · line 197

Quoted 8 lines

tests/i440fx-test.c +1 -1

@@ -191,7 +191,7 @@ static void write_area(uint32_t start, uint32_t end, uint8_t value)
     uint32_t size = end - start + 1;
     uint8_t *data;
 
-    data = g_malloc0(size);
+    data = g_malloc(size);
     memset(data, value, size);
     memwrite(start, data, size);
 

Technically you are right of course, but I remember some historical mess around this, in this file.

Quoted 5 lines

Plus I vaguely recall g_new[0]() being the most recent preference.

https://developer.gnome.org/glib/stable/glib-Memory-Allocation.html#g-new

See e.g. commit 97f3ad3551. Markus?

TL;DR: the patch is fine.

The argument of g_malloc() isn't always the size of a type. But when it is, you should be using g_new() instead. The commit message explains:

g_new(T, n) is neater than g_malloc(sizeof(T) * n). It's also safer, 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.

The multiplication argument applies only when n != 1.

The type checking argument applies regardless of n. That's why the commit also transfroms patterns like g_malloc(sizeof(T)).

When the argument isn't written as sizeof a type, but could be, we enter "matter of taste" territory. For instance, some may perefer the idiomatic T *p = g_malloc(sizeof(*p)) to the more tightly typed p = g_new(T, 1). I therefore leave them alone. Commit messsage again:

This commit only touches allocations with size arguments of the form sizeof(T).

A separate issue is a zeroing memset() after allocation. Please use a zeroing allocator instead, because that's more concise. Precedence:
commit 0bd0adb.

Now let's apply this to the patch.

The memset() isn't zeroing, so "use a zeroing allocator instead" doesn't apply. Before the patch, the code uses one additionally, which is wasteful. The patch stops the waste.

The size argument is not the size of any type. You could still write

    data = g_new(uint8_t, size);

but the extra verbosity pretty clearly outweighs what we could gain from type checking here.

No need to use g_malloc0 to zero the memory if we memcpy to the whole buffer afterwards anyway. Actually, there is even a function which combines both steps, g_memdup, so let's use this function here instead.

Cc Riku Voipio <[email protected]>

Signed-off-by Thomas Huth <[email protected]>

---
 linux-user/syscall.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

linux-user/syscall.c +1 -2

index 98b5766..f2ada0a 100644
@@ -5325,8 +5325,7 @@ static abi_long do_open_by_handle_at(abi_long mount_fd, abi_long handle,
         return -TARGET_EFAULT;
     }
 
-    fh = g_malloc0(total_size);
-    memcpy(fh, target_fh, total_size);
+    fh = g_memdup(target_fh, total_size);
     fh->handle_bytes = size;
     fh->handle_type = tswap32(target_fh->handle_type);