On 21 August 2018 at 18:02, Paolo Bonzini <pbonz...@redhat.com> wrote:
> From: "Emilio G. Cota" <c...@braap.org>
>
> 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 <da...@gibson.dropbear.id.au>
> Signed-off-by: Emilio G. Cota <c...@braap.org>
> Message-Id: <20180819091335.22863-11-c...@braap.org>
> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
> ---
> 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
