Re: [Qemu-devel] [PATCH] Add new function qemu_register_machines

[Paul Brook]

> well, it's a matter of personal taste whether you prefer
> to have one or two interfaces for machine registration.

True. This is not a performance critical bit of code and has a well bounded 
number of occurrences so I'm inclined to go for simplicity/clarity over 
efficiency.  If we use arrays I recommend doing so consistently, even in the 
single machine case. Given your goal is to avoid repeated code I'd also use 
something along the lines of:
#define qemu_register_machines(x) qemu_do_register_machines(x, ARRAY_SIZE(x))

> The new function is an improvement if you want to reduce
> lines of code and binary code size.
>
> Removing unused entries like register_machines from
> header files is also an improvement. This part of
> my patch is fixed now in QEMU HEAD, thanks Anthony!
>
> And adding "static" to a machine declaration which is
> only used locally is also an improvement (hw/spitz.c).
>
> I assume that you agree that those last two points are
> an improvement.

Yes.

> So the patch improves two details and adds something
> where people disagree about its usefulness.
>
> What now? Will the patch be rejected because you
> don't see an improvement? Will you fix the missing
> "static" yourself? Do you want a new patch without
> qemu_register_machines? Or a new patch which uses
> qemu_register_machines for all files with more than
> one machine?

I'm never going to approve a patch until I'm happy with all of it. In general 
this is why it's a bad idea to mix cleanups and new features. In this case 
your new feature obsoletes some of your cleanups, so the case for separate 
patches is less clear.  However unless I'm feeling particularly bored or 
enthusiastic I'm unlikely to split up or rework the patch for you.

Paul