GNOME.org
 

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

Hi,
Personally, the problem I see with this type of implementation, is that we are forcing the backend on developers. Using GKeyFile as the core for configuration makes it very hard to implement something based on gconf or dconf.
We should let the developer decides where the configuration is coming from and not dictate its form "it has to be a .ini type file" 

Fabien


On 1/18/2011 1:14 PM, 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     |  118 +++++++++++++++++---------------------------
  src/data/grl-config.h     |   11 ++--
  src/grl-plugin-registry.c |    8 ++-
  3 files changed, 56 insertions(+), 81 deletions(-)

diff --git a/src/data/grl-config.c b/src/data/grl-config.c
index 07866b5..62a4fe5 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);
-}
-
-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);
+  g_key_file_set_boolean (config->priv->config, GROUP_NAME, param, value);
  }

-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),
diff --git a/src/data/grl-config.h b/src/data/grl-config.h
index b2a0f63..f0f46d2 100644
--- a/src/data/grl-config.h
+++ b/src/data/grl-config.h
@@ -112,13 +112,13 @@ 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);

@@ -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);

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;
  }




--
 Fabien Lebaillif - Delamare
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]