libnpass/gpg: refactor

https://www.kernel.org/doc/html/v4.10/process/coding-style.html

 - Centralized exiting of functions (goto cleanup)
 - Function return values and names
   (-Exxx = failure, 0 = success) for imperative commands
   (0 = failure, non-zero = success) for predicates
 - avoid macros that affect control flow

2 files changed, 68 insertions, 35 deletions

diff --git a/src/libnpass/gpg.c b/src/libnpass/gpg.c
index 0b47f42..c7cf328 100644
--- a/src/libnpass/gpg.c
+++ b/src/libnpass/gpg.c
@@ -2,6 +2,7 @@
 #include <gpgme.h>
 #include <locale.h>
 #include <stdio.h>
+#include <string.h>
 
 #include "libnpass/gpg.h"
 #include "libnpass/util.h"
@@ -9,11 +10,13 @@
 
 #define gpg_err_ret(err)                                                       \
 	do {                                                                   \
-		if (err) {                                                     \
-			gpg_cleanup();                                         \
-			err_ret(1, "%s: %s", gpgme_strsource(err),             \
-				gpgme_strerror(err));                          \
-		}                                                              \
+		int __gpg_err_ret = gpgme_err_code_to_errno(err);              \
+		if (err != 0)                                                  \
+			__gpg_err_ret = 1;                                     \
+                                                                               \
+		gpg_cleanup();                                                 \
+		err_ret(-__gpg_err_ret, "%s: %s", gpgme_strsource(err),        \
+			gpgme_strerror(err));                                  \
 	} while (0)
 
 static gpgme_ctx_t ctx = NULL;
@@ -34,9 +37,12 @@ static int gpg_init(void)
 #endif
 
 	err = gpgme_engine_check_version(GPGME_PROTOCOL_OpenPGP);
-	gpg_err_ret(err);
+	if (err != GPG_ERR_NO_ERROR)
+		gpg_err_ret(err);
+
 	err = gpgme_new(&ctx);
-	gpg_err_ret(err);
+	if (err != GPG_ERR_NO_ERROR)
+		gpg_err_ret(err);
 
 	return 0;
 }
@@ -51,49 +57,62 @@ static void gpg_cleanup(void)
 
 int gpg_key_validate(const char *fpr)
 {
-	int r;
 	gpgme_error_t err;
+	int r;
 
 	r = gpg_init();
-	if (r)
+	if (r < 0)
 		return r;
 
 	err = gpgme_get_key(ctx, fpr, key, 1);
-	gpg_err_ret(err);
+	if (err != GPG_ERR_NO_ERROR)
+		gpg_err_ret(err);
 
-	gpg_cleanup();
 	return 0;
 }
 
 int gpg_decrypt(FILE *pass_out, const char *pass_path)
 {
-	int r;
-	char buf[BUFSIZ];
 	gpgme_data_t in, out;
 	gpgme_error_t err;
+	char buf[BUFSIZ];
+	int r;
 
 	r = gpg_init();
-	if (r)
+	if (r < 0)
 		return r;
 
 	err = gpgme_data_new_from_file(&in, pass_path, 1);
-	gpg_err_ret(err);
+	if (err != GPG_ERR_NO_ERROR)
+		gpg_err_ret(err);
+
 	err = gpgme_data_new(&out);
-	gpg_err_ret(err);
+	if (err != GPG_ERR_NO_ERROR)
+		gpg_err_ret(err);
+
 	err = gpgme_op_decrypt(ctx, in, out);
-	gpg_err_ret(err);
+	if (err != GPG_ERR_NO_ERROR)
+		gpg_err_ret(err);
 
 	r = gpgme_data_seek(out, 0, SEEK_SET);
-	if (r)
-		gpg_err_ret(gpgme_err_code_from_errno(errno));
+	if (r < 0)
+		goto out_gpg_cleanup;
 
 	while ((r = gpgme_data_read(out, buf, sizeof(buf))))
 		fwrite(buf, r, 1, pass_out);
 	if (r < 0)
-		gpg_err_ret(gpgme_err_code_from_errno(errno));
+		goto out_gpg_cleanup;
 
+out_gpg_cleanup:
 	gpg_cleanup();
-	return 0;
+
+	/*
+	 * refactor err_ret for -errno
+	 */
+	if (r < 0)
+		err_ret(-errno, "%s", strerror(errno));
+	else
+		return 0;
 }
 
 int gpg_encrypt(FILE *stream, const char *fpr, const char *pass, size_t n)
