[virt-tools-list] [libosinfo 4/5] InstallConfigParam props should map to entity params

Zeeshan Ali (Khattak) zeeshanak at gnome.org
Thu Nov 8 17:24:53 UTC 2012


From: "Zeeshan Ali (Khattak)" <zeeshanak at gnome.org>

Without entity params usage, there is no real benefit of deriving from
Entity class so this was very much intended from start.

This change also corrects the gobject property for 'policy':

1. to be of its enum type. Although this means ABI "breakage", we must keep in
   mind:

  a. The property getter (the API used by apps) was treating the string
     value as enum, i-e it was broken for apps so if any app is using this
     API, they were totally screwed anyways.

  b. Its doubtful that this new API is used by any application out there.

2. not be writable. This actually not only breaks ABI but also the API:
we remove one argument of _new() function. As said above, the prop
getter is the main part of this API that an app will be using if its
using this API at all so I think its worth it to correct this while are
are causing other breakage.
---
 osinfo/osinfo_install_config_param.c | 80 ++++++++++++++++--------------------
 osinfo/osinfo_install_config_param.h |  5 ++-
 osinfo/osinfo_loader.c               | 30 +++++++-------
 3 files changed, 56 insertions(+), 59 deletions(-)

diff --git a/osinfo/osinfo_install_config_param.c b/osinfo/osinfo_install_config_param.c
index 40d59b4..0c8b96a 100644
--- a/osinfo/osinfo_install_config_param.c
+++ b/osinfo/osinfo_install_config_param.c
@@ -65,19 +65,10 @@ osinfo_install_config_param_set_property(GObject *object,
     switch (property_id)
         {
         case PROP_NAME:
-            config_param->priv->name = g_value_dup_string(value);
+            osinfo_entity_set_param(OSINFO_ENTITY(config_param),
+                                    OSINFO_INSTALL_CONFIG_PARAM_PROP_NAME,
+                                    g_value_get_string(value));
             break;
-        case PROP_POLICY:
-            {
-            const gchar *policy = g_value_get_string(value);
-            if (g_strcmp0(policy, "required") == 0)
-                config_param->priv->policy =
-                    OSINFO_INSTALL_CONFIG_PARAM_POLICY_REQUIRED;
-            else if (g_strcmp0(policy, "optional") == 0)
-                config_param->priv->policy =
-                    OSINFO_INSTALL_CONFIG_PARAM_POLICY_OPTIONAL;
-            break;
-            }
         default:
             /* We don't have any other property... */
             G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec);
@@ -97,10 +88,20 @@ osinfo_install_config_param_get_property(GObject *object,
     switch (property_id)
         {
         case PROP_NAME:
-            g_value_set_string(value, config_param->priv->name);
+            {
+            const gchar *name;
+
+            name = osinfo_install_config_param_get_name(config_param);
+            g_value_set_string(value, name);
             break;
+            }
         case PROP_POLICY:
-            g_value_set_enum(value, config_param->priv->policy);
+            {
+            OsinfoInstallConfigParamPolicy policy;
+
+            policy = osinfo_install_config_param_get_policy(config_param);
+            g_value_set_enum(value, policy);
+            }
             break;
         default:
             /* We don't have any other property... */
@@ -154,21 +155,19 @@ osinfo_install_config_param_class_init (OsinfoInstallConfigParamClass *klass)
      *
      * The policy of the configuration parameter.
      **/
-    pspec = g_param_spec_string("policy",
-                                "Policy",
-                                _("Parameter policy"),
-                                NULL,
-                                G_PARAM_WRITABLE |
-                                G_PARAM_READABLE |
-                                G_PARAM_CONSTRUCT_ONLY |
-                                G_PARAM_STATIC_NAME |
-                                G_PARAM_STATIC_NICK |
-                                G_PARAM_STATIC_BLURB);
+    pspec = g_param_spec_enum("policy",
+                              "Policy",
+                              _("Parameter policy"),
+                              OSINFO_TYPE_INSTALL_CONFIG_PARAM_POLICY,
+                              OSINFO_INSTALL_CONFIG_PARAM_POLICY_OPTIONAL,
+                              G_PARAM_READABLE |
+                              G_PARAM_STATIC_NAME |
+                              G_PARAM_STATIC_NICK |
+                              G_PARAM_STATIC_BLURB);
     g_object_class_install_property(g_klass,
                                     PROP_POLICY,
                                     pspec);
 
-
     g_klass->finalize = osinfo_install_config_param_finalize;
 
     g_type_class_add_private (klass, sizeof (OsinfoInstallConfigParamPrivate));
@@ -189,19 +188,14 @@ osinfo_install_config_param_init (OsinfoInstallConfigParam *config_param)
 /**
  * osinfo_install_config_param_new:
  * @name: the configuration parameter name
- * @policy: the configuration parameter policy
  *
  * Construct a new configuration parameter to a #OsinfoInstallScript.
  *
  * Returns: (transfer full): the new configuration parameter
  */
-OsinfoInstallConfigParam *osinfo_install_config_param_new(const gchar *name,
-                                                          const gchar *policy)
+OsinfoInstallConfigParam *osinfo_install_config_param_new(const gchar *name)
 {
-    return g_object_new(OSINFO_TYPE_INSTALL_CONFIG_PARAM,
-                        "name", name,
-                        "policy", policy,
-                        NULL);
+    return g_object_new(OSINFO_TYPE_INSTALL_CONFIG_PARAM, "name", name, NULL);
 }
 
 /**
@@ -212,7 +206,8 @@ OsinfoInstallConfigParam *osinfo_install_config_param_new(const gchar *name,
  */
 const gchar *osinfo_install_config_param_get_name(const OsinfoInstallConfigParam *config_param)
 {
-    return config_param->priv->name;
+    return osinfo_entity_get_param_value(OSINFO_ENTITY(config_param),
+                                         OSINFO_INSTALL_CONFIG_PARAM_PROP_NAME);
 }
 
 /**
@@ -223,7 +218,10 @@ const gchar *osinfo_install_config_param_get_name(const OsinfoInstallConfigParam
  */
 OsinfoInstallConfigParamPolicy osinfo_install_config_param_get_policy(const OsinfoInstallConfigParam *config_param)
 {
-    return config_param->priv->policy;
+    return osinfo_entity_get_param_value_enum(OSINFO_ENTITY(config_param),
+                                              OSINFO_INSTALL_CONFIG_PARAM_PROP_POLICY,
+                                              OSINFO_TYPE_INSTALL_CONFIG_PARAM_POLICY,
+                                              OSINFO_INSTALL_CONFIG_PARAM_POLICY_OPTIONAL);
 }
 
 /**
@@ -235,11 +233,8 @@ OsinfoInstallConfigParamPolicy osinfo_install_config_param_get_policy(const Osin
  */
 gboolean osinfo_install_config_param_is_required(const OsinfoInstallConfigParam *config_param)
 {
-    if (config_param->priv->policy ==
-            OSINFO_INSTALL_CONFIG_PARAM_POLICY_REQUIRED)
-        return TRUE;
-
-    return FALSE;
+    return (osinfo_install_config_param_get_policy(config_param) ==
+            OSINFO_INSTALL_CONFIG_PARAM_POLICY_REQUIRED);
 }
 
 /**
@@ -251,11 +246,8 @@ gboolean osinfo_install_config_param_is_required(const OsinfoInstallConfigParam
  */
 gboolean osinfo_install_config_param_is_optional(const OsinfoInstallConfigParam *config_param)
 {
-    if (config_param->priv->policy ==
-            OSINFO_INSTALL_CONFIG_PARAM_POLICY_OPTIONAL)
-        return TRUE;
-
-    return FALSE;
+    return (osinfo_install_config_param_get_policy(config_param) ==
+            OSINFO_INSTALL_CONFIG_PARAM_POLICY_OPTIONAL);
 }
 
 /*
diff --git a/osinfo/osinfo_install_config_param.h b/osinfo/osinfo_install_config_param.h
index dbe5e23..930614d 100644
--- a/osinfo/osinfo_install_config_param.h
+++ b/osinfo/osinfo_install_config_param.h
@@ -36,6 +36,9 @@
 #define OSINFO_IS_INSTALL_CONFIG_PARAM_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), OSINFO_TYPE_INSTALL_CONFIG_PARAM))
 #define OSINFO_INSTALL_CONFIG_PARAM_GET_CLASS(obj)  (G_TYPE_INSTANCE_GET_CLASS ((obj), OSINFO_TYPE_INSTALL_CONFIG_PARAM, OsinfoInstallConfigParamClass))
 
+#define OSINFO_INSTALL_CONFIG_PARAM_PROP_NAME   "name"
+#define OSINFO_INSTALL_CONFIG_PARAM_PROP_POLICY "policy"
+
 typedef struct _OsinfoInstallConfigParam        OsinfoInstallConfigParam;
 typedef struct _OsinfoInstallConfigParamClass   OsinfoInstallConfigParamClass;
 typedef struct _OsinfoInstallConfigParamPrivate OsinfoInstallConfigParamPrivate;
@@ -66,7 +69,7 @@ struct _OsinfoInstallConfigParamClass
 
 GType osinfo_install_config_param_get_type(void);
 
-OsinfoInstallConfigParam *osinfo_install_config_param_new(const gchar *name, const gchar *policy);
+OsinfoInstallConfigParam *osinfo_install_config_param_new(const gchar *name);
 
 const gchar *osinfo_install_config_param_get_name(const OsinfoInstallConfigParam *config_param);
 
diff --git a/osinfo/osinfo_loader.c b/osinfo/osinfo_loader.c
index 4ad2d72..e7f2c71 100644
--- a/osinfo/osinfo_loader.c
+++ b/osinfo/osinfo_loader.c
@@ -566,12 +566,12 @@ static void osinfo_loader_deployment(OsinfoLoader *loader,
     osinfo_db_add_deployment(loader->priv->db, deployment);
 }
 
-static void osinfo_loader_install_config_param(OsinfoLoader *loader,
-                                               OsinfoEntity *entity,
-                                               const gchar *xpath,
-                                               xmlXPathContextPtr ctxt,
-                                               xmlNodePtr root,
-                                               GError **err)
+static void osinfo_loader_install_config_params(OsinfoLoader *loader,
+                                                OsinfoEntity *entity,
+                                                const gchar *xpath,
+                                                xmlXPathContextPtr ctxt,
+                                                xmlNodePtr root,
+                                                GError **err)
 {
     xmlNodePtr *nodes = NULL;
     int nnodes = osinfo_loader_nodeset(xpath, ctxt, &nodes, err);
@@ -582,8 +582,10 @@ static void osinfo_loader_install_config_param(OsinfoLoader *loader,
     for (i = 0 ; i < nnodes ; i++) {
         gchar *name = (gchar *)xmlGetProp(nodes[i], BAD_CAST "name");
         gchar *policy = (gchar *)xmlGetProp(nodes[i], BAD_CAST "policy");
-        OsinfoInstallConfigParam *param =
-            osinfo_install_config_param_new(name, policy);
+        OsinfoInstallConfigParam *param = osinfo_install_config_param_new(name);
+        osinfo_entity_set_param(OSINFO_ENTITY(param),
+                                OSINFO_INSTALL_CONFIG_PARAM_PROP_POLICY,
+                                policy);
         osinfo_install_script_add_config_param(OSINFO_INSTALL_SCRIPT(entity),
                                                param);
 
@@ -649,12 +651,12 @@ static void osinfo_loader_install_script(OsinfoLoader *loader,
                                 value);
     g_free(value);
 
-    osinfo_loader_install_config_param(loader,
-                                       OSINFO_ENTITY(installScript),
-                                       "./config/*",
-                                       ctxt,
-                                       root,
-                                       err);
+    osinfo_loader_install_config_params(loader,
+                                        OSINFO_ENTITY(installScript),
+                                        "./config/*",
+                                        ctxt,
+                                        root,
+                                        err);
 
     osinfo_db_add_install_script(loader->priv->db, installScript);
 
-- 
1.8.0




More information about the virt-tools-list mailing list