GNOME.org
 

[gnome-keyring] gcr: Review fixes for gnupg icon support

commit 7f2f8f33ee6316015661c0668fe3f63493a6e14b
Author: Stef Walter <stefw collabora co uk>
Date:   Tue Jul 12 14:53:06 2011 +0200

    gcr: Review fixes for gnupg icon support
    
     * Reviewed by Phillip Withnall
     * Memory leak fixes.
     * Documentation
     * Other minor bits.

 gcr/gcr-gnupg-collection.c        |    5 ++++
 gcr/gcr-gnupg-key.c               |   38 ++++++++++++++++++++++++++++--------
 gcr/gcr-gnupg-util.c              |   12 +++++++++++
 gcr/gcr-memory-icon.c             |   34 +++++++++++++++++++++++++++++++-
 gcr/gcr-memory-icon.h             |    2 +-
 gcr/gcr-record.c                  |   10 +++++++++
 gcr/tests/test-gnupg-collection.c |    5 ++++
 gcr/tests/test-memory-icon.c      |    2 +-
 8 files changed, 95 insertions(+), 13 deletions(-)
---
diff --git a/gcr/gcr-gnupg-collection.c b/gcr/gcr-gnupg-collection.c
index 82991e5..42d8c2c 100644
--- a/gcr/gcr-gnupg-collection.c
+++ b/gcr/gcr-gnupg-collection.c
@@ -284,6 +284,7 @@ process_records_as_public_key (GcrGnupgCollectionLoad *load, GPtrArray *records,
 {
 	GPtrArray *attr_records = NULL;
 	const gchar *fingerprint;
+	gchar *orig_fingerprint;
 	GcrGnupgKey *key;
 	guint i;
 
@@ -295,8 +296,12 @@ process_records_as_public_key (GcrGnupgCollectionLoad *load, GPtrArray *records,
 		_gcr_debug ("adding %d user id attribute(s) to key/fingerprint: %s/%s",
 		            (gint)attr_records->len, keyid, fingerprint);
 
+		if (!g_hash_table_lookup_extended (load->attributes, fingerprint,
+		                                   (gpointer*)&orig_fingerprint, NULL))
+			g_assert_not_reached ();
 		if (!g_hash_table_steal (load->attributes, fingerprint))
 			g_assert_not_reached ();
+		g_free (orig_fingerprint);
 
 		/* Move all the attribute records over to main records set */
 		for (i = 0; i < attr_records->len; i++)
diff --git a/gcr/gcr-gnupg-key.c b/gcr/gcr-gnupg-key.c
index 1274ec6..cbd4371 100644
--- a/gcr/gcr-gnupg-key.c
+++ b/gcr/gcr-gnupg-key.c
@@ -184,7 +184,7 @@ _gcr_gnupg_key_class_init (GcrGnupgKeyClass *klass)
 	gobject_class->get_property = _gcr_gnupg_key_get_property;
 
 	/**
-	 * GcrGnupgKey::public-records:
+	 * GcrGnupgKey:public-records:
 	 *
 	 * Public key data. Should always be present.
 	 */
@@ -193,7 +193,7 @@ _gcr_gnupg_key_class_init (GcrGnupgKeyClass *klass)
 	                             G_TYPE_PTR_ARRAY, G_PARAM_READWRITE));
 
 	/**
-	 * GcrGnupgKey::secret-records:
+	 * GcrGnupgKey:secret-records:
 	 *
 	 * Secret key data. The keyid of this data must match public-dataset.
 	 * If present, this key represents a secret key.
@@ -203,7 +203,7 @@ _gcr_gnupg_key_class_init (GcrGnupgKeyClass *klass)
 	                             G_TYPE_PTR_ARRAY, G_PARAM_READWRITE));
 
 	/**
-	 * GcrGnupgKey::keyid:
+	 * GcrGnupgKey:keyid:
 	 *
 	 * Key identifier.
 	 */
@@ -212,7 +212,7 @@ _gcr_gnupg_key_class_init (GcrGnupgKeyClass *klass)
 	                              "", G_PARAM_READABLE));
 
 	/**
-	 * GcrGnupgKey::label:
+	 * GcrGnupgKey:label:
 	 *
 	 * User readable label for this key.
 	 */
