Re: [Qemu-devel] [PATCH 3/7] target-alpha: Add support for -cpu ?

["Andreas Färber" <afaerber@suse.de>]

Am 06.12.2012 16:42, schrieb Andreas Färber:
> Am 06.12.2012 16:37, schrieb Eduardo Habkost:
>> On Wed, Oct 31, 2012 at 04:04:00AM +0100, Andreas Färber wrote:
>>> Implement alphabetical listing of CPU subclasses.
>>>
>>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>>> ---
>>>  target-alpha/cpu.c |   41 +++++++++++++++++++++++++++++++++++++++++
>>>  target-alpha/cpu.h |    4 +++-
>>>  2 Dateien geändert, 44 Zeilen hinzugefügt(+), 1 Zeile entfernt(-)
>>>
>>> diff --git a/target-alpha/cpu.c b/target-alpha/cpu.c
>>> index e1a5739..ab25c44 100644
>>> --- a/target-alpha/cpu.c
>>> +++ b/target-alpha/cpu.c
>>> @@ -23,6 +23,47 @@
>>>  #include "qemu-common.h"
>>>  
>>>  
>>> +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)
>>> +{
>>> +    ObjectClass *class_a = (ObjectClass *)a;
>>> +    ObjectClass *class_b = (ObjectClass *)b;
>>> +    const char *name_a, *name_b;
>>> +
>>> +    name_a = object_class_get_name(class_a);
>>> +    name_b = object_class_get_name(class_b);
>>> +    return strcmp(name_a, name_b);
>>> +}
>>> +
>>> +static void alpha_cpu_list_entry(gpointer data, gpointer user_data)
>>> +{
>>> +    ObjectClass *oc = data;
>>> +    AlphaCPUListState *s = user_data;
>>> +
>>> +    (*s->cpu_fprintf)(s->file, "  %s\n",
>>> +                      object_class_get_name(oc));
>>> +}
>>> +
>>> +void alpha_cpu_list(FILE *f, fprintf_function cpu_fprintf)
>>> +{
>>> +    AlphaCPUListState s = {
>>> +        .file = f,
>>> +        .cpu_fprintf = cpu_fprintf,
>>> +    };
>>> +    GSList *list;
>>> +
>>> +    list = object_class_get_list(TYPE_ALPHA_CPU, false);
>>> +    list = g_slist_sort(list, alpha_cpu_list_compare);
>>> +    (*cpu_fprintf)(f, "Available CPUs:\n");
>>> +    g_slist_foreach(list, alpha_cpu_list_entry, &s);
>>> +    g_slist_free(list);
>>> +}
>>
>> target-arm has very similar code. Isn't it better to first write a
>> common reusable function to list CPU models using the list of
>> subclasses, instead of adding very similar functions to all
>> architectures?
> 
> Most ordering functions vary slightly (target-arm for "any"). It would
> be possible to generalize the struct and provide a wrapper with type and
> callback arguments,

Just remembered Anthony being against callbacks in this context:

http://lists.gnu.org/archive/html/qemu-devel/2012-02/msg02944.html

The RFC was for specifically for implementing the CPU lists. So I used
g_slist_* instead as suggested, which duplicates a few lines FWIW.
If someone has suggestions how else to share more code, I'm all ears.

Andreas

> but then again some functions add a header line like
> here, some don't, and some even hardcode some options like "host". For
> the targets that already had -cpu ? support before QOM I tried to keep
> output identical apart from possibly the order.
> 
> Andreas

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