gcr: Add review fixes and documentation.
authorStef Walter <stefw@collabora.co.uk>
Tue, 19 Apr 2011 07:12:45 +0000 (09:12 +0200)
committerStef Walter <stefw@collabora.co.uk>
Tue, 19 Apr 2011 07:59:20 +0000 (09:59 +0200)
https://bugzilla.gnome.org/show_bug.cgi?id=647885

docs/reference/gcr/gcr-sections.txt
gcr/gcr-colons.c
gcr/gcr-colons.h
gcr/gcr-debug.c
gcr/gcr-debug.h
gcr/gcr-gnupg-collection.c
gcr/gcr-gnupg-key.c
gcr/gcr-library.c
gcr/gcr-util.c
gcr/tests/test-colons.c

index 114bfb6..2f652b9 100644 (file)
@@ -429,3 +429,34 @@ GCR_UNLOCK_OPTIONS_WIDGET_CLASS
 GCR_UNLOCK_OPTIONS_WIDGET_GET_CLASS
 GcrUnlockOptionsWidgetPrivate
 </SECTION>
+
+<SECTION>
+<FILE>gcr-private</FILE>
+<SUBSECTION Private>
+GCR_COLONS_SCHEMA_PUB
+GCR_COLONS_SCHEMA_UID
+GCR_GNUPG_COLLECTION
+GCR_GNUPG_COLLECTION_CLASS
+GCR_GNUPG_COLLECTION_GET_CLASS
+GCR_GNUPG_KEY
+GCR_GNUPG_KEY_CLASS
+GCR_GNUPG_KEY_COLUMNS
+GCR_GNUPG_KEY_GET_CLASS
+GCR_IS_GNUPG_COLLECTION
+GCR_IS_GNUPG_COLLECTION_CLASS
+GCR_IS_GNUPG_KEY
+GCR_IS_GNUPG_KEY_CLASS
+GCR_TYPE_GNUPG_COLLECTION
+GCR_TYPE_GNUPG_KEY
+GcrColonColumns
+GcrColonPubColumns
+GcrColonUidColumns
+GcrColons
+GcrGnupgCollection
+GcrGnupgCollectionClass
+GcrGnupgCollectionPrivate
+GcrGnupgKey
+GcrGnupgKeyClass
+GcrGnupgKeyPrivate
+GcrLineCallback
+</SECTION>
\ No newline at end of file
index 6a5d6be..4d9c01c 100644 (file)
@@ -24,6 +24,8 @@
 #include "config.h"
 
 #include "gcr-colons.h"
+#define DEBUG_FLAG GCR_DEBUG_PARSE
+#include "gcr-debug.h"
 
 #include <string.h>
 
@@ -51,6 +53,7 @@ _gcr_colons_parse (const gchar *line, gssize n_line)
        p = result->data;
        for (;;) {
                if (result->n_columns >= MAX_COLUMNS) {
+                       _gcr_debug ("too many colons in gnupg line: %.*s", n_line, line);
                        _gcr_colons_free (result);
                        return NULL;
                }
@@ -65,6 +68,7 @@ _gcr_colons_parse (const gchar *line, gssize n_line)
                p++;
        }
 
+       _gcr_debug ("parsed line %.*s into %d columns", n_line, line, result->n_columns);
        return result;
 }
 
@@ -104,8 +108,10 @@ _gcr_colons_get_string (GcrColons *colons, guint column)
        converted = g_convert (text, -1, "UTF-8", "ISO-8859-1", NULL, NULL, NULL);
        g_free (text);
 
-       if (!converted)
-               g_return_val_if_reached (NULL);
+       if (!converted) {
+               _gcr_debug ("failed to convert value from latin1 to utf-8: %s", text);
+               return NULL;
+       }
 
        return converted;
 }
@@ -115,8 +121,11 @@ _gcr_colons_get_raw (GcrColons *colons, guint column)
 {
        g_return_val_if_fail (colons, NULL);
 
-       if (column >= colons->n_columns)
+       if (column >= colons->n_columns) {
+               _gcr_debug ("only %d columns exist, tried to access %d",
+                           colons->n_columns, column);
                return NULL;
+       }
 
        return colons->columns[column];
 }
@@ -141,15 +150,3 @@ _gcr_colons_get_schema (GcrColons *colons)
                return g_quark_try_string (value);
        return 0;
 }
