GNOME.org
 

Re: [PATCH] config: rework code to use GKeyFile instead of GHashTable+GValue


Pushed both patches (grilo, grilo-plugins).
the plugin for grilo had a bug though, you cannot free that plugin_id in grl-plugin_registry. The reason is that g_hash_table_insert keeps the original reference. Previously this reference always existed because that plugin_id was actually a static string and was never freed.
I also added the implementation for grl_config_has_param (you added the function but forgot to provide the actual implementation :)). 

Iago

On Tue, 18 Jan 2011 15:46:53 +0000, llandwerlin gmail com wrote:

From: Lionel Landwerlin <lionel g landwerlin linux intel com>
Signed-off-by: Lionel Landwerlin <lionel g landwerlin linux intel com> 
---
 src/data/grl-config.c     |  138
+++++++++++++++++++++------------------------
 src/data/grl-config.h     |   16 +++---
 src/grl-plugin-registry.c |    8 ++-
 3 files changed, 77 insertions(+), 85 deletions(-)

diff --git a/src/data/grl-config.c b/src/data/grl-config.c
index 07866b5..72a1335 100644
--- a/src/data/grl-config.c
+++ b/src/data/grl-config.c
@@ -32,6 +32,8 @@
 #include "grl-config.h"
 #include "grl-log.h"

+#define GROUP_NAME "none"
+
 #define GRL_LOG_DOMAIN_DEFAULT  config_log_domain
 GRL_LOG_DOMAIN(config_log_domain);

@@ -39,7 +41,7 @@ GRL_LOG_DOMAIN(config_log_domain);
(G_TYPE_INSTANCE_GET_PRIVATE ((o), GRL_TYPE_CONFIG, GrlConfigPrivate)) 

 struct _GrlConfigPrivate {
-  GHashTable *config;
+  GKeyFile *config;
 };

 static void grl_config_dispose (GObject *object);
@@ -48,15 +50,6 @@ static void grl_config_finalize (GObject *object);
 G_DEFINE_TYPE (GrlConfig, grl_config, G_TYPE_OBJECT);

 static void
-free_val (GValue *val)
-{
-  if (val) {
-    g_value_unset (val);
-    g_free (val);
-  }
-}
-
-static void
 grl_config_class_init (GrlConfigClass *klass)
 {
   GObjectClass *gobject_class = (GObjectClass *)klass;
@@ -71,10 +64,9 @@ static void
 grl_config_init (GrlConfig *self)
 {
   self->priv = GRL_CONFIG_GET_PRIVATE (self);
-  self->priv->config = g_hash_table_new_full (g_str_hash,
-					      g_str_equal,
-					      g_free,
-					      (GDestroyNotify) free_val);
+  self->priv->config = g_key_file_new ();
+
+  g_key_file_load_from_data (self->priv->config, "[]\n", -1,
G_KEY_FILE_NONE, NULL);
 }

 static void
@@ -124,105 +116,87 @@ grl_config_new (const gchar *plugin, const
gchar *source)
 void
grl_config_set (GrlConfig *config, const gchar *param, const GValue *value) 
 {
-  GValue *copy;
   g_return_if_fail (GRL_IS_CONFIG (config));
-  copy = g_new0 (GValue, 1);
-  g_value_init (copy, G_VALUE_TYPE (value));
-  g_value_copy (value, copy);
- g_hash_table_insert (config->priv->config, g_strdup (param), copy); 
+
+  switch (G_VALUE_TYPE (value)) {
+  case G_TYPE_STRING:
+    g_key_file_set_string (config->priv->config, GROUP_NAME, param,
+                           g_value_get_string (value));
+    break;
+
+  case G_TYPE_FLOAT:
+    g_key_file_set_double (config->priv->config, GROUP_NAME, param,
+                           g_value_get_double (value));
+    break;
+
+  case G_TYPE_INT:
+    g_key_file_set_integer (config->priv->config, GROUP_NAME, param,
+                            g_value_get_int (value));
+    break;
+
+  case G_TYPE_BOOLEAN:
+    g_key_file_set_boolean (config->priv->config, GROUP_NAME, param,
+                            g_value_get_boolean (value));
+    break;
+
+  default:
+    g_return_if_reached ();
+    break;
+  }
 }

 void
 grl_config_set_string (GrlConfig *config, const gchar *param, const
gchar *value)
 {
-  GValue v = { 0 };
-  g_value_init (&v, G_TYPE_STRING);
-  g_value_set_string (&v, value);
-  grl_config_set (config, param, &v);
-  g_value_unset (&v);
+ g_key_file_set_string (config->priv->config, GROUP_NAME, param, value); 
 }

 void
grl_config_set_int (GrlConfig *config, const gchar *param, gint value) 
 {
-  GValue v = { 0 };
-  g_value_init (&v, G_TYPE_INT);
-  g_value_set_int (&v, value);
-  grl_config_set (config, param, &v);
+ g_key_file_set_integer (config->priv->config, GROUP_NAME, param, value); 
 }


 void
grl_config_set_float (GrlConfig *config, const gchar *param, gfloat value) 
 {
-  GValue v = { 0 };
-  g_value_init (&v, G_TYPE_FLOAT);
-  g_value_set_float (&v, value);
-  grl_config_set (config, param, &v);
+  g_key_file_set_double (config->priv->config, GROUP_NAME, param,
(gdouble) value);
 }

 void
 grl_config_set_boolean (GrlConfig *config, const gchar *param,
gboolean value)
 {
-  GValue v = { 0 };
-  g_value_init (&v, G_TYPE_BOOLEAN);
-  g_value_set_boolean (&v, value);
-  grl_config_set (config, param, &v);
+ g_key_file_set_boolean (config->priv->config, GROUP_NAME, param, value); 
 }

-const GValue *
-grl_config_get (GrlConfig *config, const gchar *param)
-{
-  g_return_val_if_fail (GRL_IS_CONFIG (config), NULL);
-  return g_hash_table_lookup (config->priv->config, param);
-}
-
-const gchar *
+gchar *
 grl_config_get_string (GrlConfig *config, const gchar *param)
 {
   g_return_val_if_fail (GRL_IS_CONFIG (config), NULL);
-  const GValue *value = grl_config_get (config, param);
-  if (!value || !G_VALUE_HOLDS_STRING (value)) {
-    return NULL;
-  } else {
-    return g_value_get_string (value);
-  }
+  return g_key_file_get_string (config->priv->config, GROUP_NAME,
param, NULL);
 }

 gint
 grl_config_get_int (GrlConfig *config, const gchar *param)
 {
   g_return_val_if_fail (GRL_IS_CONFIG (config), 0);
-  const GValue *value = grl_config_get (config, param);
-  if (!value || !G_VALUE_HOLDS_INT (value)) {
-    return 0;
-  } else {
-    return g_value_get_int (value);
-  }
+  return g_key_file_get_integer (config->priv->config, GROUP_NAME,
param, NULL);
 }

 gfloat
 grl_config_get_float (GrlConfig *config, const gchar *param)
 {
   g_return_val_if_fail (GRL_IS_CONFIG (config), 0.0);
-  const GValue *value = grl_config_get (config, param);
-  if (!value || !G_VALUE_HOLDS_FLOAT (value)) {
-    return 0.0;
-  } else {
-    return g_value_get_float (value);
-  }
+ return (gfloat) g_key_file_get_double (config->priv->config, GROUP_NAME, 
+                                         param, NULL);
 }

 gboolean
 grl_config_get_boolean (GrlConfig *config, const gchar *param)
 {
   g_return_val_if_fail (GRL_IS_CONFIG (config), FALSE);
-  const GValue *value = grl_config_get (config, param);
-  if (!value || !G_VALUE_HOLDS_BOOLEAN (value)) {
-    return FALSE;
-  } else {
-    return g_value_get_boolean (value);
-  }
+  return g_key_file_get_boolean (config->priv->config, GROUP_NAME,
param, NULL);
 }

 /**
@@ -348,7 +322,7 @@ grl_config_set_password(GrlConfig *config, const
gchar *password)
  *
  * Since: 0.1.4
  */
-const gchar *
+gchar *
 grl_config_get_plugin (GrlConfig *config)
 {
   return grl_config_get_string (GRL_CONFIG (config),
@@ -363,7 +337,7 @@ grl_config_get_plugin (GrlConfig *config)
  *
  * Since: 0.1.4
  */
-const gchar *
+gchar *
 grl_config_get_api_key (GrlConfig *config)
 {
   return grl_config_get_string (GRL_CONFIG (config),
@@ -378,7 +352,7 @@ grl_config_get_api_key (GrlConfig *config)
  *
  * Since: 0.1.4
  */
-const gchar *
+gchar *
 grl_config_get_api_token (GrlConfig *config)
 {
   return grl_config_get_string (GRL_CONFIG (config),
@@ -393,7 +367,7 @@ grl_config_get_api_token (GrlConfig *config)
  *
  * Since: 0.1.4
  */
-const gchar *
+gchar *
 grl_config_get_api_secret (GrlConfig *config)
 {
   return grl_config_get_string (GRL_CONFIG (config),
@@ -406,7 +380,7 @@ grl_config_get_api_secret (GrlConfig *config)
  *
  * Returns: the username
  */
-const gchar *
+gchar *
 grl_config_get_username (GrlConfig *config)
 {
   return grl_config_get_string (GRL_CONFIG (config),
@@ -419,9 +393,25 @@ grl_config_get_username (GrlConfig *config)
  *
  * Returns: the password
  */
-const gchar *
+gchar *
 grl_config_get_password(GrlConfig *config)
 {
   return grl_config_get_string (GRL_CONFIG (config),
                                 GRL_CONFIG_KEY_PASSWORD);
 }
+
+/**
+ * grl_config_has_param:
+ * @config: the config instance
+ * @key: the param
+ *
+ * Returns: TRUE if @params has a defined value within @config, FALSE 
+ * otherwise.
+ *
+ * Since: 0.1.8
+ */
+gboolean
+grl_config_has_param (GrlConfig *config, const gchar *param)
+{
+
+}
diff --git a/src/data/grl-config.h b/src/data/grl-config.h
index b2a0f63..dbad20a 100644
--- a/src/data/grl-config.h
+++ b/src/data/grl-config.h
@@ -112,17 +112,17 @@ void grl_config_set_username (GrlConfig
*config, const gchar *secret);
void grl_config_set_password (GrlConfig *config, const gchar *secret); 

-const gchar *grl_config_get_plugin (GrlConfig *config);
+gchar *grl_config_get_plugin (GrlConfig *config);

-const gchar *grl_config_get_api_key (GrlConfig *config);
+gchar *grl_config_get_api_key (GrlConfig *config);

-const gchar *grl_config_get_api_token (GrlConfig *config);
+gchar *grl_config_get_api_token (GrlConfig *config);

-const gchar *grl_config_get_api_secret (GrlConfig *config);
+gchar *grl_config_get_api_secret (GrlConfig *config);

-const gchar *grl_config_get_username (GrlConfig *config);
+gchar *grl_config_get_username (GrlConfig *config);

-const gchar *grl_config_get_password (GrlConfig *config);
+gchar *grl_config_get_password (GrlConfig *config);

 GType grl_config_get_type (void) G_GNUC_CONST;
GrlConfig *grl_config_new (const gchar *plugin, const gchar *source); 
@@ -139,9 +139,8 @@ void grl_config_set_float (GrlConfig *config,
const gchar *param, gfloat value);

 void grl_config_set_boolean (GrlConfig *config, const gchar *param,
gboolean value);
-const GValue *grl_config_get (GrlConfig *config, const gchar *param);
-const gchar *grl_config_get_string (GrlConfig *config, const gchar *param); +gchar *grl_config_get_string (GrlConfig *config, const gchar *param); 

 gint grl_config_get_int (GrlConfig *config, const gchar *param);

@@ -149,6 +148,7 @@ gfloat grl_config_get_float (GrlConfig *config,
const gchar *param);
gboolean grl_config_get_boolean (GrlConfig *config, const gchar *param);
+gboolean grl_config_has_param (GrlConfig *config, const gchar *param); 

 G_END_DECLS

diff --git a/src/grl-plugin-registry.c b/src/grl-plugin-registry.c
index c33657f..6e20f0d 100644
--- a/src/grl-plugin-registry.c
+++ b/src/grl-plugin-registry.c
@@ -584,7 +584,7 @@ grl_plugin_registry_load_all (GrlPluginRegistry
*registry, GError **error)
"All configured plugin paths are invalid. " \ 
                           "Failed to load plugins.");
   }
-
+
   return loaded_one;
 }

@@ -882,7 +882,7 @@ grl_plugin_registry_add_config (GrlPluginRegistry
*registry,
                                 GrlConfig *config,
                                 GError **error)
 {
-  const gchar *plugin_id;
+  gchar *plugin_id;
   GList *configs = NULL;

   g_return_val_if_fail (config != NULL, FALSE);
@@ -899,7 +899,7 @@ grl_plugin_registry_add_config (GrlPluginRegistry
*registry,
     }
     return FALSE;
   }
-
+
configs = g_hash_table_lookup (registry->priv->configs, plugin_id); 
   if (configs) {
     /* Notice that we are using g_list_append on purpose to avoid
@@ -912,6 +912,8 @@ grl_plugin_registry_add_config (GrlPluginRegistry
*registry,
 			 configs);
   }

+  g_free (plugin_id);
+
   return TRUE;
 }
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]