Skip to contentSkip to conversation

Small optimizations for code using g_malloc0 + memset/memcpy

PATCH Applied Thomas Huth 4 participants 9 messages Oct 9, 2015 – Oct 29, 2015

lore
Share & tools
lore

Thomas Huth

author
#0 lore raw

v2 cover Small optimizations for code using g_malloc0 + memset/memcpy

There are a couple of spots in the QEMU code which use g_malloc0, directly followed by a memset or memcpy which fill the whole allocated buffer. In this case it either does not make sense to zero the buffer via g_malloc0 first (so g_malloc should be used instead), or if the second command is a memset(..., 0, ...), then the memset does not make much sense, of course, since the buffer has already been zeroed by the g_malloc0.

v2:
- Switched some of the g_malloc0()s to g_new0()s where appropriate
- Added Reviewed-by-s
- The spapr_vscsi patch has been picked up by David already, so not sending it again

Thomas Huth (4):
hw/dma/pxa2xx: Remove superfluous memset hw/input/tsc210x: Remove superfluous memset tests/i44fx-test: No need for zeroing memory before memset linux-user/syscall: Replace g_malloc0 + memcpy with g_memdup

 hw/dma/pxa2xx_dma.c  | 3 +--
 hw/input/tsc210x.c   | 8 ++------
 linux-user/syscall.c | 3 +--
 tests/i440fx-test.c  | 2 +-
 4 files changed, 5 insertions(+), 11 deletions(-)
Signature · 1 line
1.8.3.1
author
#1 lore raw

PATCH v2 1/4 hw/dma/pxa2xx: Remove superfluous memset

g_malloc0 already clears the memory, so no need for the additional memset here. And while we're at it, also convert the g_malloc0 to the preferred g_new0.

Signed-off-by Thomas Huth <[email protected]>
---
 hw/dma/pxa2xx_dma.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
hw/dma/pxa2xx_dma.c +1 -2
index d4501fb..54cdb25 100644
@@ -459,9 +459,8 @@ static int pxa2xx_dma_init(SysBusDevice *sbd)
         return -1;
     }
 
-    s->chan = g_malloc0(sizeof(PXA2xxDMAChannel) * s->channels);
+    s->chan = g_new0(PXA2xxDMAChannel, s->channels);
 
-    memset(s->chan, 0, sizeof(PXA2xxDMAChannel) * s->channels);
     for (i = 0; i < s->channels; i ++)
         s->chan[i].state = DCSR_STOPINTR;
Signature · 1 line
1.8.3.1
Reviewed-by on v2 1/4
#2 lore raw

Re: [Qemu-devel] [PATCH v2 1/4] hw/dma/pxa2xx: Remove superfluous memset

Quoted 8 lines · Thomas Huth

g_malloc0 already clears the memory, so no need for the additional memset here. And while we're at it, also convert the g_malloc0 to the preferred g_new0.

Signed-off-by Thomas Huth <[email protected]>
---
 hw/dma/pxa2xx_dma.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
Reviewed-by Eric Blake <[email protected]>
Quoted 17 lines
hw/dma/pxa2xx_dma.c +1 -2
index d4501fb..54cdb25 100644
@@ -459,9 +459,8 @@ static int pxa2xx_dma_init(SysBusDevice *sbd)
         return -1;
     }
 
-    s->chan = g_malloc0(sizeof(PXA2xxDMAChannel) * s->channels);
+    s->chan = g_new0(PXA2xxDMAChannel, s->channels);
 
-    memset(s->chan, 0, sizeof(PXA2xxDMAChannel) * s->channels);
     for (i = 0; i < s->channels; i ++)
         s->chan[i].state = DCSR_STOPINTR;
Signature · 2 lines
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org
author
#3 lore raw

PATCH v2 2/4 hw/input/tsc210x: Remove superfluous memset

In reply to 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 and use the type-safe g_new0 instead.

Signed-off-by Thomas Huth <[email protected]>
---
 hw/input/tsc210x.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)
hw/input/tsc210x.c +2 -6
index fae3385..1073bbf 100644
@@ -1086,9 +1086,7 @@ uWireSlave *tsc2102_init(qemu_irq pint)
 {
     TSC210xState *s;
 
-    s = (TSC210xState *)
-            g_malloc0(sizeof(TSC210xState));
-    memset(s, 0, sizeof(TSC210xState));
+    s = g_new0(TSC210xState, 1);
     s->x = 160;
     s->y = 160;
     s->pressure = 0;

@@ -1135,9 +1133,7 @@ uWireSlave *tsc2301_init(qemu_irq penirq, qemu_irq kbirq, qemu_irq dav)
 {
     TSC210xState *s;
 
-    s = (TSC210xState *)
-            g_malloc0(sizeof(TSC210xState));
-    memset(s, 0, sizeof(TSC210xState));
+    s = g_new0(TSC210xState, 1);
     s->x = 400;
     s->y = 240;
     s->pressure = 0;
Signature · 1 line
1.8.3.1
Reviewed-by on v2 2/4
#4 lore raw

Re: [Qemu-devel] [PATCH v2 2/4] hw/input/tsc210x: Remove superfluous memset

Quoted 9 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 and use the type-safe g_new0 instead.

Signed-off-by Thomas Huth <[email protected]>
---
 hw/input/tsc210x.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)
Reviewed-by Eric Blake <[email protected]>
Signature · 2 lines
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org
author
#5 lore raw

PATCH v2 3/4 tests/i44fx-test: No need for zeroing memory before memset

In reply to Thomas Huth

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

Signed-off-by Thomas Huth <[email protected]>
Reviewed-by Laszlo Ersek <[email protected]>
---
 tests/i440fx-test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tests/i440fx-test.c +1 -1
index d0bc8de..7fa1709 100644
@@ -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);
Signature · 1 line
1.8.3.1
author
#6 lore raw

PATCH v2 4/4 linux-user/syscall: Replace g_malloc0 + memcpy with g_memdup

In reply to Thomas Huth

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.

Signed-off-by Thomas Huth <[email protected]>
Reviewed-by Eric Blake <[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);
Signature · 1 line
1.8.3.1
Applied
#7 lore raw

Re: [Qemu-devel] [PATCH v2 4/4] linux-user/syscall: Replace g_malloc0 + memcpy with g_memdup

Quoted 4 lines · Thomas Huth

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.

Applied to linux-user, thanks

Quoted 21 lines
Signed-off-by Thomas Huth <[email protected]>
Reviewed-by Eric Blake <[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);
Applied
#8 lore raw
In reply to Thomas Huth
Quoted 7 lines · Thomas Huth

There are a couple of spots in the QEMU code which use g_malloc0, directly followed by a memset or memcpy which fill the whole allocated buffer. In this case it either does not make sense to zero the buffer via g_malloc0 first (so g_malloc should be used instead), or if the second command is a memset(..., 0, ...), then the memset does not make much sense, of course, since the buffer has already been zeroed by the g_malloc0.

Applied to -trivial, thanks!
The 4/4 patch has been picked up by Riku.

/mjt