Refactor signature result return format

I was trying to take a shortcut and not introduce a wrapper struct for
the signature results, so packed it all into alpm_sigresult_t in the
first iteration. However, this is painful when one wants to add new
fields or only return information regarding a single signature.

Refactor the type into a few components which are exposed to the end
user, and will allow a lot more future flexibility. This also exposes
more information regarding the key to the frontend than was previously
available.

The "private" void *data pointer is used by the library to store the
actual key object returned by gpgme; it is typed this way so the
frontend has no expectations of what is there, and so we don't have any
hard gpgme requirement in our public API.

Signed-off-by: Dan McGee <dan@archlinux.org>

6 files changed, 82 insertions, 68 deletions

diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
index fc8f0bcd..c94cdf7f 100644
--- a/lib/libalpm/alpm.h
+++ b/lib/libalpm/alpm.h
@@ -209,16 +209,30 @@ typedef struct _alpm_backup_t {
 	char *hash;
 } alpm_backup_t;
 
+typedef struct _alpm_pgpkey_t {
+	void *data;
+	char *fingerprint;
+	char *uid;
+	char *name;
+	char *email;
+	time_t created;
+	time_t expires;
+} alpm_pgpkey_t;
+
 /** Signature result. Contains the number of signatures found and pointers to
  * arrays containing key and status info. All contained arrays have size
  * #count.*/
 typedef struct _alpm_sigresult_t {
-	int count;
-	alpm_sigstatus_t *status;
-	alpm_sigvalidity_t *validity;
-	char **uid;
+	alpm_pgpkey_t key;
+	alpm_sigstatus_t status;
+	alpm_sigvalidity_t validity;
 } alpm_sigresult_t;
 
+typedef struct _alpm_siglist_t {
+	size_t count;
+	alpm_sigresult_t *results;
+} alpm_siglist_t;
+
 /*
  * Logging facilities
  */
@@ -776,11 +790,11 @@ alpm_list_t *alpm_pkg_unused_deltas(alpm_pkg_t *pkg);
  * Signatures
  */
 
-int alpm_pkg_check_pgp_signature(alpm_pkg_t *pkg, alpm_sigresult_t *result);
+int alpm_pkg_check_pgp_signature(alpm_pkg_t *pkg, alpm_siglist_t *siglist);
 
-int alpm_db_check_pgp_signature(alpm_db_t *db, alpm_sigresult_t *result);
+int alpm_db_check_pgp_signature(alpm_db_t *db, alpm_siglist_t *siglist);
 
