Am 10.02.2014 11:21, schrieb Eduardo Habkost: > +static const TypeInfo x86_cpu_host_type_info = { > + .name = CPU_CLASS_NAME("host"), > + .parent = TYPE_X86_CPU, > + .instance_size = sizeof(X86CPU), > + .instance_init = x86_cpu_instance_init_host, > + .abstract = false, > + .class_size = sizeof(X86CPUClass), > + .class_init = x86_cpu_class_init_host, > +};
This looks broken: .class_data is not set but the loading of the cpudef happens in the TYPE_X86_CPU initfn. My preferred solution would be to move the cpudef-loading from TYPE_X86_CPU's instance_init to a separate one specified for the models only, allowing non-cpudef-based models. Not finished investigating yet. For now I've prepended a patch implementing my generalized CPUClass::class_by_name instead of a custom x86_cpu_class_by_name(). Other style nits that I'm working on cleaning up are declarations in the middle of blocks, keeping _class_init naming convention (pretty sure my patches always had the most-specific-to-least-specific naming), strictly distinguishing between type and class, adding to my gtk-style documentation rather than new custom comments, placing struct documentation in the header and keeping the diff nicely readable AFAP. I'd further like to keep some other conventions from previous CPU subclasses, like pulling the model for loop out of the model registration function. My patches had always tried to turn what is now x86_cpu_load_def() into an instance_init function rather than calling it from one - did you have reasons not to? Did you consider converting the host model in a first step to make the patch smaller? I'd rather finish my investigations and discuss my v10 patches but Paolo is already asking whether Eduardo should send a PULL, so here's my textual reply informing of some thoughts and WIP. ;) Regards, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg