From 689c7e2204e3e924cf24ddfb9c399215b786ec1b Mon Sep 17 00:00:00 2001 From: sinanmohd Date: Mon, 15 Apr 2024 18:10:57 +0530 Subject: 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 --- src/libnpass/gpg.c | 95 +++++++++++++++++++++++++++++++++---------------- src/libnpass/libnpass.c | 8 ++--- 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 #include #include +#include #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) -- cgit v1.2.3