GNOME.org
 

[gnome-keyring] gcr: Add review fixes and documentation.

commit 681cc443cc514a7fe9df81da014bc07eaf99324b
Author: Stef Walter <stefw collabora co uk>
Date:   Tue Apr 19 09:12:45 2011 +0200

    gcr: Add review fixes and documentation.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=647885

 docs/reference/gcr/gcr-sections.txt |   31 ++++++++++
 gcr/gcr-colons.c                    |   27 ++++-----
 gcr/gcr-colons.h                    |    8 +--
 gcr/gcr-debug.c                     |    1 +
 gcr/gcr-debug.h                     |    1 +
 gcr/gcr-gnupg-collection.c          |  110 ++++++++++++++++++++++------------
 gcr/gcr-gnupg-key.c                 |   51 ++++++++++++++--
 gcr/gcr-library.c                   |    8 +++
 gcr/gcr-util.c                      |   11 +++-
 gcr/tests/test-colons.c             |   13 ----
 10 files changed, 178 insertions(+), 83 deletions(-)
---
diff --git a/docs/reference/gcr/gcr-sections.txt b/docs/reference/gcr/gcr-sections.txt
index 114bfb6..2f652b9 100644
--- a/docs/reference/gcr/gcr-sections.txt
+++ b/docs/reference/gcr/gcr-sections.txt
@@ -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
diff --git a/gcr/gcr-colons.c b/gcr/gcr-colons.c
index 6a5d6be..4d9c01c 100644
--- a/gcr/gcr-colons.c
+++ b/gcr/gcr-colons.c
@@ -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");
-}
diff --git a/gcr/gcr-colons.h b/gcr/gcr-colons.h
index 565df0a..5333998 100644
--- a/gcr/gcr-colons.h
+++ b/gcr/gcr-colons.h
@@ -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 */
diff --git a/gcr/gcr-debug.c b/gcr/gcr-debug.c
index 04a3530..6df2341 100644
--- a/gcr/gcr-debug.c
+++ b/gcr/gcr-debug.c
@@ -37,6 +37,7 @@ static GcrDebugFlags current_flags = 0;
 
 static GDebugKey keys[] = {
 	{ "certificate-chain", GCR_DEBUG_CERTIFICATE_CHAIN },
+	{ "parse", GCR_DEBUG_PARSE },
 	{ 0, }
 };
 
diff --git a/gcr/gcr-debug.h b/gcr/gcr-debug.h
index 46de32c..200c939 100644
--- a/gcr/gcr-debug.h
+++ b/gcr/gcr-debug.h
@@ -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);
diff --git a/gcr/gcr-gnupg-collection.c b/gcr/gcr-gnupg-collection.c
index 97f5840..c6accf4 100644
--- a/gcr/gcr-gnupg-collection.c
+++ b/gcr/gcr-gnupg-collection.c
@@ -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)
diff --git a/gcr/gcr-gnupg-key.c b/gcr/gcr-gnupg-key.c
index 225d628..874a3ba 100644
--- a/gcr/gcr-gnupg-key.c
+++ b/gcr/gcr-gnupg-key.c
@@ -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 }
 	};
diff --git a/gcr/gcr-library.c b/gcr/gcr-library.c
index c52ba38..41cedf7 100644
--- a/gcr/gcr-library.c
+++ b/gcr/gcr-library.c
@@ -62,6 +62,14 @@
  */
 
 /**
+ * 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.
diff --git a/gcr/gcr-util.c b/gcr/gcr-util.c
index 739d0c7..418b73c 100644
--- a/gcr/gcr-util.c
+++ b/gcr/gcr-util.c
@@ -27,12 +27,17 @@
 
 #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)
diff --git a/gcr/tests/test-colons.c b/gcr/tests/test-colons.c
index 1bca077..4e153d5 100644
--- a/gcr/tests/test-colons.c
+++ b/gcr/tests/test-colons.c
@@ -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);
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]