@@ -230,7 +230,7 @@ _gcr_gnupg_key_class_init (GcrGnupgKeyClass *klass)
 	                              "", G_PARAM_READABLE));
 
 	/**
-	 * GcrGnupgKey::markup:
+	 * GcrGnupgKey:markup:
 	 *
 	 * User readable markup which contains key label.
 	 */
@@ -239,7 +239,7 @@ _gcr_gnupg_key_class_init (GcrGnupgKeyClass *klass)
 	                              "", G_PARAM_READABLE));
 
 	/**
-	 * GcrGnupgKey::short-keyid:
+	 * GcrGnupgKey:short-keyid:
 	 *
 	 * User readable key identifier.
 	 */
@@ -248,7 +248,7 @@ _gcr_gnupg_key_class_init (GcrGnupgKeyClass *klass)
 	                              "", G_PARAM_READABLE));
 
 	/**
-	 * GcrGnupgKey::icon:
+	 * GcrGnupgKey:icon:
 	 *
 	 * Icon for this key.
 	 */
@@ -425,6 +425,14 @@ _gcr_gnupg_key_get_keyid_for_records (GPtrArray *records)
 	return NULL;
 }
 
+/**
+ * _gcr_gnupg_key_get_fingerprint_for_records:
+ * @records: Array of GcrRecord*
+ *
+ * Get the fingerprint field for some record data:
+ *
+ * Returns: (transfer none): The fingerprint.
+ */
 const gchar*
 _gcr_gnupg_key_get_fingerprint_for_records (GPtrArray *records)
 {
@@ -465,13 +473,17 @@ load_user_attribute_icon (GcrGnupgKey *self)
 		g_return_val_if_fail (data != NULL, NULL);
 
 		/* Header is 16 bytes long */
-		if (n_data <= IMAGE_HEADER_LEN)
+		if (n_data <= IMAGE_HEADER_LEN) {
+			g_free (data);
 			continue;
+		}
 
 		/* These are the header bytes. See gnupg doc/DETAILS */
 		g_assert (IMAGE_JPEG_SIG_LEN < IMAGE_HEADER_LEN);
-		if (memcmp (data, IMAGE_JPEG_SIG, IMAGE_JPEG_SIG_LEN) != 0)
+		if (memcmp (data, IMAGE_JPEG_SIG, IMAGE_JPEG_SIG_LEN) != 0) {
+			g_free (data);
 			continue;
+		}
 
 		/* We have a valid header */
 		return G_ICON (_gcr_memory_icon_new_full ("image/jpeg", data,
@@ -482,6 +494,14 @@ load_user_attribute_icon (GcrGnupgKey *self)
 	return NULL;
 }
 
+/**
+ * _gcr_gnupg_key_get_icon:
+ * @self: A gnupg key.
+ *
+ * Get the display icon for this key.
+ *
+ * Return value: (transfer none): The icon, owned by the key.
+ */
 GIcon*
 _gcr_gnupg_key_get_icon (GcrGnupgKey *self)
 {
diff --git a/gcr/gcr-gnupg-util.c b/gcr/gcr-gnupg-util.c
index 5e0025f..93677b0 100644
--- a/gcr/gcr-gnupg-util.c
+++ b/gcr/gcr-gnupg-util.c
@@ -27,6 +27,17 @@
 
 #include <gcrypt.h>
 
+/**
+ * _gcr_gnupg_build_xa1_record:
+ * @meta: Status metadata record about the attribute data.
+ * @attribute: Pointer to attribute data.
+ * @n_attribute: Length of attribute data.
+ *
+ * Build a record for attribute data. We use this records to convert attribute
+ * data into something we can keep with an array of GcrRecord.
+ *
+ * Returns: (transfer full): The newly allocated record.
+ */
 GcrRecord*
 _gcr_gnupg_build_xa1_record (GcrRecord *meta, gpointer attribute,
                              gsize n_attribute)
@@ -65,6 +76,7 @@ _gcr_gnupg_build_xa1_record (GcrRecord *meta, gpointer attribute,
 	if (expiry == NULL)
 		expiry = "";
 
+	/* These values are from gnupg doc/DETAILS */
 	if (flags & 0x02)
 		status = "r";
 	else if (flags & 0x04)
diff --git a/gcr/gcr-memory-icon.c b/gcr/gcr-memory-icon.c
index 83fb5ea..26e83a5 100644
--- a/gcr/gcr-memory-icon.c
+++ b/gcr/gcr-memory-icon.c
@@ -66,7 +66,6 @@ _gcr_memory_icon_class_init (GcrMemoryIconClass *klass)
 {
 	GObjectClass *gobject_class = G_OBJECT_CLASS (klass);
 
-	_gcr_memory_icon_parent_class = g_type_class_peek_parent (klass);
 	g_type_class_add_private (klass, sizeof (GcrMemoryIconPrivate));
 
 	gobject_class->finalize = _gcr_memory_icon_finalize;
@@ -127,6 +126,11 @@ _gcr_memory_icon_load (GLoadableIcon *icon, int size, gchar **type,
 
 	is = g_memory_input_stream_new_from_data ((guchar*)self->pv->data + self->pv->offset,
 	                                          self->pv->n_data, NULL);
+
+	/*
+	 * Hold a reference to this object from the stream, so that we can rely
+	 * on the data hanging around.
+	 */
 	g_object_set_data_full (G_OBJECT (is), "back-reference", g_object_ref (self),
 	                        g_object_unref);
 
@@ -164,16 +168,42 @@ _gcr_memory_icon_iface_loadable_icon (GLoadableIconIface *iface)
 	iface->load_finish = _gcr_memory_icon_finish;
 }
 
+/**
+ * _gcr_memory_icon_new:
+ * @image_type: MIME content-type of the image.
+ * @data: Data for the image.
+ * @n_data: Length of data.
+ *
+ * Create a new GIcon based on image data in memory. The data will be copied
+ * by the new icon.
+ *
+ * Returns: (transfer full): A newly allocated icon.
+ */
 GIcon*
 _gcr_memory_icon_new (const gchar *image_type, gconstpointer data, gsize n_data)
 {
 	g_return_val_if_fail (image_type != NULL, NULL);
 	g_return_val_if_fail (data != NULL, NULL);
+	g_return_val_if_fail (n_data != 0, NULL);
 
 	return _gcr_memory_icon_new_full (image_type, g_memdup (data, n_data),
 	                                  n_data, 0, g_free);
 }
 
+/**
+ * _gcr_memory_icon_new_full:
+ * @image_type: MIME content-type of the image.
+ * @data: Data for the image.
+ * @n_data: Length of data.
+ * @offset: Offset of the start of the image in @data.
+ * @destroy: Callback to free or release @data when no longer needed.
+ *
+ * Create a new GIcon based on image data in memory. The data will be used
+ * directly from the @data passed. Use @destroy to control the lifetime of
+ * the data in memory.
+ *
+ * Returns: (transfer full): A newly allocated icon.
+ */
 GIcon*
 _gcr_memory_icon_new_full (const gchar *image_type, gpointer data, gsize n_data,
                            goffset offset, GDestroyNotify destroy)
@@ -182,7 +212,7 @@ _gcr_memory_icon_new_full (const gchar *image_type, gpointer data, gsize n_data,
 
 	g_return_val_if_fail (image_type != NULL, NULL);
 	g_return_val_if_fail (data != NULL, NULL);
-	g_return_val_if_fail (offset <= n_data, NULL);
+	g_return_val_if_fail (offset < n_data, NULL);
 
 	self = g_object_new (GCR_TYPE_MEMORY_ICON, NULL);
 	self->pv->data = data;
diff --git a/gcr/gcr-memory-icon.h b/gcr/gcr-memory-icon.h
index c02767f..61c1542 100644
--- a/gcr/gcr-memory-icon.h
+++ b/gcr/gcr-memory-icon.h
@@ -50,7 +50,7 @@ struct _GcrMemoryIconClass {
 	GObjectClass parent_class;
 };
 
-GType               _gcr_memory_icon_get_type             (void);
+GType               _gcr_memory_icon_get_type             (void) G_GNUC_CONST;
 
 GIcon*              _gcr_memory_icon_new                  (const gchar *image_type,
                                                            gconstpointer data,
diff --git a/gcr/gcr-record.c b/gcr/gcr-record.c
index a417d0d..0ff6503 100644
--- a/gcr/gcr-record.c
+++ b/gcr/gcr-record.c
@@ -216,6 +216,16 @@ _gcr_record_get_uint (GcrRecord *record, guint column, guint *value)
 	return TRUE;
 }
 
+/**
+ * _gcr_record_get_base64:
+ * @record: The record
+ * @column: The column to decode.
+ * @n_data: Location to return size of returned data.
+ *
+ * Decode a column of a record as base64 data.
+ *
+ * Returns: (transfer full): The decoded value, or %NULL if not found.
+ */
 gpointer
 _gcr_record_get_base64 (GcrRecord *record, guint column, gsize *n_data)
 {
diff --git a/gcr/tests/test-gnupg-collection.c b/gcr/tests/test-gnupg-collection.c
index 3808596..8098865 100644
--- a/gcr/tests/test-gnupg-collection.c
+++ b/gcr/tests/test-gnupg-collection.c
@@ -135,6 +135,7 @@ test_load (Test *test, gconstpointer unused)
 	GcrGnupgKey *key;
 	GList *l, *objects;
 	GcrRecord *record;
+	GHashTable *check;
 
 	_gcr_gnupg_collection_load_async (test->collection, NULL, on_async_ready, test);
 	egg_test_wait_until (500000);
@@ -160,11 +161,15 @@ test_load (Test *test, gconstpointer unused)
 	/* The list of objects should be correct */
 	objects = gcr_collection_get_objects (GCR_COLLECTION (test->collection));
 	g_assert_cmpuint (g_hash_table_size (test->keys), ==, g_list_length (objects));
+	check = g_hash_table_new (g_str_hash, g_str_equal);
 	for (l = objects; l != NULL; l = g_list_next (l)) {
 		g_assert (GCR_IS_GNUPG_KEY (l->data));
 		key = g_hash_table_lookup (test->keys, _gcr_gnupg_key_get_keyid (l->data));
 		g_assert (key == l->data);
+		g_hash_table_replace (check, (gchar*)_gcr_gnupg_key_get_keyid (l->data), "");
 	}
+	g_assert_cmpuint (g_hash_table_size (check), ==, g_hash_table_size (test->keys));
+	g_hash_table_destroy (check);
 	g_list_free (objects);
 
 	/* Phillip R. Zimmerman's key should have a photo */
diff --git a/gcr/tests/test-memory-icon.c b/gcr/tests/test-memory-icon.c
index ebf200b..c232be1 100644
--- a/gcr/tests/test-memory-icon.c
+++ b/gcr/tests/test-memory-icon.c
@@ -37,7 +37,7 @@ typedef struct {
 	GAsyncResult *result;
 } Test;
 
-static const unsigned char test_data[256] = {
+static const guint8 test_data[256] = {
 	0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15,
 	16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31,
 	32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47,
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]