Re: [PATCH v5 04/11] mac_oldworld: Drop some variables

[Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>]

On 16/06/2020 14:47, BALATON Zoltan wrote:

> Values not used frequently enough may not worth putting in a local
> variable, especially with names almost as long as the original value
> because that does not improve readability, to the contrary it makes it
> harder to see what value is used. Drop a few such variables.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>  hw/ppc/mac_oldworld.c | 33 ++++++++++++++++-----------------
>  1 file changed, 16 insertions(+), 17 deletions(-)
> 
> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
> index d1c4244b1e..4200008851 100644
> --- a/hw/ppc/mac_oldworld.c
> +++ b/hw/ppc/mac_oldworld.c
> @@ -83,14 +83,11 @@ static void ppc_heathrow_reset(void *opaque)
>  static void ppc_heathrow_init(MachineState *machine)
>  {
>      ram_addr_t ram_size = machine->ram_size;
> -    const char *kernel_filename = machine->kernel_filename;
> -    const char *kernel_cmdline = machine->kernel_cmdline;
> -    const char *initrd_filename = machine->initrd_filename;
>      const char *boot_device = machine->boot_order;
>      PowerPCCPU *cpu = NULL;
>      CPUPPCState *env = NULL;
>      char *filename;
> -    int linux_boot, i;
> +    int i;
>      MemoryRegion *bios = g_new(MemoryRegion, 1);
>      uint32_t kernel_base, initrd_base, cmdline_base = 0;
>      int32_t kernel_size, initrd_size;
> @@ -108,8 +105,6 @@ static void ppc_heathrow_init(MachineState *machine)
>      void *fw_cfg;
>      uint64_t tbfreq;
>  
> -    linux_boot = (kernel_filename != NULL);
> -
>      /* init CPUs */
>      for (i = 0; i < smp_cpus; i++) {
>          cpu = POWERPC_CPU(cpu_create(machine->cpu_type));
> @@ -159,7 +154,7 @@ static void ppc_heathrow_init(MachineState *machine)
>          exit(1);
>      }
>  
> -    if (linux_boot) {
> +    if (machine->kernel_filename) {
>          uint64_t lowaddr = 0;
>          int bswap_needed;
>  
> @@ -169,30 +164,33 @@ static void ppc_heathrow_init(MachineState *machine)
>          bswap_needed = 0;
>  #endif
>          kernel_base = KERNEL_LOAD_ADDR;
> -        kernel_size = load_elf(kernel_filename, NULL,
> +        kernel_size = load_elf(machine->kernel_filename, NULL,
>                                 translate_kernel_address, NULL,
>                                 NULL, &lowaddr, NULL, NULL, 1, PPC_ELF_MACHINE,
>                                 0, 0);
>          if (kernel_size < 0)
> -            kernel_size = load_aout(kernel_filename, kernel_base,
> +            kernel_size = load_aout(machine->kernel_filename, kernel_base,
>                                      ram_size - kernel_base, bswap_needed,
>                                      TARGET_PAGE_SIZE);
>          if (kernel_size < 0)
> -            kernel_size = load_image_targphys(kernel_filename,
> +            kernel_size = load_image_targphys(machine->kernel_filename,
>                                                kernel_base,
>                                                ram_size - kernel_base);
>          if (kernel_size < 0) {
> -            error_report("could not load kernel '%s'", kernel_filename);
> +            error_report("could not load kernel '%s'",
> +                         machine->kernel_filename);
>              exit(1);
>          }
>          /* load initrd */
> -        if (initrd_filename) {
> -            initrd_base = TARGET_PAGE_ALIGN(kernel_base + kernel_size + KERNEL_GAP);
> -            initrd_size = load_image_targphys(initrd_filename, initrd_base,
> +        if (machine->initrd_filename) {
> +            initrd_base = TARGET_PAGE_ALIGN(kernel_base + kernel_size +
> +                                            KERNEL_GAP);
> +            initrd_size = load_image_targphys(machine->initrd_filename,
> +                                              initrd_base,
>                                                ram_size - initrd_base);
>              if (initrd_size < 0) {
>                  error_report("could not load initial ram disk '%s'",
> -                             initrd_filename);
> +                             machine->initrd_filename);
>                  exit(1);
>              }
>              cmdline_base = TARGET_PAGE_ALIGN(initrd_base + initrd_size);
> @@ -336,9 +334,10 @@ static void ppc_heathrow_init(MachineState *machine)
>      fw_cfg_add_i16(fw_cfg, FW_CFG_MACHINE_ID, ARCH_HEATHROW);
>      fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, kernel_base);
>      fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, kernel_size);
> -    if (kernel_cmdline) {
> +    if (machine->kernel_cmdline) {
>          fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, cmdline_base);
> -        pstrcpy_targphys("cmdline", cmdline_base, TARGET_PAGE_SIZE, kernel_cmdline);
> +        pstrcpy_targphys("cmdline", cmdline_base, TARGET_PAGE_SIZE,
> +                         machine->kernel_cmdline);
>      } else {
>          fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, 0);
>      }

Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.