| Message ID | 20230127164718.98156-7-shentey@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | PC cleanups | expand |
On Fri, 27 Jan 2023, Bernhard Beschow wrote: > Signed-off-by: Bernhard Beschow <shentey@gmail.com> Why? I'd rather replace locals with direct call to function as it's not expensive (just returns a global) and adding a local name to it is not much shorter so why do that? Regards, BALATON Zoltan > --- > hw/i386/pc_piix.c | 2 +- > hw/i386/pc_q35.c | 7 ++++--- > 2 files changed, 5 insertions(+), 4 deletions(-) > > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c > index ee9d9a4175..5bde4533cc 100644 > --- a/hw/i386/pc_piix.c > +++ b/hw/i386/pc_piix.c > @@ -241,7 +241,7 @@ static void pc_init1(MachineState *machine, > isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix3), "isa.0")); > } else { > pci_bus = NULL; > - isa_bus = isa_bus_new(NULL, get_system_memory(), system_io, > + isa_bus = isa_bus_new(NULL, system_memory, system_io, > &error_abort); > i8257_dma_init(isa_bus, 0); > pcms->hpet_enabled = false; > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c > index a97846ab9b..b97979bebb 100644 > --- a/hw/i386/pc_q35.c > +++ b/hw/i386/pc_q35.c > @@ -124,6 +124,7 @@ static void pc_q35_init(MachineState *machine) > DeviceState *lpc_dev; > BusState *idebus[MAX_SATA_PORTS]; > ISADevice *rtc_state; > + MemoryRegion *system_memory = get_system_memory(); > MemoryRegion *system_io = get_system_io(); > MemoryRegion *pci_memory; > MemoryRegion *rom_memory; > @@ -191,7 +192,7 @@ static void pc_q35_init(MachineState *machine) > rom_memory = pci_memory; > } else { > pci_memory = NULL; > - rom_memory = get_system_memory(); > + rom_memory = system_memory; > } > > pc_guest_info_init(pcms); > @@ -214,7 +215,7 @@ static void pc_q35_init(MachineState *machine) > } > > /* allocate ram and load rom/bios */ > - pc_memory_init(pcms, get_system_memory(), rom_memory, &ram_memory, > + pc_memory_init(pcms, system_memory, rom_memory, &ram_memory, > pci_hole64_size); > > object_property_add_child(OBJECT(machine), "q35", OBJECT(phb)); > @@ -223,7 +224,7 @@ static void pc_q35_init(MachineState *machine) > object_property_set_link(OBJECT(phb), MCH_HOST_PROP_PCI_MEM, > OBJECT(pci_memory), NULL); > object_property_set_link(OBJECT(phb), MCH_HOST_PROP_SYSTEM_MEM, > - OBJECT(get_system_memory()), NULL); > + OBJECT(system_memory), NULL); > object_property_set_link(OBJECT(phb), MCH_HOST_PROP_IO_MEM, > OBJECT(system_io), NULL); > object_property_set_int(OBJECT(phb), PCI_HOST_BELOW_4G_MEM_SIZE, >
Am 27. Januar 2023 19:30:21 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>: >On Fri, 27 Jan 2023, Bernhard Beschow wrote: >> Signed-off-by: Bernhard Beschow <shentey@gmail.com> > >Why? I'd rather replace locals with direct call to function as it's not expensive (just returns a global) and adding a local name to it is not much shorter so why do that? The function has the assumption baked in to return a global which I'd like to factor out into a single variable assignment. This allows for easier experimentation with alternatives to global variables. Moreover, extrating to a single variable mirrors how similar things are done in the pc and q35 machines. Best regards, Bernhard > >Regards, >BALATON Zoltan > >> --- >> hw/i386/pc_piix.c | 2 +- >> hw/i386/pc_q35.c | 7 ++++--- >> 2 files changed, 5 insertions(+), 4 deletions(-) >> >> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c >> index ee9d9a4175..5bde4533cc 100644 >> --- a/hw/i386/pc_piix.c >> +++ b/hw/i386/pc_piix.c >> @@ -241,7 +241,7 @@ static void pc_init1(MachineState *machine, >> isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix3), "isa.0")); >> } else { >> pci_bus = NULL; >> - isa_bus = isa_bus_new(NULL, get_system_memory(), system_io, >> + isa_bus = isa_bus_new(NULL, system_memory, system_io, >> &error_abort); >> i8257_dma_init(isa_bus, 0); >> pcms->hpet_enabled = false; >> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c >> index a97846ab9b..b97979bebb 100644 >> --- a/hw/i386/pc_q35.c >> +++ b/hw/i386/pc_q35.c >> @@ -124,6 +124,7 @@ static void pc_q35_init(MachineState *machine) >> DeviceState *lpc_dev; >> BusState *idebus[MAX_SATA_PORTS]; >> ISADevice *rtc_state; >> + MemoryRegion *system_memory = get_system_memory(); >> MemoryRegion *system_io = get_system_io(); >> MemoryRegion *pci_memory; >> MemoryRegion *rom_memory; >> @@ -191,7 +192,7 @@ static void pc_q35_init(MachineState *machine) >> rom_memory = pci_memory; >> } else { >> pci_memory = NULL; >> - rom_memory = get_system_memory(); >> + rom_memory = system_memory; >> } >> >> pc_guest_info_init(pcms); >> @@ -214,7 +215,7 @@ static void pc_q35_init(MachineState *machine) >> } >> >> /* allocate ram and load rom/bios */ >> - pc_memory_init(pcms, get_system_memory(), rom_memory, &ram_memory, >> + pc_memory_init(pcms, system_memory, rom_memory, &ram_memory, >> pci_hole64_size); >> >> object_property_add_child(OBJECT(machine), "q35", OBJECT(phb)); >> @@ -223,7 +224,7 @@ static void pc_q35_init(MachineState *machine) >> object_property_set_link(OBJECT(phb), MCH_HOST_PROP_PCI_MEM, >> OBJECT(pci_memory), NULL); >> object_property_set_link(OBJECT(phb), MCH_HOST_PROP_SYSTEM_MEM, >> - OBJECT(get_system_memory()), NULL); >> + OBJECT(system_memory), NULL); >> object_property_set_link(OBJECT(phb), MCH_HOST_PROP_IO_MEM, >> OBJECT(system_io), NULL); >> object_property_set_int(OBJECT(phb), PCI_HOST_BELOW_4G_MEM_SIZE, >>
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index ee9d9a4175..5bde4533cc 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -241,7 +241,7 @@ static void pc_init1(MachineState *machine, isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix3), "isa.0")); } else { pci_bus = NULL; - isa_bus = isa_bus_new(NULL, get_system_memory(), system_io, + isa_bus = isa_bus_new(NULL, system_memory, system_io, &error_abort); i8257_dma_init(isa_bus, 0); pcms->hpet_enabled = false; diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index a97846ab9b..b97979bebb 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -124,6 +124,7 @@ static void pc_q35_init(MachineState *machine) DeviceState *lpc_dev; BusState *idebus[MAX_SATA_PORTS]; ISADevice *rtc_state; + MemoryRegion *system_memory = get_system_memory(); MemoryRegion *system_io = get_system_io(); MemoryRegion *pci_memory; MemoryRegion *rom_memory; @@ -191,7 +192,7 @@ static void pc_q35_init(MachineState *machine) rom_memory = pci_memory; } else { pci_memory = NULL; - rom_memory = get_system_memory(); + rom_memory = system_memory; } pc_guest_info_init(pcms); @@ -214,7 +215,7 @@ static void pc_q35_init(MachineState *machine) } /* allocate ram and load rom/bios */ - pc_memory_init(pcms, get_system_memory(), rom_memory, &ram_memory, + pc_memory_init(pcms, system_memory, rom_memory, &ram_memory, pci_hole64_size); object_property_add_child(OBJECT(machine), "q35", OBJECT(phb)); @@ -223,7 +224,7 @@ static void pc_q35_init(MachineState *machine) object_property_set_link(OBJECT(phb), MCH_HOST_PROP_PCI_MEM, OBJECT(pci_memory), NULL); object_property_set_link(OBJECT(phb), MCH_HOST_PROP_SYSTEM_MEM, - OBJECT(get_system_memory()), NULL); + OBJECT(system_memory), NULL); object_property_set_link(OBJECT(phb), MCH_HOST_PROP_IO_MEM, OBJECT(system_io), NULL); object_property_set_int(OBJECT(phb), PCI_HOST_BELOW_4G_MEM_SIZE,
Signed-off-by: Bernhard Beschow <shentey@gmail.com> --- hw/i386/pc_piix.c | 2 +- hw/i386/pc_q35.c | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-)