[Workman-devel] [PATCH 4/4] Add attributes property to WorkmanObject
Josh Poimboeuf
jpoimboe at redhat.com
Thu Feb 28 21:51:49 UTC 2013
On Thu, Feb 28, 2013 at 10:40:37AM +0000, Daniel P. Berrange wrote:
> On Wed, Feb 27, 2013 at 05:55:37PM -0600, Josh Poimboeuf wrote:
> > - 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 | 110 ++++++++++++++++++++++++++++++++++++++++-------
> > workman/workman-object.h | 16 -------
> > workman/workman-state.h | 7 +--
> > 3 files changed, 99 insertions(+), 34 deletions(-)
> >
> > diff --git a/workman/workman-object.c b/workman/workman-object.c
> > index 11bf434..d22e0a8 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,
> > @@ -58,6 +101,12 @@ workman_object_get_property(GObject *object,
> > case PROP_STATE:
> > g_value_set_uint(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);
> > break;
> > @@ -80,6 +129,9 @@ workman_object_set_property(GObject *object,
> > case PROP_STATE:
> > self->priv->state = g_value_get_uint(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);
> > break;
> > @@ -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;
> > @@ -128,6 +192,14 @@ workman_object_class_init(WorkmanObjectClass *klass)
> > 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);
> > +
> > + g_object_class_install_property(g_klass, PROP_ATTRIBUTES, pspec);
> > +
> > g_type_class_add_private(klass, sizeof(WorkmanObjectPrivate));
> > }
> >
> > @@ -151,12 +223,11 @@ const gchar *workman_object_get_name(WorkmanObject *obj)
> > *
> > * Returns: (transfer full) (element-type WorkmanAttribute): the attributes for this object
> > */
> > -GList *workman_object_get_attributes(WorkmanObject *obj,
> > +GList *workman_object_get_attributes(WorkmanObject *self,
> > WorkmanState state,
> > GError **error)
> > {
> > - WorkmanObjectClass *klass = WORKMAN_OBJECT_GET_CLASS(obj);
> > - return klass->get_attributes(obj, state, error);
> > + return workman_object_copy_attributes(self->priv->attributes, state);
> > }
> >
> >
> > @@ -165,31 +236,40 @@ GList *workman_object_get_attributes(WorkmanObject *obj,
> > *
> > * Returns: (transfer full): the attribute or NULL
> > */
> > -WorkmanAttribute *workman_object_get_attribute(WorkmanObject *obj,
> > +WorkmanAttribute *workman_object_get_attribute(WorkmanObject *self,
> > WorkmanState state,
> > const gchar *name,
> > GError **error)
> > {
> > - WorkmanObjectClass *klass = WORKMAN_OBJECT_GET_CLASS(obj);
> > - return klass->get_attribute(obj, state, name, error);
> > + GList *a;
> > + WorkmanAttribute *attr;
> > +
> > + for (a = self->priv->attributes; a; a = a->next) {
> > + attr = a->data;
> > + if (workman_attribute_get_state(attr) == state &&
> > + !strcmp(workman_attribute_get_name(attr), name)) {
>
> You can use g_str_equal() instead of !strcmp
>
> > + g_object_ref(a);
> > + return attr;
>
> Instead of these two lines (with typo in first) just use
>
> return g_object_ref(attr);
>
> > + }
> > + }
> > +
> > + return NULL;
> > }
> >
> >
> > -gboolean workman_object_refresh_attributes(WorkmanObject *obj,
> > +gboolean workman_object_refresh_attributes(WorkmanObject *self,
> > WorkmanState state,
> > GError **error)
> > {
> > - WorkmanObjectClass *klass = WORKMAN_OBJECT_GET_CLASS(obj);
> > - return klass->refresh_attributes(obj, state, error);
> > + /* TODO: call plugin->get_attributes and update obj->priv->attributes */
> > }
> >
> >
> > -gboolean workman_object_save_attributes(WorkmanObject *obj,
> > +gboolean workman_object_save_attributes(WorkmanObject *self,
> > WorkmanState state,
> > GError **error)
> > {
> > - WorkmanObjectClass *klass = WORKMAN_OBJECT_GET_CLASS(obj);
> > - return klass->save_attributes(obj, state, error);
> > + /* TODO: call plugin->update_attribute for each changed attribute */
> > }
> >
> >
> > diff --git a/workman/workman-object.h b/workman/workman-object.h
> > index 7754783..88eb27c 100644
> > --- a/workman/workman-object.h
> > +++ b/workman/workman-object.h
> > @@ -59,22 +59,6 @@ struct _WorkmanObjectClass
> > GObjectClass parent_class;
> >
> >
> > - /* class members */
> > - GList *(*get_attributes)(WorkmanObject *obj,
> > - WorkmanState state,
> > - GError **error);
> > - WorkmanAttribute *(*get_attribute)(WorkmanObject *obj,
> > - WorkmanState state,
> > - const gchar *name,
> > - GError **error);
> > - gboolean (*refresh_attributes)(WorkmanObject *obj,
> > - WorkmanState state,
> > - GError **error);
> > - gboolean (*save_attributes)(WorkmanObject *obj,
> > - WorkmanState state,
> > - GError **error);
> > -
> > -
> > /* Remove from padding when adding new virtual functions */
> > gpointer padding[20];
> > };
> > diff --git a/workman/workman-state.h b/workman/workman-state.h
> > index d3a86f2..2e8072d 100644
> > --- a/workman/workman-state.h
> > +++ b/workman/workman-state.h
> > @@ -31,9 +31,10 @@ G_BEGIN_DECLS
> >
> >
> > typedef enum {
> > - WORKMAN_STATE_ALL,
> > - WORKMAN_STATE_ACTIVE,
> > - WORKMAN_STATE_PERSISTENT,
> > + WORKMAN_STATE_ACTIVE = 1 << 0,
> > + WORKMAN_STATE_PERSISTENT = 1 << 1,
> > + WORKMAN_STATE_ALL = WORKMAN_STATE_ACTIVE |
> > + WORKMAN_STATE_PERSISTENT,
>
> If you do this, then wrt my point about using g_param_spec_enum
> in the previous patch - this will imply a change to use
> g_param_spec_flags instead.
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);
+
+ g_object_class_install_property(g_klass, PROP_ATTRIBUTES, pspec);
+
g_type_class_add_private(klass, sizeof(WorkmanObjectPrivate));
}
@@ -150,12 +222,11 @@ const gchar *workman_object_get_name(WorkmanObject *obj)
*
* Returns: (transfer full) (element-type WorkmanAttribute): the attributes for this object
*/
-GList *workman_object_get_attributes(WorkmanObject *obj,
+GList *workman_object_get_attributes(WorkmanObject *self,
WorkmanState state,
GError **error)
{
- WorkmanObjectClass *klass = WORKMAN_OBJECT_GET_CLASS(obj);
- return klass->get_attributes(obj, state, error);
+ return workman_object_copy_attributes(self->priv->attributes, state);
}
@@ -164,31 +235,38 @@ GList *workman_object_get_attributes(WorkmanObject *obj,
*
* Returns: (transfer full): the attribute or NULL
*/
-WorkmanAttribute *workman_object_get_attribute(WorkmanObject *obj,
+WorkmanAttribute *workman_object_get_attribute(WorkmanObject *self,
WorkmanState state,
const gchar *name,
GError **error)
{
- WorkmanObjectClass *klass = WORKMAN_OBJECT_GET_CLASS(obj);
- return klass->get_attribute(obj, state, name, error);
+ GList *a;
+ WorkmanAttribute *attr;
+
+ for (a = self->priv->attributes; a; a = a->next) {
+ attr = a->data;
+ if (workman_attribute_get_state(attr) == state &&
+ g_str_equal(workman_attribute_get_name(attr), name))
+ return g_object_ref(attr);
+ }
+
+ return NULL;
}
-gboolean workman_object_refresh_attributes(WorkmanObject *obj,
+gboolean workman_object_refresh_attributes(WorkmanObject *self,
WorkmanState state,
GError **error)
{
- WorkmanObjectClass *klass = WORKMAN_OBJECT_GET_CLASS(obj);
- return klass->refresh_attributes(obj, state, error);
+ /* TODO: call plugin->get_attributes and update obj->priv->attributes */
}
-gboolean workman_object_save_attributes(WorkmanObject *obj,
+gboolean workman_object_save_attributes(WorkmanObject *self,
WorkmanState state,
GError **error)
{
- WorkmanObjectClass *klass = WORKMAN_OBJECT_GET_CLASS(obj);
- return klass->save_attributes(obj, state, error);
+ /* TODO: call plugin->update_attribute for each changed attribute */
}
diff --git a/workman/workman-object.h b/workman/workman-object.h
index 7754783..88eb27c 100644
--- a/workman/workman-object.h
+++ b/workman/workman-object.h
@@ -59,22 +59,6 @@ struct _WorkmanObjectClass
GObjectClass parent_class;
- /* class members */
- GList *(*get_attributes)(WorkmanObject *obj,
- WorkmanState state,
- GError **error);
- WorkmanAttribute *(*get_attribute)(WorkmanObject *obj,
- WorkmanState state,
- const gchar *name,
- GError **error);
- gboolean (*refresh_attributes)(WorkmanObject *obj,
- WorkmanState state,
- GError **error);
- gboolean (*save_attributes)(WorkmanObject *obj,
- WorkmanState state,
- GError **error);
-
-
/* Remove from padding when adding new virtual functions */
gpointer padding[20];
};
diff --git a/workman/workman-state.h b/workman/workman-state.h
index d3a86f2..2e8072d 100644
--- a/workman/workman-state.h
+++ b/workman/workman-state.h
@@ -31,9 +31,10 @@ G_BEGIN_DECLS
typedef enum {
- WORKMAN_STATE_ALL,
- WORKMAN_STATE_ACTIVE,
- WORKMAN_STATE_PERSISTENT,
+ WORKMAN_STATE_ACTIVE = 1 << 0,
+ WORKMAN_STATE_PERSISTENT = 1 << 1,
+ WORKMAN_STATE_ALL = WORKMAN_STATE_ACTIVE |
+ WORKMAN_STATE_PERSISTENT,
} WorkmanState;
--
1.7.11.7
More information about the Workman-devel
mailing list