Re: [Qemu-devel] [PATCH qom-cpu 1/4] cpu: Introduce CPUListState struct

[Andreas Färber]

Am 18.12.2012 16:59, schrieb Igor Mammedov:
> On Tue, 18 Dec 2012 08:53:40 +0100
> Andreas Färber <address@hidden> wrote:
> 
>> This generalizes {ARM,M68k,Alpha}CPUListState to avoid declaring it for
>> each target.
>>
>> Signed-off-by: Andreas Färber <address@hidden>
>> ---
>>  include/qemu/cpu.h   |   12 ++++++++++++
>>  target-alpha/cpu.c   |    9 ++-------
>>  target-arm/helper.c  |    9 ++-------
>>  target-m68k/helper.c |    9 ++-------
>>  4 Dateien geändert, 18 Zeilen hinzugefügt(+), 21 Zeilen entfernt(-)
>>
> 
> Could we use cpus.h for CPUListState?
> 
> It has related list_cpus() and it doesn't included in any headers yet.
> And we won't pollute base CPU qom definition with utility structures.

I'd be open for another header, but it's probably orthogonal to the
qemu-common.h dependency issue.

Note however that I have been moving the QOM-cleaned-up declarations of
CPU functions to qemu/cpu.h as the canonical CPU header as well.

CPUListState does not contain CPUState in any way, it's more for the
target than for the actual CPU, so that's a good point to investigate.

>> diff --git a/target-alpha/cpu.c b/target-alpha/cpu.c
>> index d065085..915278f 100644
>> --- a/target-alpha/cpu.c
>> +++ b/target-alpha/cpu.c
>> @@ -33,11 +33,6 @@ static void alpha_cpu_realize(Object *obj, Error **err)
>>  #endif
>>  }
>>  
>> -typedef struct AlphaCPUListState {
>> -    fprintf_function cpu_fprintf;
>> -    FILE *file;
>> -} AlphaCPUListState;
>> -
>>  /* Sort alphabetically by type name. */
>>  static gint alpha_cpu_list_compare(gconstpointer a, gconstpointer b)
>>  {
>> @@ -53,7 +48,7 @@ static gint alpha_cpu_list_compare(gconstpointer a,
>> gconstpointer b) static void alpha_cpu_list_entry(gpointer data, gpointer
> Perhaps *cpu_list_entry() could be generalized too, they all look alike.

Actually, apart from varying indentation and whether or not they're
prefixed with the architecture, some print more information than just
the class name. In particular other targets print just the base name for
backwards compatibility (something that I could optionally change for
alpha, where we don't have any to keep).

What I was considering was to move to a general location a GCompareFunc
comparing just the ObjectClass names. But so far there's always
exceptions to that rule (any, host, PVR order, new vs. old naming
scheme, ...) so that no two are identical yet.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg