[Qemu-devel] [PATCH v2 0/4] Small optimizations for code using g_malloc0 + memset/memcpy

[Qemu-devel] [PATCH v2 0/4] Small optimizations for code using g_malloc0 + memset/memcpy

[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.

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(-)

-- 
1.8.3.1

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

[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 <thuth@redhat.com>
---
 hw/dma/pxa2xx_dma.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/dma/pxa2xx_dma.c b/hw/dma/pxa2xx_dma.c
index d4501fb..54cdb25 100644
--- a/hw/dma/pxa2xx_dma.c
+++ b/hw/dma/pxa2xx_dma.c
@@ -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;
 
-- 
1.8.3.1

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

[Eric Blake]

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

On 10/09/2015 09:56 AM, Thomas Huth wrote:
> 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 <thuth@redhat.com>
> ---
>  hw/dma/pxa2xx_dma.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

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

> 
> diff --git a/hw/dma/pxa2xx_dma.c b/hw/dma/pxa2xx_dma.c
> index d4501fb..54cdb25 100644
> --- a/hw/dma/pxa2xx_dma.c
> +++ b/hw/dma/pxa2xx_dma.c
> @@ -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;
>  
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


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

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

[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 <thuth@redhat.com>
---
 hw/input/tsc210x.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/hw/input/tsc210x.c b/hw/input/tsc210x.c
index fae3385..1073bbf 100644
--- a/hw/input/tsc210x.c
+++ b/hw/input/tsc210x.c
@@ -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;
-- 
1.8.3.1

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

[Eric Blake]

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

On 10/09/2015 09:56 AM, Thomas Huth wrote:
> 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 <thuth@redhat.com>
> ---
>  hw/input/tsc210x.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)

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

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


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

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

[Thomas Huth]

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

Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 tests/i440fx-test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/i440fx-test.c b/tests/i440fx-test.c
index d0bc8de..7fa1709 100644
--- a/tests/i440fx-test.c
+++ b/tests/i440fx-test.c
@@ -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);
 
-- 
1.8.3.1

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

[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 <thuth@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 linux-user/syscall.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 98b5766..f2ada0a 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -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);
 
-- 
1.8.3.1

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

[Riku Voipio]

On perjantaina 9. lokakuuta 2015 18.56.38 EEST, Thomas Huth wrote:
> 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

> Signed-off-by: Thomas Huth <thuth@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
>  linux-user/syscall.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 98b5766..f2ada0a 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -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);
>  

Re: [Qemu-devel] [PATCH v2 0/4] Small optimizations for code using g_malloc0 + memset/memcpy

[Michael Tokarev]

09.10.2015 18:56, Thomas Huth wrote:
> 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