qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PULL 45/74] spapr: do not use CPU_FOREACH_REVERSE


From: Peter Maydell
Subject: Re: [Qemu-devel] [PULL 45/74] spapr: do not use CPU_FOREACH_REVERSE
Date: Fri, 24 Aug 2018 16:20:24 +0100

On 21 August 2018 at 18:02, Paolo Bonzini <address@hidden> wrote:
> From: "Emilio G. Cota" <address@hidden>
>
> This paves the way for implementing the CPU list with an RCU list,
> which cannot be traversed in reverse order.
>
> Note that this is the only caller of CPU_FOREACH_REVERSE.
>
> Acked-by: David Gibson <address@hidden>
> Signed-off-by: Emilio G. Cota <address@hidden>
> Message-Id: <address@hidden>
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  hw/ppc/spapr.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index e5d8253..ab9c04e 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -622,9 +622,12 @@ static void spapr_populate_cpu_dt(CPUState *cs, void 
> *fdt, int offset,
>
>  static void spapr_populate_cpus_dt_node(void *fdt, sPAPRMachineState *spapr)
>  {
> +    CPUState **rev;
>      CPUState *cs;
> +    int n_cpus;
>      int cpus_offset;
>      char *nodename;
> +    int i;
>
>      cpus_offset = fdt_add_subnode(fdt, 0, "cpus");
>      _FDT(cpus_offset);
> @@ -635,8 +638,19 @@ static void spapr_populate_cpus_dt_node(void *fdt, 
> sPAPRMachineState *spapr)
>       * We walk the CPUs in reverse order to ensure that CPU DT nodes
>       * created by fdt_add_subnode() end up in the right order in FDT
>       * for the guest kernel the enumerate the CPUs correctly.
> +     *
> +     * The CPU list cannot be traversed in reverse order, so we need
> +     * to do extra work.
>       */
> -    CPU_FOREACH_REVERSE(cs) {
> +    n_cpus = 0;
> +    rev = NULL;
> +    CPU_FOREACH(cs) {
> +        rev = g_renew(CPUState *, rev, n_cpus + 1);
> +        rev[n_cpus++] = cs;
> +    }
> +
> +    for (i = n_cpus - 1; i >= 0; i--) {
> +        CPUState *cs = rev[i];
>          PowerPCCPU *cpu = POWERPC_CPU(cs);
>          int index = spapr_get_vcpu_id(cpu);
>          DeviceClass *dc = DEVICE_GET_CLASS(cs);
> --

Hi -- Coverity points out in CID1395181 that this introduces
a memory leak -- we allocate memory into the rev pointer
with g_renew(), but we never free it before leaving the function.

thanks
-- PMM



reply via email to

[Prev in Thread] Current Thread [Next in Thread]