From cf70ac8af421dd9f1a53d9e5dbf89b976b02290b Mon Sep 17 00:00:00 2001 From: sinanmohd Date: Tue, 16 Apr 2024 21:11:07 +0530 Subject: libnpass: refactor https://www.kernel.org/doc/html/v4.10/process/coding-style.html - Function return values and names (-Exxx = failure, 0 = success) for imperative commands (0 = failure, non-zero = success) for predicates --- include/libnpass/libnpass.h | 7 +- src/libnpass/gpg.c | 8 +- src/libnpass/libnpass.c | 304 ++++++++++++++++++++++---------------------- src/npass/npass.c | 2 +- 4 files changed, 157 insertions(+), 164 deletions(-) diff --git a/include/libnpass/libnpass.h b/include/libnpass/libnpass.h index d82de9c..186b907 100644 --- a/include/libnpass/libnpass.h +++ b/include/libnpass/libnpass.h @@ -21,11 +21,8 @@ struct store { pass_store_t type; }; -DIR *openstore(const char *spath); -int readstore(DIR *dirp, struct store *s); int readstore_all(const char *path, struct store **stor); - -pass_store_t pass_store_type(const char *spath); +int pass_store_type(const char *spath); int pass_store_cmp(const void *vp1, const void *vp2); int pass_init(const char *fpr); @@ -34,4 +31,4 @@ int pass_add(const char *path, const char *pass, size_t n); int pass_rm(const char *path); ssize_t pass_getpass(char **lineptr, size_t *n, FILE *stream); -int pass_gen(pass_gen_t gen, char *pass, size_t n); +int pass_gen(pass_gen_t gen, char *pass, int len); diff --git a/src/libnpass/gpg.c b/src/libnpass/gpg.c index c7cf328..671397e 100644 --- a/src/libnpass/gpg.c +++ b/src/libnpass/gpg.c @@ -12,7 +12,7 @@ do { \ int __gpg_err_ret = gpgme_err_code_to_errno(err); \ if (err != 0) \ - __gpg_err_ret = 1; \ + __gpg_err_ret = EPERM; \ \ gpg_cleanup(); \ err_ret(-__gpg_err_ret, "%s: %s", gpgme_strsource(err), \ @@ -106,9 +106,6 @@ int gpg_decrypt(FILE *pass_out, const char *pass_path) out_gpg_cleanup: gpg_cleanup(); - /* - * refactor err_ret for -errno - */ if (r < 0) err_ret(-errno, "%s", strerror(errno)); else @@ -154,9 +151,6 @@ int gpg_encrypt(FILE *stream, const char *fpr, const char *pass, size_t n) out_gpg_cleanup: gpg_cleanup(); - /* - * refactor err_ret for -errno - */ if (r < 0) err_ret(-errno, "%s", strerror(errno)); else diff --git a/src/libnpass/libnpass.c b/src/libnpass/libnpass.c index 94a0647..f62f098 100644 --- a/src/libnpass/libnpass.c +++ b/src/libnpass/libnpass.c @@ -15,9 +15,9 @@ #include "libnpass/util.h" #include "util.h" -#define DEF_PASS_DIR "pass" -#define DEF_STOR_LEN 64 -#define FPR_MAX NAME_MAX +#define DEF_PASS_DIR "pass" +#define DEF_STOR_SIZE 64 +#define FPR_MAX NAME_MAX static char pass_dir[PATH_MAX] = {0}; const char *pass_gen_set[] = { @@ -33,14 +33,16 @@ const char *pass_gen_set[] = { "!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~", }; -static int set_pass_dir(void); +static int openstore(const char *spath, DIR **dirp); +static int readstore(DIR *dirp, struct store *s); static int is_storeobj(struct dirent *dir); +static int set_pass_dir(void); static int set_pass_dir(void) { - int r; - const char *env; struct stat statbuf; + const char *env; + int r; env = getenv("PASSWORD_STORE_DIR"); if (env) { @@ -56,7 +58,7 @@ static int set_pass_dir(void) r = snprintf(pass_dir, sizeof(pass_dir), "%s/%s", env, ".password-store"); if (r < 0 || (size_t)r > sizeof(pass_dir)) - err_ret(PASS_STORE_INV, "failed to build path"); + err_ret(-ENAMETOOLONG, "Path exceeded PATH_MAX"); r = stat(pass_dir, &statbuf); if (!r) @@ -68,7 +70,7 @@ static int set_pass_dir(void) r = snprintf(pass_dir, sizeof(pass_dir), "%s/%s", env, DEF_PASS_DIR); if (r < 0 || (size_t)r > sizeof(pass_dir)) - err_ret(PASS_STORE_INV, "failed to build path"); + err_ret(-ENAMETOOLONG, "Path exceeded PATH_MAX"); return 0; } @@ -77,17 +79,17 @@ static int set_pass_dir(void) r = snprintf(pass_dir, sizeof(pass_dir), "%s/%s/%s", env, ".local/share", DEF_PASS_DIR); if (r < 0 || (size_t)r > sizeof(pass_dir)) - err_ret(PASS_STORE_INV, "failed to build path"); + err_ret(-ENAMETOOLONG, "Path exceeded PATH_MAX"); return 0; } - return 1; + return -EINVAL; } static int is_storeobj(struct dirent *dir) { - int r; char *s; + int r; switch (dir->d_type) { case DT_DIR: @@ -120,84 +122,83 @@ int pass_store_cmp(const void *vp1, const void *vp2) return strcmp(sp1->name, sp2->name); } -pass_store_t pass_store_type(const char *spath) +int pass_store_type(const char *spath) { - int r; - struct stat sbuf; char abs_path[PATH_MAX]; + struct stat sbuf; + int r; r = set_pass_dir(); - if (r) - err_ret(PASS_STORE_INV, "PASSWORD_STORE_DIR not set"); + if (r < 0) + err_ret(-EINVAL, "PASSWORD_STORE_DIR must be set"); r = snprintf(abs_path, sizeof(abs_path), "%s/%s", pass_dir, (spath) ? spath : ""); if (r < 0 || (size_t)r >= sizeof(abs_path)) - err_ret(PASS_STORE_INV, "failed to build path"); + err_ret(-ENAMETOOLONG, "Path exceeded PATH_MAX"); r = stat(abs_path, &sbuf); if (!r && (sbuf.st_mode & S_IFMT) == S_IFDIR) return PASS_STORE_DIR; r = snprintf(abs_path, sizeof(abs_path), "%s/%s.gpg", pass_dir, spath); if (r < 0 || (size_t)r >= sizeof(abs_path)) - err_ret(PASS_STORE_INV, "failed to build path"); + err_ret(-ENAMETOOLONG, "Path exceeded PATH_MAX"); r = stat(abs_path, &sbuf); - if (r) - err_ret(PASS_STORE_INV, "%s", strerror(errno)); + if (r < 0) + err_ret(-errno, "%s", strerror(errno)); if ((sbuf.st_mode & S_IFMT) == S_IFREG) { return PASS_STORE_ENC; } else { - err_ret(PASS_STORE_INV, "%s is not a regular file", abs_path); + err_ret(-EINVAL, "%s Not a regular file", abs_path); } } -DIR *openstore(const char *spath) +static int openstore(const char *spath, DIR **dirp) { - int r; - DIR *d; - const char *path; - char abs_path[PATH_MAX]; pass_store_t store_type; + char abs_path[PATH_MAX]; + const char *path; + int r; store_type = pass_store_type(spath); if (store_type != PASS_STORE_DIR) - err_ret(NULL, "%s is not a passwordstore directory", spath); + err_ret(-EINVAL, "%s Not a passwordstore path", spath); if (spath) { r = snprintf(abs_path, sizeof(abs_path), "%s/%s", pass_dir, spath); if (r < 0 || (size_t)r >= sizeof(abs_path)) - err_ret(NULL, "failed to build path"); + err_ret(-ENAMETOOLONG, "Path exceeded PATH_MAX"); path = abs_path; } else { path = pass_dir; } - d = opendir(path); - if (!d) - err_ret(NULL, "%s", strerror(errno)); + *dirp = opendir(path); + if (!*dirp) + err_ret(-errno, "%s", strerror(errno)); - return d; + return 0; } -int readstore(DIR *dirp, struct store *s) +static int readstore(DIR *dirp, struct store *s) { struct dirent *dir; - errno = 0; - - while ((dir = readdir(dirp))) { - if (is_storeobj(dir)) + do { + errno = 0; + dir = readdir(dirp); + if (dir && is_storeobj(dir)) break; - } + } while (dir); - if (!dir) { - if (errno != 0) - err_ret(1, "%s", strerror(errno)); + if (dir == NULL) { + if (errno) + err_ret(-errno, "%s", strerror(errno)); - return EOF; + return 1; } strcpy(s->name, dir->d_name); @@ -221,139 +222,140 @@ int readstore(DIR *dirp, struct store *s) int readstore_all(const char *path, struct store **stor) { - int r; - void *p; - size_t i; + int ret, size, len; DIR *dirp; - size_t len = DEF_STOR_LEN; + void *p; - *stor = malloc(sizeof(struct store) * len); + size = DEF_STOR_SIZE; + *stor = malloc(sizeof(struct store) * size); if (!*stor) - err_ret(-1, "%s", strerror(errno)); + err_ret(-errno, "%s", strerror(errno)); - dirp = openstore(path); - if (!dirp) { + ret = openstore(path, &dirp); + if (ret < 0) { free(*stor); - return -1; + return ret; }; - for (i = 0; !(r = readstore(dirp, *stor + i)); i++) { - if (i < len - 1) + for (len = 0; !(ret = readstore(dirp, *stor + len)); len++) { + if (len < size - 1) continue; - len *= 2; - p = realloc(*stor, sizeof(**stor) * len); - if (!p) { - free(*stor); - err_ret(-1, "%s", strerror(errno)); - } else { + size *= 2; + p = realloc(*stor, sizeof(**stor) * size); + if (p) { *stor = p; + } else { + free(*stor); + err_ret(-errno, "%s", strerror(errno)); } } - return i; + return (ret < 0) ? ret : len; } int pass_init(const char *fpr) { - int r; char gpg_id_path[PATH_MAX]; FILE *gpg_id; + int r, fpr_len; r = set_pass_dir(); - if (r) - err_ret(1, "PASSWORD_STORE_DIR not set"); + if (r < 0) + err_ret(r, "PASSWORD_STORE_DIR must be set"); r = gpg_key_validate(fpr); if (r < 0) - err_ret(1, "key not usable, try gpg --full-generate-key"); + err_ret(r, "Try gpg --full-generate-key"); r = r_mkdir(pass_dir, S_IRWXU); if (r < 0) - err_ret(1, "%s %s", pass_dir, strerror(errno)); + err_ret(r, "%s %s", pass_dir, strerror(errno)); r = snprintf(gpg_id_path, sizeof(gpg_id_path), "%s/%s", pass_dir, ".gpg-id"); if (r < 0 || (size_t)r >= sizeof(gpg_id_path)) - err_ret(1, "failed to build path"); + err_ret(-ENAMETOOLONG, "Path exceeded PATH_MAX"); gpg_id = fopen(gpg_id_path, "w"); if (!gpg_id) - err_ret(1, "%s %s", gpg_id_path, strerror(errno)); + err_ret(-errno, "%s %s", gpg_id_path, strerror(errno)); - r = fwrite(fpr, strlen(fpr), 1, gpg_id); + fpr_len = strlen(fpr); + r = fwrite(fpr, sizeof(*fpr), fpr_len, gpg_id); fclose(gpg_id); - if (!r) - err_ret(1, "write failed"); + if (r != fpr_len) + err_ret(-EPERM, "%s %s", gpg_id_path, "Write failed"); return 0; } int pass_cat(FILE *out, const char *path) { - int r; - pass_store_t store_type; char pass_path[PATH_MAX]; + int ret; - store_type = pass_store_type(path); - if (store_type != PASS_STORE_ENC) - err_ret(1, "%s is not a passwordstore file", path); + ret = pass_store_type(path); + if (ret < 0) + return ret; + if (ret != PASS_STORE_ENC) + err_ret(-EINVAL, "%s is Not a password", path); - r = set_pass_dir(); - if (r) - err_ret(1, "PASSWORD_STORE_DIR not set"); - - r = snprintf(pass_path, sizeof(pass_path), "%s/%s.gpg", pass_dir, path); - if (r < 0 || (size_t)r >= sizeof(pass_path)) - err_ret(1, "failed to build path"); + ret = snprintf(pass_path, sizeof(pass_path), "%s/%s.gpg", pass_dir, + path); + if (ret < 0 || (size_t)ret >= sizeof(pass_path)) + err_ret(-ENAMETOOLONG, "Path exceeded PATH_MAX"); - r = gpg_decrypt(out, pass_path); - return (r < 0) ? 1 : r; + return gpg_decrypt(out, pass_path); } ssize_t pass_getpass(char **lineptr, size_t *n, FILE *stream) { - ssize_t r; struct termios new, old; + int ret; - r = tcgetattr(fileno(stream), &old); - if (r) - return -1; + ret = tcgetattr(fileno(stream), &old); + if (ret < 0) + return ret; new = old; new.c_lflag &= ~ECHO; - r = tcsetattr(fileno(stream), TCSAFLUSH, &new); - if (r) - return -1; + ret = tcsetattr(fileno(stream), TCSAFLUSH, &new); + if (ret < 0) + err_ret(-errno, "%s", strerror(errno)); - r = getline(lineptr, n, stream); - if (r > 0 && (*lineptr)[r - 1] == '\n') - (*lineptr)[r - 1] = '\0'; + ret = getline(lineptr, n, stream); + if (ret > 0 && (*lineptr)[ret - 1] == '\n') + (*lineptr)[ret - 1] = '\0'; + else if (ret < 0) + err_ret(-errno, "%s", strerror(errno)); - (void)tcsetattr(fileno(stream), TCSAFLUSH, &old); + ret = tcsetattr(fileno(stream), TCSAFLUSH, &old); + if (ret < 0) + err_ret(-errno, "%s", strerror(errno)); - return r; + return 0; } -int pass_gen(pass_gen_t gen, char *pass, size_t n) +int pass_gen(pass_gen_t gen, char *pass, int len) { - int r; - FILE *rand; size_t pass_gen_len; + FILE *rand; + int ret; - if (n <= 0) - err_ret(1, "small password"); + if (len <= 0) + err_ret(-EINVAL, "Small password"); rand = fopen("/dev/urandom", "r"); if (!rand) - err_ret(1, "%s", strerror(errno)); + err_ret(-errno, "%s", strerror(errno)); - r = fread(pass, sizeof(pass[0]), n, rand); - if (r != (int)n) - err_ret(1, "fread failed"); + ret = fread(pass, sizeof(pass[0]), len, rand); + if (ret != len) + err_ret(-EPERM, "Failed to read /dev/urandom"); pass_gen_len = strlen(pass_gen_set[gen]); - for (size_t i = 0; i < n; i++) + for (int i = 0; i < len; i++) pass[i] = pass_gen_set[gen][pass[i] % (pass_gen_len - 1)]; return 0; @@ -361,80 +363,80 @@ int pass_gen(pass_gen_t gen, char *pass, size_t n) int pass_add(const char *path, const char *pass, size_t n) { - int r; - char *rc; - FILE *gpg_id, *out_stream; char gpg_id_path[PATH_MAX], fpr[FPR_MAX], pass_path[PATH_MAX]; + FILE *gpg_id, *out_stream; + char *p; + int ret; - r = set_pass_dir(); - if (r) - err_ret(1, "PASSWORD_STORE_DIR not set"); + ret = set_pass_dir(); + if (ret < 0) + err_ret(ret, "PASSWORD_STORE_DIR must be set"); - r = snprintf(gpg_id_path, sizeof(gpg_id_path), "%s/%s", pass_dir, - ".gpg-id"); - if (r < 0 || (size_t)r >= sizeof(gpg_id_path)) - err_ret(1, "failed to build path"); + ret = snprintf(gpg_id_path, sizeof(gpg_id_path), "%s/%s", pass_dir, + ".gpg-id"); + if (ret < 0 || (size_t)ret >= sizeof(gpg_id_path)) + err_ret(-EINVAL, "Path exceeded PATH_MAX"); gpg_id = fopen(gpg_id_path, "r"); if (!gpg_id) - err_ret(1, "%s %s", gpg_id_path, strerror(errno)); + err_ret(-errno, "%s %s", gpg_id_path, strerror(errno)); - rc = fgets(fpr, sizeof(fpr), gpg_id); - if (!rc) - err_ret(1, "failed to read %s", gpg_id_path); + p = fgets(fpr, sizeof(fpr), gpg_id); + if (!p) + err_ret(-EPERM, "Failed to read %s", gpg_id_path); fclose(gpg_id); util_strtrim(fpr); - r = gpg_key_validate(fpr); - if (r < 0) - err_ret(1, "invalid key , try gpg --list-keys"); + ret = gpg_key_validate(fpr); + if (ret < 0) + err_ret(ret, "Invalid key , try gpg --list-keys"); /* TODO: guard against .*\.gpg\.gpg[/$] */ - r = snprintf(pass_path, sizeof(pass_path), "%s/%s.gpg", pass_dir, path); - if (r < 0 || (size_t)r >= sizeof(pass_path)) - err_ret(1, "failed to build path"); + ret = snprintf(pass_path, sizeof(pass_path), "%s/%s.gpg", pass_dir, + path); + if (ret < 0 || (size_t)ret >= sizeof(pass_path)) + err_ret(-errno, "Path exceeded PATH_MAX"); - rc = strdup(pass_path); - if (!rc) - err_ret(1, "%s", strerror(errno)); - (void)r_mkdir(dirname(rc), S_IRWXU); - free(rc); + ret = r_mkdir(dirname(p), S_IRWXU); + if (ret < 0) + return ret; - r = access(pass_path, F_OK); - if (!(errno & ENOENT)) - err_ret(1, "an entry already exists for %s", path); + errno = 0; + ret = access(pass_path, F_OK); + if (errno != ENOENT) + err_ret(-EEXIST, "%s %s", path, strerror(EEXIST)); out_stream = fopen(pass_path, "w"); if (!out_stream) - err_ret(1, "%s", strerror(errno)); - - r = gpg_encrypt(out_stream, fpr, pass, n); + err_ret(-errno, "%s", strerror(errno)); + ret = gpg_encrypt(out_stream, fpr, pass, n); fclose(out_stream); - return (r < 0) ? 1 : r; + + return ret; } int pass_rm(const char *path) { - int r = 0; char gpg_path[PATH_MAX], abs_path[PATH_MAX]; + int ret = 0; - r = set_pass_dir(); - if (r) - err_ret(1, "PASSWORD_STORE_DIR not set"); + ret = set_pass_dir(); + if (ret < 0) + err_ret(ret, "PASSWORD_STORE_DIR must be set"); - r = snprintf(gpg_path, sizeof(gpg_path), "%s.gpg", path); - if (r < 0 || (size_t)r >= sizeof(gpg_path)) - err_ret(1, "failed to build path"); + ret = snprintf(gpg_path, sizeof(gpg_path), "%s.gpg", path); + if (ret < 0 || (size_t)ret >= sizeof(gpg_path)) + err_ret(-EINVAL, "Path exceeded PATH_MAX"); - r = snprintf(abs_path, sizeof(gpg_path), "%s/%s", pass_dir, gpg_path); - if (r < 0 || (size_t)r >= sizeof(abs_path)) - err_ret(1, "failed to build path"); + ret = snprintf(abs_path, sizeof(gpg_path), "%s/%s", pass_dir, gpg_path); + if (ret < 0 || (size_t)ret >= sizeof(abs_path)) + err_ret(-EINVAL, "Path exceeded PATH_MAX"); /* TODO: guard against .*\.gpg\.gpg[/$] */ - r = unlink(abs_path); - if (r) - err_ret(1, "%s %s", abs_path, strerror(errno)); + ret = unlink(abs_path); + if (ret < 0) + err_ret(-errno, "%s %s", abs_path, strerror(errno)); return r_rmdir_empty(pass_dir, dirname(gpg_path)); } diff --git a/src/npass/npass.c b/src/npass/npass.c index 025566e..f02c4be 100644 --- a/src/npass/npass.c +++ b/src/npass/npass.c @@ -67,7 +67,7 @@ static int ls(const char *path, size_t depth) static size_t depth_state_len = DEF_DEPTTH; len = readstore_all(path, &stor); - if (len < 0) + if (len <= 0) return 1; if (!depth) { -- cgit v1.2.3