@@ -104,28 +123,42 @@ int gpg_encrypt(FILE *stream, const char *fpr, const char *pass, size_t n)
 	gpgme_error_t err;
 
 	r = gpg_init();
-	if (r)
+	if (r < 0)
 		return r;
 
 	err = gpgme_get_key(ctx, fpr, key, 1);
-	gpg_err_ret(err);
+	if (err != GPG_ERR_NO_ERROR)
+		gpg_err_ret(err);
 
 	err = gpgme_data_new_from_mem(&in, pass, n, 0);
-	gpg_err_ret(err);
+	if (err != GPG_ERR_NO_ERROR)
+		gpg_err_ret(err);
+
 	err = gpgme_data_new(&out);
-	gpg_err_ret(err);
+	if (err != GPG_ERR_NO_ERROR)
+		gpg_err_ret(err);
+
 	err = gpgme_op_encrypt(ctx, key, GPGME_ENCRYPT_ALWAYS_TRUST, in, out);
-	gpg_err_ret(err);
+	if (err != GPG_ERR_NO_ERROR)
+		gpg_err_ret(err);
 
 	r = gpgme_data_seek(out, 0, SEEK_SET);
-	if (r)
-		gpg_err_ret(gpgme_err_code_from_errno(errno));
+	if (r < 0)
+		goto out_gpg_cleanup;
 
 	while ((r = gpgme_data_read(out, buf, sizeof(buf))))
 		fwrite(buf, r, 1, stream);
-	gpg_cleanup();
 	if (r < 0)
-		gpg_err_ret(gpgme_err_code_from_errno(errno));
+		goto out_gpg_cleanup;
 
-	return 0;
+out_gpg_cleanup:
+	gpg_cleanup();
+
+	/*
+	 * refactor err_ret for -errno
+	 */
+	if (r < 0)
+		err_ret(-errno, "%s", strerror(errno));
+	else
+		return 0;
 }
diff --git a/src/libnpass/libnpass.c b/src/libnpass/libnpass.c
index 6618385..6c66a79 100644
--- a/src/libnpass/libnpass.c
+++ b/src/libnpass/libnpass.c
@@ -265,7 +265,7 @@ int pass_init(const char *fpr)
 		err_ret(1, "PASSWORD_STORE_DIR not set");
 
 	r = gpg_key_validate(fpr);
-	if (r)
+	if (r < 0)
 		err_ret(1, "key not usable, try gpg --full-generate-key");
 
 	r = r_mkdir(pass_dir, S_IRWXU);
@@ -308,7 +308,7 @@ int pass_cat(FILE *out, const char *path)
 		err_ret(1, "failed to build path");
 
 	r = gpg_decrypt(out, pass_path);
-	return r;
+	return (r < 0) ? 1 : r;
 }
 
 ssize_t pass_getpass(char **lineptr, size_t *n, FILE *stream)
@@ -386,7 +386,7 @@ int pass_add(const char *path, const char *pass, size_t n)
 	util_strtrim(fpr);
 
 	r = gpg_key_validate(fpr);
-	if (r)
+	if (r < 0)
 		err_ret(1, "invalid key , try gpg --list-keys");
 
 	/* TODO: guard against .*\.gpg\.gpg[/$] */
@@ -411,7 +411,7 @@ int pass_add(const char *path, const char *pass, size_t n)
 	r = gpg_encrypt(out_stream, fpr, pass, n);
 
 	fclose(out_stream);
-	return r;
+	return (r < 0) ? 1 : r;
 }
 
 int pass_rm(const char *path)