[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 04/15] hw/ppc/spapr_rtas: Restrict variables scope to single
From: |
Greg Kurz |
Subject: |
Re: [PATCH 04/15] hw/ppc/spapr_rtas: Restrict variables scope to single switch case |
Date: |
Thu, 9 Jan 2020 18:43:49 +0100 |
On Thu, 9 Jan 2020 16:21:22 +0100
Philippe Mathieu-Daudé <address@hidden> wrote:
> We only access these variables in RTAS_SYSPARM_SPLPAR_CHARACTERISTICS
> case, restrict their scope to avoid unnecessary initialization.
>
I guess a decent compiler can be smart enough detect that the initialization
isn't needed outside of the RTAS_SYSPARM_SPLPAR_CHARACTERISTICS branch...
Anyway, reducing scope isn't bad. The only hitch I could see is that some
people do prefer to have all variables declared upfront, but there's a nested
param_val variable already so I guess it's okay.
> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> ---
> hw/ppc/spapr_rtas.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index 6f06e9d7fe..7237e5ebf2 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -267,8 +267,6 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
> uint32_t nret, target_ulong rets)
> {
> PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
> - MachineState *ms = MACHINE(spapr);
> - unsigned int max_cpus = ms->smp.max_cpus;
> target_ulong parameter = rtas_ld(args, 0);
> target_ulong buffer = rtas_ld(args, 1);
> target_ulong length = rtas_ld(args, 2);
> @@ -276,6 +274,8 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
>
> switch (parameter) {
> case RTAS_SYSPARM_SPLPAR_CHARACTERISTICS: {
> + MachineState *ms = MACHINE(spapr);
> + unsigned int max_cpus = ms->smp.max_cpus;
The max_cpus variable used to be a global. Now that it got moved
below ms->smp, I'm not sure it's worth keeping it IMHO. What about
dropping it completely and do:
char *param_val = g_strdup_printf("MaxEntCap=%d,"
"DesMem=%" PRIu64 ","
"DesProcs=%d,"
"MaxPlatProcs=%d",
ms->smp.max_cpus,
current_machine->ram_size / MiB,
ms->smp.cpus,
ms->smp.max_cpus);
And maybe insert an empty line between the declaration of param_val
and the code for a better readability ?
> char *param_val = g_strdup_printf("MaxEntCap=%d,"
> "DesMem=%" PRIu64 ","
> "DesProcs=%d,"
[PATCH 05/15] device-hotplug: Replace current_machine by qdev_get_machine(), Philippe Mathieu-Daudé, 2020/01/09
[PATCH 06/15] migration/savevm: Replace current_machine by qdev_get_machine(), Philippe Mathieu-Daudé, 2020/01/09