#19 Sorts keys before saving options.xml

Stable
closed-accepted
nobody
Ario (11)
5
2013-05-05
2011-11-27
No

This patch proposes to sort the keys before saving to avoid changing the configuration file if only the order changes. This is because I version control my configuration files and currently, each time I quit ario, I get a different configuration file even if nothing changed ; just because the order of keys changed in the hash table.

This patch fixes the problem.

Discussion

  • Marc Pavot

    Marc Pavot - 2012-01-08

    Hello,

    The content of your patch seems to be incorrect (HTML garbage) or sourceforge has a problem with it. Can you please check your patch or re-upload it?

    Thanks
    Marc

     
  • Damien Cassou

    Damien Cassou - 2012-01-09

    If I click on 'download' in the list of attached files, I get a valid patch file as far as I can tell. Here it is in case it still does not work for you:

    Index: ario-conf.c

    --- ario-conf.c (revision 770)
    +++ ario-conf.c (working copy)
    @@ -243,6 +243,15 @@
    xmlNodeAddContent (cur, (const xmlChar *) value);
    }

    +static gint
    +ario_conf_compare_keys (gconstpointer a,
    + gconstpointer b)
    +{
    + const xmlChar * str1 = (const xmlChar *) a;
    + const xmlChar * str2 = (const xmlChar *) b;
    + return xmlStrcmp (str1, str2);
    +}
    +
    static gboolean
    ario_conf_save (G_GNUC_UNUSED gpointer data)
    {
    @@ -250,6 +259,8 @@
    xmlNodePtr cur;
    xmlDocPtr doc;
    char *xml_filename;
    + guint index;
    + GList* sorted_keys;

    if (!modified)
    return TRUE;
    @@ -259,10 +270,17 @@
    cur = xmlNewNode (NULL, (const xmlChar *) XML_ROOT_NAME);
    xmlDocSetRootElement (doc, cur);

    - g_hash_table_foreach (hash,
    - (GHFunc) ario_conf_save_foreach,
    - cur);
    + /* We sort the keys before saving to avoid changing the
    + configuration file if only the order changes */
    + sorted_keys = g_list_sort (g_hash_table_get_keys (hash),
    + ario_conf_compare_keys);

    + for (index = 0; index < g_list_length (sorted_keys); ++index) {
    + gpointer key = g_list_nth_data (sorted_keys, index);
    + gpointer value = g_hash_table_lookup (hash, key);
    + ario_conf_save_foreach ((gchar *) key, (gchar *) value, cur);
    + }
    +
    xml_filename = g_build_filename (ario_util_config_dir (), "options.xml", NULL);

    /* We save the xml file */

     
  • Marc Pavot

    Marc Pavot - 2012-04-14

    Sorry for the very late reply...

    There are some issues with this patch:
    - Memory leak: value retruned by g_hash_table_get_keys must be freed
    - The loop on the list of keys + retrieve of value in the hash table is not very efficient. Best way would be to go throught the hash table only once and to build a sorted list of key/values couples.

    Can you please change that?

    Thanks
    Marc

     
  • Damien Cassou

    Damien Cassou - 2012-04-18

    --- ario-conf-orig.c 2012-04-18 13:53:29.532028246 +0200
    +++ ario-conf.c 2012-04-18 13:51:33.716029547 +0200
    @@ -229,7 +229,7 @@
    return ret;
    }

    -static void
    +static gboolean
    ario_conf_save_foreach (gchar *key,
    gchar *value,
    xmlNodePtr root)
    @@ -241,6 +241,24 @@
    cur = xmlNewChild (root, NULL, (const xmlChar *) "option", NULL);
    xmlSetProp (cur, (const xmlChar *) "key", (const xmlChar *) key);
    xmlNodeAddContent (cur, (const xmlChar *) value);
    + return FALSE;
    +}
    +
    +static gint
    +ario_conf_compare_keys (gconstpointer a,
    + gconstpointer b)
    +{
    + const xmlChar * str1 = (const xmlChar *) a;
    + const xmlChar * str2 = (const xmlChar *) b;
    + return xmlStrcmp (str1, str2);
    +}
    +
    +static void
    +ario_conf_sorted_save_foreach (gchar *key,
    + gchar *value,
    + GTree *sorted_pairs)
    +{
    + g_tree_insert(sorted_pairs, key, value);
    }

    static gboolean
    @@ -250,6 +268,7 @@
    xmlNodePtr cur;
    xmlDocPtr doc;
    char *xml_filename;
    + GTree *sorted_pairs;

    if (!modified)
    return TRUE;
    @@ -259,9 +278,16 @@
    cur = xmlNewNode (NULL, (const xmlChar *) XML_ROOT_NAME);
    xmlDocSetRootElement (doc, cur);

    - g_hash_table_foreach (hash,
    - (GHFunc) ario_conf_save_foreach,
    - cur);
    + /* We sort the keys before saving to avoid changing the
    + configuration file if only the order changes */
    + sorted_pairs=g_tree_new(ario_conf_compare_keys);
    + g_hash_table_foreach (hash,
    + (GHFunc) ario_conf_sorted_save_foreach,
    + sorted_pairs);
    + g_tree_foreach(sorted_pairs,
    + (GTraverseFunc) ario_conf_save_foreach,
    + cur);
    + g_tree_destroy(sorted_pairs);

    xml_filename = g_build_filename (ario_util_config_dir (), "options.xml", NULL);

     
  • Marc Pavot

    Marc Pavot - 2013-05-05
    • status: open --> closed-accepted
     
  • Marc Pavot

    Marc Pavot - 2013-05-05

    Pushed to Ario svn.

    Thank you
    Marc

     

Log in to post a comment.

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks