From cf70ac8af421dd9f1a53d9e5dbf89b976b02290b Mon Sep 17 00:00:00 2001
From: sinanmohd <sinan@sinanmohd.com>
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