Re: [PATCH v2 3/3] Use g_new() & friends where that makes obvious sense

[Igor Mammedov <imammedo@xxxxxxxxxx>]

On Tue, 15 Mar 2022 15:41:56 +0100
Markus Armbruster <armbru@xxxxxxxxxx> wrote:

> 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.
> 
> This commit only touches allocations with size arguments of the form
> sizeof(T).
> 
> Patch created mechanically with:
> 
>     $ spatch --in-place --sp-file scripts/coccinelle/use-g_new-etc.cocci \
>            --macro-file scripts/cocci-macro-file.h FILES...
> 
> Signed-off-by: Markus Armbruster <armbru@xxxxxxxxxx>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@xxxxxxxxx>
> Reviewed-by: Cédric Le Goater <clg@xxxxxxxx>
> Reviewed-by: Alex Bennée <alex.bennee@xxxxxxxxxx>
> Acked-by: Dr. David Alan Gilbert <dgilbert@xxxxxxxxxx>


for */i386/*
Reviewed-by: Igor Mammedov <imammedo@xxxxxxxxxx>


nit:
possible miss, see below 

[...]
> diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
> index cf8e500514..0731f70410 100644
> --- a/hw/i386/xen/xen-hvm.c
> +++ b/hw/i386/xen/xen-hvm.c

missed:

 pfn_list = g_malloc(sizeof (*pfn_list) * nr_pfn);


> @@ -396,7 +396,7 @@ go_physmap:
>  
>      mr_name = memory_region_name(mr);
>  
> -    physmap = g_malloc(sizeof(XenPhysmap));
> +    physmap = g_new(XenPhysmap, 1);
>  
>      physmap->start_addr = start_addr;
>      physmap->size = size;
> @@ -1281,7 +1281,7 @@ static void xen_read_physmap(XenIOState *state)
>          return;
>  
>      for (i = 0; i < num; i++) {
> -        physmap = g_malloc(sizeof (XenPhysmap));
> +        physmap = g_new(XenPhysmap, 1);
>          physmap->phys_offset = strtoull(entries[i], NULL, 16);
>          snprintf(path, sizeof(path),
>                  "/local/domain/0/device-model/%d/physmap/%s/start_addr",
> @@ -1410,7 +1410,7 @@ void xen_hvm_init_pc(PCMachineState *pcms, MemoryRegion 
> **ram_memory)
>      xen_pfn_t ioreq_pfn;
>      XenIOState *state;
>  
> -    state = g_malloc0(sizeof (XenIOState));
> +    state = g_new0(XenIOState, 1);
>  
>      state->xce_handle = xenevtchn_open(NULL, 0);
>      if (state->xce_handle == NULL) {
> @@ -1463,7 +1463,7 @@ void xen_hvm_init_pc(PCMachineState *pcms, MemoryRegion 
> **ram_memory)
>      }
>  
>      /* Note: cpus is empty at this point in init */
> -    state->cpu_by_vcpu_id = g_malloc0(max_cpus * sizeof(CPUState *));
> +    state->cpu_by_vcpu_id = g_new0(CPUState *, max_cpus);
>  
>      rc = xen_set_ioreq_server_state(xen_domid, state->ioservid, true);
>      if (rc < 0) {
> @@ -1472,7 +1472,7 @@ void xen_hvm_init_pc(PCMachineState *pcms, MemoryRegion 
> **ram_memory)
>          goto err;
>      }
>  
> -    state->ioreq_local_port = g_malloc0(max_cpus * sizeof (evtchn_port_t));
> +    state->ioreq_local_port = g_new0(evtchn_port_t, max_cpus);

[...]