[Workman-devel] [PATCH 4/4] Add attributes property to WorkmanObject
Daniel P. Berrange
berrange at redhat.com
Fri Mar 1 09:52:15 UTC 2013
On Fri, Mar 01, 2013 at 09:51:20AM +0000, Daniel P. Berrange wrote:
> On Thu, Feb 28, 2013 at 03:51:49PM -0600, Josh Poimboeuf wrote:
> > Good finds. I made all your suggested changes:
> >
> > ---8<---
> >
> > Subject: [PATCH 2/2] Add attributes property to WorkmanObject
> >
> > - Add the "attributes" property to WorkmanObject.
> > - Remove the attribute-related overridable function pointers from the
> > class struct, since the subclasses won't be overriding them.
> > - Change the WorkmanState enum to use bit flags to make it easier to
> > deal with WORKMAN_STATE_ALL.
> > ---
> > workman/workman-object.c | 128 ++++++++++++++++++++++++++++++++++++++---------
> > workman/workman-object.h | 16 ------
> > workman/workman-state.h | 7 +--
> > 3 files changed, 107 insertions(+), 44 deletions(-)
> >
> > diff --git a/workman/workman-object.c b/workman/workman-object.c
> > index 45324bf..27606de 100644
> > --- a/workman/workman-object.c
> > +++ b/workman/workman-object.c
> > @@ -32,6 +32,7 @@ G_DEFINE_ABSTRACT_TYPE(WorkmanObject, workman_object, G_TYPE_OBJECT);
> > struct _WorkmanObjectPrivate {
> > gchar *name;
> > WorkmanState state;
> > + GList *attributes;
> > };
> >
> >
> > @@ -39,8 +40,50 @@ enum {
> > PROP_0,
> > PROP_NAME,
> > PROP_STATE,
> > + PROP_ATTRIBUTES,
> > };
> >
> > +static void
> > +workman_object_free_attributes(WorkmanObject *self)
> > +{
> > + WorkmanObjectPrivate *priv = self->priv;
> > +
> > + if (priv->attributes) {
> > + g_list_foreach(priv->attributes, (GFunc)g_object_unref, NULL);
> > + g_list_free(priv->attributes);
> > + }
> > +}
> > +
> > +
> > +static GList *
> > +workman_object_copy_attributes(GList *attributes,
> > + WorkmanState state)
> > +{
> > + GList *new_attrs, *a;
> > +
> > + new_attrs = NULL;
> > +
> > + for (a = attributes; a; a = a->next) {
> > + WorkmanAttribute *attr = a->data;
> > + if (workman_attribute_get_state(attr) & state) {
> > + g_object_ref(attr);
> > + new_attrs = g_list_append(new_attrs, attr);
> > + }
> > + }
> > +
> > + return new_attrs;
> > +}
> > +
> > +
> > +static void
> > +workman_object_set_attributes(WorkmanObject *self,
> > + GList *attributes)
> > +{
> > + workman_object_free_attributes(self);
> > + self->priv->attributes = workman_object_copy_attributes(attributes,
> > + WORKMAN_STATE_ALL);
> > +}
> > +
> >
> > static void
> > workman_object_get_property(GObject *object,
> > @@ -56,7 +99,13 @@ workman_object_get_property(GObject *object,
> > g_value_set_string(value, self->priv->name);
> > break;
> > case PROP_STATE:
> > - g_value_set_enum(value, self->priv->state);
> > + g_value_set_flags(value, self->priv->state);
> > + break;
> > + case PROP_ATTRIBUTES:
> > + attributes = workman_object_get_attributes(self,
> > + WORKMAN_STATE_ALL,
> > + NULL);
> > + g_value_set_pointer(value, attributes);
> > break;
> > default:
> > G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
> > @@ -78,7 +127,10 @@ workman_object_set_property(GObject *object,
> > self->priv->name = g_value_dup_string(value);
> > break;
> > case PROP_STATE:
> > - self->priv->state = g_value_get_enum(value);
> > + self->priv->state = g_value_get_flags(value);
> > + break;
> > + case PROP_ATTRIBUTES:
> > + workman_object_set_attributes(self, g_value_get_pointer(value));
> > break;
> > default:
> > G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
> > @@ -87,10 +139,21 @@ workman_object_set_property(GObject *object,
> > }
> >
> >
> > -void workman_object_finalize(GObject *object)
> > +static void
> > +workman_object_dispose(GObject *object)
> > +{
> > + WorkmanObject *self = WORKMAN_OBJECT(object);
> > +
> > + workman_object_free_attributes(self);
> > +
> > + G_OBJECT_CLASS(workman_object_parent_class)->dispose(object);
> > +}
> > +
> > +static void
> > +workman_object_finalize(GObject *object)
> > {
> > - WorkmanObject *attr = WORKMAN_OBJECT(object);
> > - WorkmanObjectPrivate *priv = attr->priv;
> > + WorkmanObject *obj = WORKMAN_OBJECT(object);
> > + WorkmanObjectPrivate *priv = obj->priv;
> >
> > g_free(priv->name);
> >
> > @@ -104,6 +167,7 @@ workman_object_class_init(WorkmanObjectClass *klass)
> > GObjectClass *g_klass = G_OBJECT_CLASS(klass);
> > GParamSpec *pspec;
> >
> > + g_klass->dispose = workman_object_dispose;
> > g_klass->finalize = workman_object_finalize;
> > g_klass->set_property = workman_object_set_property;
> > g_klass->get_property = workman_object_get_property;
> > @@ -117,16 +181,24 @@ workman_object_class_init(WorkmanObjectClass *klass)
> > G_PARAM_STATIC_STRINGS);
> > g_object_class_install_property(g_klass, PROP_NAME, pspec);
> >
> > - pspec = g_param_spec_enum("state",
> > - "State",
> > - "The object's WorkmanState",
> > - WORKMAN_TYPE_STATE,
> > - 0,
> > - G_PARAM_READWRITE |
> > - G_PARAM_CONSTRUCT_ONLY |
> > - G_PARAM_STATIC_STRINGS);
> > + pspec = g_param_spec_flags("state",
> > + "State",
> > + "The object's WorkmanState",
> > + WORKMAN_TYPE_STATE,
> > + 0,
> > + G_PARAM_READWRITE |
> > + G_PARAM_CONSTRUCT_ONLY |
> > + G_PARAM_STATIC_STRINGS);
> > g_object_class_install_property(g_klass, PROP_STATE, pspec);
> >
> > + pspec = g_param_spec_pointer("attributes",
> > + "Attributes",
> > + "The object's list of WorkmanAttributes",
> > + G_PARAM_READWRITE |
> > + G_PARAM_STATIC_STRINGS);
>
> Hmm, one thing that just occurred to me - use of the 'pointer' parameter
> type means that non-C language bindings only see an opaque pointer. They
> have no way of knowing it is a list, nor are they able to traverse it.
>
> Previously I would have suggested GValueArray, but that is now deprecated.
> We're supposed to use the GArray type in combination with a GValue, and
> g_param_spec_boxed AFAICT, but the docs are clear as mud on how you
> actually use this.
Oh, I should mention the actual APIs in your patch are fine to use GList
etc as you have done - only the property needs special handling.
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 :|
More information about the Workman-devel
mailing list