-
-GQuark
-_gcr_colons_get_schema_uid_quark (void)
-{
-       return g_quark_from_static_string ("uid");
-}
-
-GQuark
-_gcr_colons_get_schema_pub_quark (void)
-{
-       return g_quark_from_static_string ("pub");
-}
index 565df0a..5333998 100644 (file)
@@ -50,8 +50,8 @@
 
 G_BEGIN_DECLS
 
-#define GCR_COLONS_SCHEMA_UID  _gcr_colons_get_schema_uid_quark ()
-#define GCR_COLONS_SCHEMA_PUB  _gcr_colons_get_schema_pub_quark ()
+#define GCR_COLONS_SCHEMA_UID  (g_quark_from_static_string ("uid"))
+#define GCR_COLONS_SCHEMA_PUB  (g_quark_from_static_string ("pub"))
 
 /* Common columns for all schemas */
 typedef enum {
@@ -92,10 +92,6 @@ const gchar*   _gcr_colons_get_raw              (GcrColons *colons,
 
 GQuark         _gcr_colons_get_schema           (GcrColons *colons);
 
-GQuark         _gcr_colons_get_schema_uid_quark (void) G_GNUC_CONST;
-
-GQuark         _gcr_colons_get_schema_pub_quark (void) G_GNUC_CONST;
-
 G_END_DECLS
 
 #endif /* GCR_GNUPG_COLONS_H */
index 04a3530..6df2341 100644 (file)
@@ -37,6 +37,7 @@ static GcrDebugFlags current_flags = 0;
 
 static GDebugKey keys[] = {
        { "certificate-chain", GCR_DEBUG_CERTIFICATE_CHAIN },
+       { "parse", GCR_DEBUG_PARSE },
        { 0, }
 };
 
index 46de32c..200c939 100644 (file)
@@ -30,6 +30,7 @@ G_BEGIN_DECLS
 typedef enum {
        GCR_DEBUG_LIBRARY = 1 << 1,
        GCR_DEBUG_CERTIFICATE_CHAIN = 1 << 2,
+       GCR_DEBUG_PARSE = 1 << 3,
 } GcrDebugFlags;
 
 gboolean           _gcr_debug_flag_is_set              (GcrDebugFlags flag);
index 97f5840..c6accf4 100644 (file)
@@ -42,7 +42,7 @@ enum {
 };
 
 struct _GcrGnupgCollectionPrivate {
-       GHashTable *items;
+       GHashTable *items;          /* Map of char *keyid -> GcrGnupgKey *key */
        gchar *directory;
 };
 
@@ -98,6 +98,7 @@ _gcr_gnupg_collection_get_property (GObject *obj, guint prop_id, GValue *value,
                break;
        }
 }
+
 static void
 _gcr_gnupg_collection_dispose (GObject *obj)
 {
@@ -163,6 +164,18 @@ _gcr_collection_iface (GcrCollectionIface *iface)
        iface->get_objects = gcr_gnupg_collection_real_get_objects;
 }
 
+/**
+ * _gcr_gnupg_collection_new:
+ * @directory: The gnupg home directory, or %NULL
+ *
+ * Create a new GcrGnupgCollection.
+ *
+ * The gnupg home directory is where the keyring files live. If directory is
+ * %NULL then the default gnupg home directory is used.
+ *
+ * Returns: A newly allocated collection, which should be released with
+ *     g_object_unref().
+ */
 GcrCollection*
 _gcr_gnupg_collection_new (const gchar *directory)
 {
@@ -171,15 +184,21 @@ _gcr_gnupg_collection_new (const gchar *directory)
                             NULL);
 }
 
+/*
+ * We use @difference to track the keys that were in the collection before
+ * the load process, and then remove any not found, at the end of the load
+ * process. Strings are directly used from collection->pv->items keys.
+ */
+
 typedef struct {
-       GcrGnupgCollection *collection;
-       GPtrArray *dataset;
+       GcrGnupgCollection *collection;       /* reffed pointer back to collection */
+       GPtrArray *dataset;                   /* GcrColons* not yet made into a key */
        guint spawn_sig;
        guint child_sig;
        GPid gnupg_pid;
-       GString *out_data;
-       GString *err_data;
-       GHashTable *difference;
+       GString *out_data;                    /* Pending output not yet parsed into colons */
+       GString *err_data;                    /* Pending errors not yet printed */
+       GHashTable *difference;               /* Hashset gchar *keyid -> gchar *keyid */
 } GcrGnupgCollectionLoad;
 
 static void
