On Fri, May 08, 2015 at 07:45:10PM +0200, Andreas Färber wrote: > Am 01.05.2015 um 12:30 schrieb Daniel P. Berrange: > > Looks good in general. Some minor nits below, and one limitation > possibly worth mentioning in the second paragraph of the commit message: > It assumes a 1:1 mapping. I do guess most ones are, but I remember some > CPUID bits having different names for the same values, for instance.
Worst case, in that edge case, we can simply not use this stricter enum property type - just carry on with existing custom property > > diff --git a/include/qom/object.h b/include/qom/object.h > > index bf76f7a..f6a2a9d 100644 > > --- a/include/qom/object.h > > +++ b/include/qom/object.h > > @@ -1271,6 +1271,25 @@ void object_property_add_bool(Object *obj, const > > char *name, > > Error **errp); > > > > /** > > + * object_property_add_enum: > > + * @obj: the object to add a property to > > + * @name: the name of the property > > + * @typename: the name of the enum data type > > + * @get: the getter or NULL if the property is write-only. > > %NULL > > > + * @set: the setter or NULL if the property is read-only > > + * @errp: if an error occurs, a pointer to an area to store the error > > + * > > + * Add a enum property using getters/setters. This function will add a > > + * property of type 'enum'. > > This is slightly ambiguous, as I understand it the type we're actually > using is the one in @typename, not "enum"? Yeah, you are right - this doc mistake is left over from a previous version before Paolo asked me to add the @typename parameter. > > diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c > > index 9f16cdb..de142e3 100644 > > --- a/tests/check-qom-proplist.c > > +++ b/tests/check-qom-proplist.c > > @@ -32,10 +32,28 @@ typedef struct DummyObjectClass DummyObjectClass; > > #define DUMMY_OBJECT(obj) \ > > OBJECT_CHECK(DummyObject, (obj), TYPE_DUMMY) > > > > +typedef enum DummyAnimal DummyAnimal; > > + > > +enum DummyAnimal { > > + DUMMY_FROG, > > + DUMMY_ALLIGATOR, > > + DUMMY_PLATYPUS, > > + > > + DUMMY_LAST, > > +}; > > + > > +static const char *const dummyanimalmap[DUMMY_LAST + 1] = { > > dummy_animal_map would be slightly easier to read. Sure > > +static void test_dummy_badenum(void) > > +{ > > + Error *err = NULL; > > + Object *parent = container_get(object_get_root(), > > + "/objects"); > > + DummyObject *dobj = DUMMY_OBJECT( > > + object_new_propv(TYPE_DUMMY, > > + parent, > > + "dummy0", > > + &err, > > + "bv", "yes", > > + "sv", "Hiss hiss hiss", > > + "av", "yeti", > > + NULL)); > > + > > + g_assert(dobj == NULL); > > Superfluous. > > > + g_assert(err != NULL); > > + g_assert(g_str_equal(error_get_pretty(err), > > + "Invalid parameter 'yeti'")); > > Same question as in previous test: alternatives? Yep, will check Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|