Re: [PATCH] config: rework code to use GKeyFile instead =?UTF-8?Q?of=09GHashTable+GValue?=

From: Iago Toral <itoral igalia com>
To: <grilo-list gnome org>
Subject: Re: [PATCH] config: rework code to use GKeyFile instead of GHashTable+GValue
Date: Tue, 18 Jan 2011 15:17:53 +0000


Hi Fabien,

yes, at the moment we are forcing the backend and that is somethingthat can be improved in the future, however, notice that the API doesnot expose the GKeyFile in any significant way, so in the future weshould be able to extend and enhance the current system to let usersselect other backends without big problems.


Iago

On Tue, 18 Jan 2011 15:03:54 +0000, Fabien Lebaillif - Delamare<fabien developers arq-media com> wrote:

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 somethingbased

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: LionelLandwerlin<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, constgchar *source)

  void

grl_config_set (GrlConfig *config, const gchar *param, constGValue *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, gintvalue)

  {
-  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, constgchar *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 areinvalid. " \

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

Follow-Ups:
- Re: [PATCH] config: rework code to use GKeyFile instead of GHashTable+GValue
  - From: Juan A.

References:
- [PATCH] config: rework code to use GKeyFile instead of GHashTable+GValue
  - From: llandwerlin
- Re: [PATCH] config: rework code to use GKeyFile instead of GHashTable+GValue
  - From: Fabien Lebaillif - Delamare

[Date Prev][Date Next] [Thread Prev][Thread Next] [Thread Index] [Date Index] [Author Index]