On 08/19/13 11:24, Markus Armbruster wrote: > Laszlo Ersek <ler...@redhat.com> writes:
>> 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? The only way we've found thus far is to introduce a new alias for git-format-patch that hardwires it. > Should http://wiki.qemu.org/Contribute/SubmitAPatch ask for this? Not a bad idea, but I'm afraid new contributors don't read it because they don't know about it, and veteran contributors don't read it because they don't need it. The wiki would need a usable table of contents anyway, I have great trouble every time I want to find anything. Wikipedia probably should not have a flat sitemap for its millions of articles. The qemu wiki should, for its handful. Laszlo > >> 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! >
