Re: [Qemu-devel] [PATCH v3 7/7] qom: don't pass string table to object_get_enum method

[Daniel P. Berrange]

On Fri, May 08, 2015 at 07:54:48PM +0200, Andreas Färber wrote:
> Am 01.05.2015 um 12:30 schrieb Daniel P. Berrange:
> > Now that properties can be explicitly registered as an enum
> > type, there is no need to pass the string table to the
> > object_get_enum method. The object property registration
> > already has a pointer to the string table.
> > 
> > In changing this method signature, the hostmem backend object
> > has to be converted to use the new enum property registration
> > code, which simplifies it somewhat.
> > 
> > Signed-off-by: Daniel P. Berrange <address@hidden>
> > ---
> >  backends/hostmem.c         | 22 ++++++++--------------
> >  include/qom/object.h       |  4 ++--
> >  numa.c                     |  2 +-
> >  qom/object.c               | 32 ++++++++++++++++++++++++--------
> >  tests/check-qom-proplist.c | 46 
> > ++++++++++++++++++++++++++++++++++++++++++++++
> >  5 files changed, 81 insertions(+), 25 deletions(-)
> > diff --git a/qom/object.c b/qom/object.c
> > index ba0e4b8..6d2a2a9 100644
> > --- a/qom/object.c
> > +++ b/qom/object.c
> > @@ -1026,13 +1026,35 @@ int64_t object_property_get_int(Object *obj, const 
> > char *name,
> >      return retval;
> >  }
> >  
> > +typedef struct EnumProperty {
> > +    const char * const *strings;
> > +    int (*get)(Object *, Error **);
> > +    void (*set)(Object *, int, Error **);
> 
> Since get and set and moved unchanged, I would prefer placing it in the
> final destination in the original patch to avoid churn.

Yep, easy to do.

> > diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c
> > index de142e3..d5cd38b 100644
> > --- a/tests/check-qom-proplist.c
> > +++ b/tests/check-qom-proplist.c
> > @@ -249,6 +249,51 @@ static void test_dummy_badenum(void)
> >  }
> >  
> >  
> > +
> > +static void test_dummy_getenum(void)
> > +{
> > +    Error *err = NULL;
> > +    int val;
> > +    Object *parent = container_get(object_get_root(),
> > +                                   "/objects");
> > +    DummyObject *dobj = DUMMY_OBJECT(
> > +        object_new_propv(TYPE_DUMMY,
> > +                         parent,
> > +                         "dummy0",
> > +                         &err,
> > +                         "av", "platypus",
> > +                         NULL));
> > +
> > +    g_assert(dobj != NULL);
> > +    g_assert(err == NULL);
> > +    g_assert(dobj->av == DUMMY_PLATYPUS);
> > +
> > +    val = object_property_get_enum(OBJECT(dobj),
> > +                                   "av",
> > +                                   "DummyAnimal",
> > +                                   &err);
> > +    g_assert(err == NULL);
> 
> Is there any significant difference between g_assert()'ing on error and
> passing &error_abort?

I didn't know about &error_abort until now :-) I will use that.

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 :|