-int alpm_sigresult_cleanup(alpm_sigresult_t *result);
+int alpm_siglist_cleanup(alpm_siglist_t *siglist);
 
 /*
  * Groups
diff --git a/lib/libalpm/signing.c b/lib/libalpm/signing.c
index 5beea5e1..ba5efa5d 100644
--- a/lib/libalpm/signing.c
+++ b/lib/libalpm/signing.c
@@ -196,16 +196,16 @@ error:
  * The return value will be 0 if nothing abnormal happened during the signature
  * check, and -1 if an error occurred while checking signatures or if a
  * signature could not be found; pm_errno will be set. Note that "abnormal"
- * does not include a failed signature; the value in #result should be checked
+ * does not include a failed signature; the value in #siglist should be checked
  * to determine if the signature(s) are good.
  * @param handle the context handle
  * @param path the full path to a file
  * @param base64_sig optional PGP signature data in base64 encoding
- * @result
+ * @param siglist a pointer to storage for signature results
  * @return 0 in normal cases, -1 if the something failed in the check process
  */
 int _alpm_gpgme_checksig(alpm_handle_t *handle, const char *path,
-		const char *base64_sig, alpm_sigresult_t *result)
+		const char *base64_sig, alpm_siglist_t *siglist)
 {
 	int ret = -1, sigcount;
 	gpgme_error_t err;
@@ -221,10 +221,10 @@ int _alpm_gpgme_checksig(alpm_handle_t *handle, const char *path,
 		RET_ERR(handle, ALPM_ERR_NOT_A_FILE, -1);
 	}
 
-	if(!result) {
+	if(!siglist) {
 		RET_ERR(handle, ALPM_ERR_WRONG_ARGS, -1);
 	}
-	result->count = 0;
+	siglist->count = 0;
 
 	if(!base64_sig) {
 		sigpath = _alpm_sigpath(handle, path);
@@ -294,14 +294,9 @@ int _alpm_gpgme_checksig(alpm_handle_t *handle, const char *path,
 			gpgsig; gpgsig = gpgsig->next, sigcount++);
 	_alpm_log(handle, ALPM_LOG_DEBUG, "%d signatures returned\n", sigcount);
 
-	result->status = calloc(sigcount, sizeof(alpm_sigstatus_t));
-	result->validity = calloc(sigcount, sizeof(alpm_sigvalidity_t));
-	result->uid = calloc(sigcount, sizeof(char*));
-	if(!result->status || !result->validity || !result->uid) {
-		handle->pm_errno = ALPM_ERR_MEMORY;
-		goto error;
-	}
-	result->count = sigcount;
+	CALLOC(siglist->results, sigcount, sizeof(alpm_sigresult_t),
+			handle->pm_errno = ALPM_ERR_MEMORY; goto error);
+	siglist->count = sigcount;
 
 	for(gpgsig = verify_result->signatures, sigcount = 0; gpgsig;
 			gpgsig = gpgsig->next, sigcount++) {
@@ -309,6 +304,7 @@ int _alpm_gpgme_checksig(alpm_handle_t *handle, const char *path,
 		alpm_sigstatus_t status;
 		alpm_sigvalidity_t validity;
 		gpgme_key_t key;
+		alpm_sigresult_t *result;
 
 		_alpm_log(handle, ALPM_LOG_DEBUG, "fingerprint: %s\n", gpgsig->fpr);
 		summary_list = list_sigsum(gpgsig->summary);
@@ -323,21 +319,26 @@ int _alpm_gpgme_checksig(alpm_handle_t *handle, const char *path,
 				string_validity(gpgsig->validity),
 				gpgme_strerror(gpgsig->validity_reason));
 
+		result = siglist->results + sigcount;
 		err = gpgme_get_key(ctx, gpgsig->fpr, &key, 0);
 		if(gpg_err_code(err) == GPG_ERR_EOF) {
 			_alpm_log(handle, ALPM_LOG_DEBUG, "key lookup failed, unknown key\n");
 			err = GPG_ERR_NO_ERROR;
-			STRDUP(result->uid[sigcount], gpgsig->fpr,
+			/* we dupe the fpr in this case since we have no key to point at */
+			STRDUP(result->key.fingerprint, gpgsig->fpr,
 					handle->pm_errno = ALPM_ERR_MEMORY; goto error);
 		} else {
 			CHECK_ERR();
 			if(key->uids) {
-				const char *uid = key->uids->uid;
-				STRDUP(result->uid[sigcount], uid,
-						handle->pm_errno = ALPM_ERR_MEMORY; goto error);
-				_alpm_log(handle, ALPM_LOG_DEBUG, "key user: %s\n", uid);
+				result->key.data = key;
+				result->key.fingerprint = key->subkeys->fpr;
+				result->key.uid = key->uids->uid;
+				result->key.name = key->uids->name;
+				result->key.email = key->uids->email;
+				result->key.created = key->subkeys->timestamp;
+				result->key.expires = key->subkeys->expires;
+				_alpm_log(handle, ALPM_LOG_DEBUG, "key user: %s\n", key->uids->uid);
 			}
-			gpgme_key_unref(key);
 		}
 
 		switch(gpg_err_code(gpgsig->status)) {
@@ -379,8 +380,8 @@ int _alpm_gpgme_checksig(alpm_handle_t *handle, const char *path,
 				break;
 		}
 
-		result->status[sigcount] = status;
-		result->validity[sigcount] = validity;
+		result->status = status;
+		result->validity = validity;
 	}
 
 	ret = 0;
@@ -435,13 +436,13 @@ char *_alpm_sigpath(alpm_handle_t *handle, const char *path)
 int _alpm_check_pgp_helper(alpm_handle_t *handle, const char *path,
 		const char *base64_sig, int optional, int marginal, int unknown)
 {
-	alpm_sigresult_t result;
+	alpm_siglist_t siglist;
 	int ret;
 
-	memset(&result, 0, sizeof(result));
+	memset(&siglist, 0, sizeof(alpm_siglist_t));
 
 	_alpm_log(handle, ALPM_LOG_DEBUG, "checking signatures for %s\n", path);
-	ret = _alpm_gpgme_checksig(handle, path, base64_sig, &result);
+	ret = _alpm_gpgme_checksig(handle, path, base64_sig, &siglist);
 	if(ret && handle->pm_errno == ALPM_ERR_SIG_MISSING) {
 		if(optional) {
 			_alpm_log(handle, ALPM_LOG_DEBUG, "missing optional signature\n");
@@ -455,13 +456,13 @@ int _alpm_check_pgp_helper(alpm_handle_t *handle, const char *path,
 		_alpm_log(handle, ALPM_LOG_DEBUG, "signature check failed\n");
 		/* ret will already be -1 */
 	} else {
-		int num;
-		for(num = 0; !ret && num < result.count; num++) {
-			switch(result.status[num]) {
+		size_t num;
+		for(num = 0; !ret && num < siglist.count; num++) {
+			switch(siglist.results[num].status) {
 				case ALPM_SIGSTATUS_VALID:
 				case ALPM_SIGSTATUS_KEY_EXPIRED:
 					_alpm_log(handle, ALPM_LOG_DEBUG, "signature is valid\n");
-					switch(result.validity[num]) {
+					switch(siglist.results[num].validity) {
 						case ALPM_SIGVALIDITY_FULL:
 							_alpm_log(handle, ALPM_LOG_DEBUG, "signature is fully trusted\n");
 							break;
@@ -493,7 +494,7 @@ int _alpm_check_pgp_helper(alpm_handle_t *handle, const char *path,
 		}
 	}
 
-	alpm_sigresult_cleanup(&result);
+	alpm_siglist_cleanup(&siglist);
 	return ret;
 }
 
@@ -503,14 +504,14 @@ int _alpm_check_pgp_helper(alpm_handle_t *handle, const char *path,
  * @return a int value : 0 (valid), 1 (invalid), -1 (an error occurred)
  */
 int SYMEXPORT alpm_pkg_check_pgp_signature(alpm_pkg_t *pkg,
-		alpm_sigresult_t *result)
+		alpm_siglist_t *siglist)
 {
 	ASSERT(pkg != NULL, return -1);
-	ASSERT(result != NULL, RET_ERR(pkg->handle, ALPM_ERR_WRONG_ARGS, -1));
+	ASSERT(siglist != NULL, RET_ERR(pkg->handle, ALPM_ERR_WRONG_ARGS, -1));
 	pkg->handle->pm_errno = 0;
 
 	return _alpm_gpgme_checksig(pkg->handle, alpm_pkg_get_filename(pkg),
-			pkg->base64_sig, result);
+			pkg->base64_sig, siglist);
 }
 
 /**
@@ -519,31 +520,29 @@ int SYMEXPORT alpm_pkg_check_pgp_signature(alpm_pkg_t *pkg,
  * @return a int value : 0 (valid), 1 (invalid), -1 (an error occurred)
  */
 int SYMEXPORT alpm_db_check_pgp_signature(alpm_db_t *db,
-		alpm_sigresult_t *result)
+		alpm_siglist_t *siglist)
 {
 	ASSERT(db != NULL, return -1);
-	ASSERT(result != NULL, RET_ERR(db->handle, ALPM_ERR_WRONG_ARGS, -1));
+	ASSERT(siglist != NULL, RET_ERR(db->handle, ALPM_ERR_WRONG_ARGS, -1));
 	db->handle->pm_errno = 0;
 
-	return _alpm_gpgme_checksig(db->handle, _alpm_db_path(db), NULL, result);
+	return _alpm_gpgme_checksig(db->handle, _alpm_db_path(db), NULL, siglist);
 }
 
-int SYMEXPORT alpm_sigresult_cleanup(alpm_sigresult_t *result)
+int SYMEXPORT alpm_siglist_cleanup(alpm_siglist_t *siglist)
 {
-	ASSERT(result != NULL, return -1);
-	/* Because it is likely result is on the stack, uid and status may have bogus
-	 * values in the struct. Only look at them if count is greater than 0. */
-	if(result->count > 0) {
-		free(result->status);
-		free(result->validity);
-		if(result->uid) {
-			int i;
-			for(i = 0; i < result->count; i++) {
-				free(result->uid[i]);
-			}
-			free(result->uid);
+	ASSERT(siglist != NULL, return -1);
+	size_t num;
+	for(num = 0; num < siglist->count; num++) {
+		alpm_sigresult_t *result = siglist->results + num;
+		if(result->key.data) {
+			gpgme_key_unref(result->key.data);
+		} else {
+			free(result->key.fingerprint);
 		}
 	}
+	FREE(siglist->results);
+	siglist->count = 0;
 	return 0;
 }
 
diff --git a/lib/libalpm/signing.h b/lib/libalpm/signing.h
index 3c98a482..8e47b2cd 100644
--- a/lib/libalpm/signing.h
+++ b/lib/libalpm/signing.h
@@ -23,7 +23,7 @@
 
 char *_alpm_sigpath(alpm_handle_t *handle, const char *path);
 int _alpm_gpgme_checksig(alpm_handle_t *handle, const char *path,
-		const char *base64_sig, alpm_sigresult_t *result);
+		const char *base64_sig, alpm_siglist_t *result);
 int _alpm_check_pgp_helper(alpm_handle_t *handle, const char *path,
 		const char *base64_sig, int optional, int marginal, int unknown);
 
diff --git a/src/pacman/package.c b/src/pacman/package.c
index 80c6bf2f..a8c15eb8 100644
--- a/src/pacman/package.c
+++ b/src/pacman/package.c
@@ -149,17 +149,17 @@ void dump_pkg_full(alpm_pkg_t *pkg, enum pkg_from from, int extra)
 				alpm_pkg_get_base64_sig(pkg) ? _("Yes") : _("None"));
 	}
 	if(from == PKG_FROM_FILE) {
-		alpm_sigresult_t result;
-		int err = alpm_pkg_check_pgp_signature(pkg, &result);
+		alpm_siglist_t siglist;
+		int err = alpm_pkg_check_pgp_signature(pkg, &siglist);
 		if(err && alpm_errno(config->handle) == ALPM_ERR_SIG_MISSING) {
 			string_display(_("Signatures     :"), _("None"));
 		} else if(err) {
 			string_display(_("Signatures     :"),
 					alpm_strerror(alpm_errno(config->handle)));
 		} else {
-			signature_display(_("Signatures     :"), &result);
+			signature_display(_("Signatures     :"), &siglist);
 		}
-		alpm_sigresult_cleanup(&result);
+		alpm_siglist_cleanup(&siglist);
 	}
 	string_display(_("Description    :"), alpm_pkg_get_desc(pkg));
 
diff --git a/src/pacman/util.c b/src/pacman/util.c
index cb62ec66..47fbaeb5 100644
--- a/src/pacman/util.c
+++ b/src/pacman/util.c
@@ -669,7 +669,7 @@ void list_display_linebreak(const char *title, const alpm_list_t *list)
 	}
 }
 
-void signature_display(const char *title, alpm_sigresult_t *result)
+void signature_display(const char *title, alpm_siglist_t *siglist)
 {
 	int len = 0;
 
@@ -677,13 +677,14 @@ void signature_display(const char *title, alpm_sigresult_t *result)
 		len = string_length(title) + 1;
 		printf("%s ", title);
 	}
-	if(result->count == 0) {
+	if(siglist->count == 0) {
 		printf(_("None"));
 	} else {
-		int i;
-		for(i = 0; i < result->count; i++) {
+		size_t i;
+		for(i = 0; i < siglist->count; i++) {
 			char sigline[PATH_MAX];
 			const char *status, *validity, *name;
+			alpm_sigresult_t *result = siglist->results + i;
 			/* Don't re-indent the first result */
 			if(i != 0) {
 				int j;
@@ -691,7 +692,7 @@ void signature_display(const char *title, alpm_sigresult_t *result)
 					printf(" ");
 				}
 			}
-			switch(result->status[i]) {
+			switch(result->status) {
 				case ALPM_SIGSTATUS_VALID:
 					status = _("Valid");
 					break;
@@ -711,7 +712,7 @@ void signature_display(const char *title, alpm_sigresult_t *result)
 					status = _("Signature error");
 					break;
 			}
-			switch(result->validity[i]) {
+			switch(result->validity) {
 				case ALPM_SIGVALIDITY_FULL:
 					validity = _("full trust");
 					break;
@@ -726,7 +727,7 @@ void signature_display(const char *title, alpm_sigresult_t *result)
 					validity = _("unknown trust");
 					break;
 			}
-			name = result->uid[i] ? result->uid[i] : _("{Key Unknown}");
+			name = result->key.uid ? result->key.uid : result->key.fingerprint;
 			snprintf(sigline, PATH_MAX, _("%s, %s from \"%s\""),
 					status, validity, name);
 			indentprint(sigline, len);
diff --git a/src/pacman/util.h b/src/pacman/util.h
index f9f1485f..66698474 100644
--- a/src/pacman/util.h
+++ b/src/pacman/util.h
@@ -57,7 +57,7 @@ double humanize_size(off_t bytes, const char target_unit, const char **label);
 int table_display(const char *title, const alpm_list_t *header, const alpm_list_t *rows);
 void list_display(const char *title, const alpm_list_t *list);
 void list_display_linebreak(const char *title, const alpm_list_t *list);
-void signature_display(const char *title, alpm_sigresult_t *result);
+void signature_display(const char *title, alpm_siglist_t *siglist);
 void display_targets(const alpm_list_t *pkgs, int install);
 int str_cmp(const void *s1, const void *s2);
 void display_new_optdepends(alpm_pkg_t *oldpkg, alpm_pkg_t *newpkg);