@@ -330,25 +349,13 @@ on_spawn_standard_error (int fd, gpointer user_data)
 }
 
 static void
-on_each_difference_remove (gpointer key, gpointer value, gpointer user_data)
-{
-       GcrGnupgCollection *self = GCR_GNUPG_COLLECTION (user_data);
-       GObject *object;
-
-       object = g_hash_table_lookup (self->pv->items, key);
-       if (object != NULL) {
-               g_object_ref (object);
-               g_hash_table_remove (self->pv->items, key);
-               gcr_collection_emit_removed (GCR_COLLECTION (self), object);
-               g_object_unref (object);
-       }
-}
-
-static void
 on_spawn_completed (gpointer user_data)
 {
        GSimpleAsyncResult *res = G_SIMPLE_ASYNC_RESULT (user_data);
        GcrGnupgCollectionLoad *load = g_simple_async_result_get_op_res_gpointer (res);
+       GHashTableIter iter;
+       GObject *object;
+       gpointer keyid;
 
        /* Should be the last call we receive */
        g_assert (load->spawn_sig != 0);
@@ -365,7 +372,16 @@ on_spawn_completed (gpointer user_data)
                process_dataset_as_key (load);
 
        /* Remove any keys that we still have in the difference */
-       g_hash_table_foreach (load->difference, on_each_difference_remove, load->collection);
+       g_hash_table_iter_init (&iter, load->difference);
+       while (g_hash_table_iter_next (&iter, &keyid, NULL)) {
+               object = g_hash_table_lookup (load->collection->pv->items, keyid);
+               if (object != NULL) {
+                       g_object_ref (object);
+                       g_hash_table_remove (load->collection->pv->items, keyid);
+                       gcr_collection_emit_removed (GCR_COLLECTION (load->collection), object);
+                       g_object_unref (object);
+               }
+       }
 
        g_simple_async_result_complete (res);
 }
@@ -403,13 +419,16 @@ static EggSpawnCallbacks spawn_callbacks = {
        NULL
 };
 
-static void
-on_each_item_add_keyid_to_difference (gpointer key, gpointer value, gpointer user_data)
-{
-       GHashTable *difference = user_data;
-       g_hash_table_insert (difference, key, key);
-}
-
+/**
+ * _gcr_gnupg_collection_load_async:
+ * @self: The collection
+ * @cancellable: Cancellation object or %NULL
+ * @callback: Callback to call when result is ready
+ * @user_data: Data for callback
+ *
+ * Start an operation to load or reload the list of gnupg keys in this
+ * collection.
+ */
 void
 _gcr_gnupg_collection_load_async (GcrGnupgCollection *self, GCancellable *cancellable,
                                   GAsyncReadyCallback callback, gpointer user_data)
