Re: [Qemu-ppc] [Qemu-devel] [PATCH v3 6/7] Use DEFINE_MACHINE to register all machines

[Eduardo Habkost]

On Sat, Sep 19, 2015 at 10:13:21AM +0200, Andreas Färber wrote:
> Am 04.09.2015 um 20:37 schrieb Eduardo Habkost:
> > Convert all machines to use DEFINE_MACHINE instead of QEMUMachine
> > automatically using a script.
[...]
> > Changes v1 -> v2:
> > * Fix trailing spaces and newlines
> > * Don't touch "//" comments on lines that we are not changing
> > * Break long lines manually
> > 
> > Changes v2 -> v3:
> > * Replace "machine_machine" with "machine" in new variable and function
> >   names.
> >   Suggested-by: Michael Walle <address@hidden>
> > * Eliminate empty mips_malta_machine_init() and sun4u_machine_init()
> >   functions
> 
> In most (but not all) cases the last DEFINE_MACHINE(...) added a white
> line, dropped manually.
> 
> Your script also always added a semicolon after DEFINE_MACHINE(), even
> when the original machine_init() did not have it. Cleaned up manually.

We could use the conversion as an opportunity to make the code more
consistent: to either remove the semicolon from all files, or using a
semicolon on all of them. (I don't know which way is better.)

> 
> This scripted approach also regresses files with multiple machines to
> now have one module init function per machine rather than one per file.

Is that a problem?

> 
> I'm also not clear on why you chose to diverge from devices in having a
> machine-specific init function with MachineClass *mc arg rather than
> having ObjectClass *oc, void *data and having a local mc variable.

I wasn't 100% sure what was the best option, but I decided that I liked
to get more type safety and shorter class init functions for free when
using the macro.

> 
> But getting this conversion done to drop the old infrastructure seems
> more important, and this can be revisited afterwards.
> 

Thanks!

-- 
Eduardo