Laszlo Ersek <ler...@redhat.com> writes: > comments below > > On 08/16/13 13:13, arm...@redhat.com wrote: >> From: Markus Armbruster <arm...@redhat.com> >> >> Pass on the generic arguments unadulterated, and the machine-specific >> ones as separate argument. >> >> Signed-off-by: Markus Armbruster <arm...@redhat.com> >> Acked-by: Alexander Graf <ag...@suse.de> >> --- >> hw/ppc/e500.c | 35 ++++++++++++++++++----------------- >> hw/ppc/e500.h | 13 +++---------- >> hw/ppc/e500plat.c | 8 +------- >> hw/ppc/mpc8544ds.c | 8 +------- >> 4 files changed, 23 insertions(+), 41 deletions(-) > > Please always use > > -O/path/to/order_file > > when invoking git-format-patch. > > The contents of "order_file" should be minimally > > configure > Makefile* > *.json > *.h > *.c > > It's much easier to review a patch when "declarative changes" are shown > first (ie. in approximate logical dependency order).
Is there a way to put this in .git/config? Should http://wiki.qemu.org/Contribute/SubmitAPatch ask for this? > Then, > >> -void ppce500_init(PPCE500Params *params) >> +void ppce500_init(QEMUMachineInitArgs *args, PPCE500Params *params) >> { >> MemoryRegion *address_space_mem = get_system_memory(); >> MemoryRegion *ram = g_new(MemoryRegion, 1); >> @@ -584,8 +585,8 @@ void ppce500_init(PPCE500Params *params) >> PPCE500CCSRState *ccsr; >> >> /* Setup CPUs */ >> - if (params->cpu_model == NULL) { >> - params->cpu_model = "e500v2_v30"; >> + if (args->cpu_model == NULL) { >> + args->cpu_model = "e500v2_v30"; >> } > > As discussed before, this change will modify the "args.cpu_model" member > in main(), but that's OK. > > >> @@ -634,7 +635,7 @@ void ppce500_init(PPCE500Params *params) >> >> /* Fixup Memory size on a alignment boundary */ >> ram_size &= ~(RAM_SIZES_ALIGN - 1); >> - params->ram_size = ram_size; >> + args->ram_size = ram_size; > > This hackery (commendably left intact by the patch) is convincing me > that QEMUMachineInitArgs should not have a "ram_size" member at all. If > "ram_size" is a well-founded global, then let's treat it as such. Whatever. Global variables are often bad style (there are exceptions). Even worse is passing what is essentially global state down call chains while keeping the global variables around for random poking. And that's what we tend to do %-/ > Reviewed-by: Laszlo Ersek <ler...@redhat.com> Thanks!