@@ -418,19 +437,20 @@ _gcr_gnupg_collection_load_async (GcrGnupgCollection *self, GCancellable *cancel
        GcrGnupgCollectionLoad *load;
        GError *error = NULL;
        GPtrArray *argv;
+       GHashTableIter iter;
+       gpointer keyid;
 
        g_return_if_fail (GCR_IS_GNUPG_COLLECTION (self));
 
-       /* Not yet implemented */
-       g_return_if_fail (cancellable == NULL);
+       /* TODO: Cancellation not yet implemented */
 
        argv = g_ptr_array_new ();
-       g_ptr_array_add (argv, GPG_EXECUTABLE);
-       g_ptr_array_add (argv, "--list-keys");
-       g_ptr_array_add (argv, "--fixed-list-mode");
-       g_ptr_array_add (argv, "--with-colons");
+       g_ptr_array_add (argv, (gpointer)GPG_EXECUTABLE);
+       g_ptr_array_add (argv, (gpointer)"--list-keys");
+       g_ptr_array_add (argv, (gpointer)"--fixed-list-mode");
+       g_ptr_array_add (argv, (gpointer)"--with-colons");
        if (self->pv->directory) {
-               g_ptr_array_add (argv, "--homedir");
+               g_ptr_array_add (argv, (gpointer)"--homedir");
                g_ptr_array_add (argv, (gpointer)self->pv->directory);
        }
        g_ptr_array_add (argv, NULL);
@@ -442,15 +462,16 @@ _gcr_gnupg_collection_load_async (GcrGnupgCollection *self, GCancellable *cancel
        load->dataset = g_ptr_array_new_with_free_func (_gcr_colons_free);
        load->err_data = g_string_sized_new (128);
        load->out_data = g_string_sized_new (1024);
-       load->difference = g_hash_table_new (g_str_hash, g_str_equal);
        load->collection = g_object_ref (self);
 
        /*
         * Track all the keys we currently have, at end remove those that
         * didn't get listed by the gpg process.
         */
-       g_hash_table_foreach (self->pv->items, on_each_item_add_keyid_to_difference,
-                             load->difference);
+       load->difference = g_hash_table_new (g_str_hash, g_str_equal);
+       g_hash_table_iter_init (&iter, self->pv->items);
+       while (g_hash_table_iter_next (&iter, &keyid, NULL))
+               g_hash_table_insert (load->difference, keyid, keyid);
 
        g_simple_async_result_set_op_res_gpointer (res, load,
                                                   _gcr_gnupg_collection_load_free);
@@ -466,6 +487,7 @@ _gcr_gnupg_collection_load_async (GcrGnupgCollection *self, GCancellable *cancel
        if (error) {
                g_simple_async_result_set_from_error (res, error);
                g_simple_async_result_complete_in_idle (res);
+               g_clear_error (&error);
        } else {
                load->child_sig = g_child_watch_add_full (G_PRIORITY_DEFAULT,
                                                          load->gnupg_pid,
@@ -475,8 +497,18 @@ _gcr_gnupg_collection_load_async (GcrGnupgCollection *self, GCancellable *cancel
        }
 
        g_object_unref (res);
+       g_ptr_array_unref (argv);
 }
 
+/**
+ * _gcr_gnupg_collection_load_finish:
+ * @self: The collection
+ * @result: The result passed to the callback
+ * @error: Location to raise an error on failure.
+ *
+ * Get the result of an operation to load or reload the list of gnupg keys
+ * in this collection.
+ */
 gboolean
 _gcr_gnupg_collection_load_finish (GcrGnupgCollection *self, GAsyncResult *result,
                                    GError **error)
index 225d628..874a3ba 100644 (file)
@@ -103,7 +103,6 @@ _gcr_gnupg_key_finalize (GObject *obj)
 
        if (self->pv->dataset)
                g_ptr_array_free (self->pv->dataset, TRUE);
-       self->pv->dataset = NULL;
 
        G_OBJECT_CLASS (_gcr_gnupg_key_parent_class)->finalize (obj);
 }
@@ -116,8 +115,7 @@ _gcr_gnupg_key_set_property (GObject *obj, guint prop_id, const GValue *value,
 
        switch (prop_id) {
        case PROP_DATASET:
-               g_return_if_fail (!self->pv->dataset);
-               self->pv->dataset = g_value_dup_boxed (value);
+               _gcr_gnupg_key_set_dataset (self, g_value_get_boxed (value));
                break;
        default:
                G_OBJECT_WARN_INVALID_PROPERTY_ID (obj, prop_id, pspec);
@@ -167,7 +165,7 @@ _gcr_gnupg_key_class_init (GcrGnupgKeyClass *klass)
 
        g_object_class_install_property (gobject_class, PROP_DATASET,
                 g_param_spec_boxed ("dataset", "Dataset", "Colon Dataset",
-                                    G_TYPE_PTR_ARRAY, G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY));
+                                    G_TYPE_PTR_ARRAY, G_PARAM_READWRITE));
 
        g_object_class_install_property (gobject_class, PROP_LABEL,
                 g_param_spec_string ("label", "Label", "Key label",
@@ -186,13 +184,30 @@ _gcr_gnupg_key_class_init (GcrGnupgKeyClass *klass)
                                      "", G_PARAM_READABLE));
 }
 
+/**
+ * _gcr_gnupg_key_new:
+ * @dataset: array of GcrColons*
+ *
+ * Create a new GcrGnupgKey for the colons data passed.
+ *
+ * Returns: A newly allocated key, which should be released with
+ *     g_object_unref().
+ */
 GcrGnupgKey*
 _gcr_gnupg_key_new (GPtrArray *dataset)
 {
+       g_return_val_if_fail (dataset, NULL);
        return g_object_new (GCR_TYPE_GNUPG_KEY, "dataset", dataset, NULL);
 }
 
-
+/**
+ * _gcr_gnupg_key_get_dataset:
+ * @self: The key
+ *
+ * Get the colons data this key is based on.
+ *
+ * Returns: An array of GcrColons*, owned by the key.
+ */
 GPtrArray*
 _gcr_gnupg_key_get_dataset (GcrGnupgKey *self)
 {
@@ -200,6 +215,13 @@ _gcr_gnupg_key_get_dataset (GcrGnupgKey *self)
        return self->pv->dataset;
 }
 
+/**
+ * _gcr_gnupg_key_set_dataset:
+ * @self: The key
+ * @dataset: The new array of GcrColons*
+ *
+ * Change the colons data that this key is based on.
+ */
 void
 _gcr_gnupg_key_set_dataset (GcrGnupgKey *self, GPtrArray *dataset)
 {
@@ -222,6 +244,14 @@ _gcr_gnupg_key_set_dataset (GcrGnupgKey *self, GPtrArray *dataset)
        g_object_thaw_notify (obj);
 }
 
+/**
+ * _gcr_gnupg_key_get_keyid_for_colons:
+ * @dataset: Array of GcrColons*
+ *
+ * Get the keyid for some colons data.
+ *
+ * Returns: The keyid, owned by the colons data.
+ */
 const gchar*
 _gcr_gnupg_key_get_keyid_for_colons (GPtrArray *dataset)
 {
@@ -234,13 +264,20 @@ _gcr_gnupg_key_get_keyid_for_colons (GPtrArray *dataset)
        return _gcr_colons_get_raw (colons, GCR_COLONS_PUB_KEYID);
 }
 
+/**
+ * _gcr_gnupg_key_get_columns:
+ *
+ * Get the columns that we should display for gnupg keys.
+ *
+ * Returns: The columns, NULL terminated, should not be freed.
+ */
 const GcrColumn*
 _gcr_gnupg_key_get_columns (void)
 {
        static GcrColumn columns[] = {
-               { "label", G_TYPE_STRING, G_TYPE_STRING, N_("Name"),
+               { "label", G_TYPE_STRING, G_TYPE_STRING, NC_("column", "Name"),
                  GCR_COLUMN_SORTABLE },
-               { "keyid", G_TYPE_STRING, G_TYPE_STRING, N_("Key ID"),
+               { "keyid", G_TYPE_STRING, G_TYPE_STRING, NC_("column", "Key ID"),
                  GCR_COLUMN_SORTABLE },
                { NULL }
        };
index 38ff811..73a79ab 100644 (file)
  */
 
 /**
+ * SECTION:gcr-private
+ * @title: Private declarations
+ * @short_description: private declarations to supress warnings.
+ *
+ * This section is only here to supress warnings, and should not be displayed.
+ */
+
+/**
  * GCR_DATA_ERROR:
  *
  * The #GError domain for data parsing errors.
index 739d0c7..418b73c 100644 (file)
 
 #include <string.h>
 
-/*
+/**
+ * _gcr_util_parse_lines:
+ * @string: The string to parse lines from, will be modified
+ * @last_line: Whether or not we should run for last line or not
+ * @callback: Call for each line
+ * @user_data: Data for callback
+ *
  * Calls callback for each line. If last_line, also sends the remainder
  * data that comes after the last line break. \n and \r\n are line separators.
- * Neither are sent.
+ * Neither are included in data passed to callback.
  */
-
 void
 _gcr_util_parse_lines (GString *string, gboolean last_line,
                        GcrLineCallback callback, gpointer user_data)
index 1bca077..4e153d5 100644 (file)
@@ -161,18 +161,6 @@ test_get_schema (Test *test, gconstpointer unused)
        g_assert_cmpstr (g_quark_to_string (schema), ==, "one");
 }
 
-static void
-test_schemas (void)
-{
-       GQuark check;
-
-       check = _gcr_colons_get_schema_uid_quark ();
-       g_assert_cmpstr (g_quark_to_string (check), ==, "uid");
-
-       check = _gcr_colons_get_schema_pub_quark ();
-       g_assert_cmpstr (g_quark_to_string (check), ==, "pub");
-}
-
 int
 main (int argc, char **argv)
 {
@@ -182,7 +170,6 @@ main (int argc, char **argv)
        g_test_add_func ("/gcr/colons/parse_part", test_parse_part);
        g_test_add_func ("/gcr/colons/parse_too_long", test_parse_too_long);
        g_test_add_func ("/gcr/colons/free_null", test_free_null);
-       g_test_add_func ("/gcr/colons/schemas", test_schemas);
        g_test_add_func ("/gcr/colons/find", test_find);
        g_test_add ("/gcr/colons/get_string", Test, NULL, setup, test_get_string, teardown);
        g_test_add ("/gcr/colons/get_string_null", Test, NULL, setup, test_get_string_null, teardown);