diff mbox series

[for-3.1] hw/arm/exynos4210: Zero memory allocated for Exynos4210State

Message ID 20181105151132.13884-1-peter.maydell@linaro.org
State Superseded
Headers show
Series [for-3.1] hw/arm/exynos4210: Zero memory allocated for Exynos4210State | expand

Commit Message

Peter Maydell Nov. 5, 2018, 3:11 p.m. UTC
In exynos4210_init() we allocate memory for an Exynos4210State
struct. Generally devices can assume that the memory allocated
for their state struct is zero-initialized; we broke that
assumption here by using g_new(). Use g_new0() instead.
(In particular, some code assumes that the various irq arrays
in the Exynos4210Irq sub-struct are zero-initialized.)

In the longer term, this code should be QOMified, and then
the struct memory will be allocated elsewhere and by functions
which always zero-initalize it; but for 3.1 this is a
simple fix.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

---
I suggested this fix the other day:
https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg00135.html
so here it is as an actual patch.

Probably we should go through other uses of g_new() in
board/device code at some point.

 hw/arm/exynos4210.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.19.1

Comments

Philippe Mathieu-Daudé Nov. 5, 2018, 3:29 p.m. UTC | #1
Hi Peter,

On 5/11/18 16:11, Peter Maydell wrote:
> In exynos4210_init() we allocate memory for an Exynos4210State

> struct. Generally devices can assume that the memory allocated

> for their state struct is zero-initialized; we broke that

> assumption here by using g_new(). Use g_new0() instead.

> (In particular, some code assumes that the various irq arrays

> in the Exynos4210Irq sub-struct are zero-initialized.)

> 

> In the longer term, this code should be QOMified, and then

> the struct memory will be allocated elsewhere and by functions

> which always zero-initalize it; but for 3.1 this is a

> simple fix.

> 

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

> ---

> I suggested this fix the other day:

> https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg00135.html

> so here it is as an actual patch.


I did the same patch as you suggested but continued more soft-freeze 
testing and planed to send it today, sorry.

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>


> 

> Probably we should go through other uses of g_new() in

> board/device code at some point.

> 

>   hw/arm/exynos4210.c | 2 +-

>   1 file changed, 1 insertion(+), 1 deletion(-)

> 

> diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c

> index 827318a0036..af82e955421 100644

> --- a/hw/arm/exynos4210.c

> +++ b/hw/arm/exynos4210.c

> @@ -162,7 +162,7 @@ static uint64_t exynos4210_calc_affinity(int cpu)

>   

>   Exynos4210State *exynos4210_init(MemoryRegion *system_mem)

>   {

> -    Exynos4210State *s = g_new(Exynos4210State, 1);

> +    Exynos4210State *s = g_new0(Exynos4210State, 1);

>       qemu_irq gate_irq[EXYNOS4210_NCPUS][EXYNOS4210_IRQ_GATE_NINPUTS];

>       SysBusDevice *busdev;

>       DeviceState *dev;

>
diff mbox series

Patch

diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index 827318a0036..af82e955421 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -162,7 +162,7 @@  static uint64_t exynos4210_calc_affinity(int cpu)
 
 Exynos4210State *exynos4210_init(MemoryRegion *system_mem)
 {
-    Exynos4210State *s = g_new(Exynos4210State, 1);
+    Exynos4210State *s = g_new0(Exynos4210State, 1);
     qemu_irq gate_irq[EXYNOS4210_NCPUS][EXYNOS4210_IRQ_GATE_NINPUTS];
     SysBusDevice *busdev;
     DeviceState *dev;