On 03/10/2022 21:13, BALATON Zoltan wrote:
By slight reorganisation we can avoid an else branch and some code duplication which makes it easier to follow the code.Signed-off-by: BALATON Zoltan <bala...@eik.bme.hu> --- hw/ppc/mac_newworld.c | 6 +++--- hw/ppc/mac_oldworld.c | 7 +++---- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c index 6bc3bd19be..73b01e8c6d 100644 --- a/hw/ppc/mac_newworld.c +++ b/hw/ppc/mac_newworld.c @@ -194,9 +194,11 @@ static void ppc_core99_init(MachineState *machine) machine->kernel_filename); exit(1); } + cmdline_base = TARGET_PAGE_ALIGN(kernel_base + kernel_size + + KERNEL_GAP); /* load initrd */ if (machine->initrd_filename) { - initrd_base = TARGET_PAGE_ALIGN(kernel_base + kernel_size + KERNEL_GAP); + initrd_base = cmdline_base; initrd_size = load_image_targphys(machine->initrd_filename, initrd_base, machine->ram_size - initrd_base); @@ -206,8 +208,6 @@ static void ppc_core99_init(MachineState *machine) exit(1); } cmdline_base = TARGET_PAGE_ALIGN(initrd_base + initrd_size); - } else { - cmdline_base = TARGET_PAGE_ALIGN(kernel_base + kernel_size + KERNEL_GAP); } ppc_boot_device = 'm'; } else { diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c index cb67e44081..b424729a39 100644 --- a/hw/ppc/mac_oldworld.c +++ b/hw/ppc/mac_oldworld.c @@ -168,10 +168,11 @@ static void ppc_heathrow_init(MachineState *machine) machine->kernel_filename); exit(1); } + cmdline_base = TARGET_PAGE_ALIGN(kernel_base + kernel_size + + KERNEL_GAP); /* load initrd */ if (machine->initrd_filename) { - initrd_base = TARGET_PAGE_ALIGN(kernel_base + kernel_size + - KERNEL_GAP); + initrd_base = cmdline_base; initrd_size = load_image_targphys(machine->initrd_filename, initrd_base, machine->ram_size - initrd_base); @@ -181,8 +182,6 @@ static void ppc_heathrow_init(MachineState *machine) exit(1); } cmdline_base = TARGET_PAGE_ALIGN(initrd_base + initrd_size); - } else { - cmdline_base = TARGET_PAGE_ALIGN(kernel_base + kernel_size + KERNEL_GAP); } ppc_boot_device = 'm'; } else {
Is there any particular reason why you would want to avoid the else branch? I don't feel this patch is an improvement since by always setting cmdline_base to a non-zero value, as a reviewer I then have to check all other uses of cmdline_base in the file to ensure that this doesn't cause any issues.
I much prefer the existing version since setting the values of cmdline_base and initrd_base is very clearly scoped within the if statement.
ATB, Mark.
