clean up error handling

4 files changed, 84 insertions, 37 deletions

diff --git a/nslcd/hackers.h b/nslcd/hackers.h
index 24b4488..22643af 100644
--- a/nslcd/hackers.h
+++ b/nslcd/hackers.h
@@ -4,6 +4,9 @@
 #include <pthread.h>
 #include <pwd.h>
 
+#define GID_INVALID ((gid_t)-1)
+#define UID_INVALID ((uid_t)-1)
+
 struct session {
 	pthread_rwlock_t lock;
 	size_t cnt;
diff --git a/nslcd/hackers_parse.c b/nslcd/hackers_parse.c
index c26feae..7eb0e4f 100644
--- a/nslcd/hackers_parse.c
+++ b/nslcd/hackers_parse.c
@@ -31,8 +31,7 @@
 #include <unistd.h> /* for sysconf(3) */
 
 #include "hackers_parse.h"
-
-/* None of the malloc-ish calls are error checked.  Bite me. */
+#include "hackers.h"
 
 #define DEFAULT_PASSWORD "!"
 
@@ -40,7 +39,8 @@
 	do {                                                         \
 		errno = 0;                                           \
 		if (!(expr)) {                                       \
-			error(0, errno, "ASSERT(%s) failed", #expr); \
+			error(0, errno, "%s:%d: ASSERT(%s) failed",  \
+			      __FILE__, __LINE__, #expr);            \
 			goto error;                                  \
 		}                                                    \
 	} while(0)
@@ -76,10 +76,11 @@
 #define	PW_SHELL 	 (1 << 6)
 #define PW_ALL		((1 << 7) - 1)
 
+/* Returns GID_INVALID on error */
 static 
 gid_t
 name2gid(const char *name) {
-	gid_t gid = 0;
+	gid_t gid = GID_INVALID;
 	char *buf = NULL;
 	ssize_t buflen = sysconf(_SC_GETGR_R_SIZE_MAX);
 	if (buflen < 1)
@@ -107,16 +108,17 @@ bool
 is_numeric(const char *str) {
 	size_t i;
 	for (i = 0; str[i] != '\0'; i++) {
-		if ('0' > str[i]  || str[i] > '9')
+		if ('0' > str[i] || str[i] > '9')
 			return false;
 	}
 	return (i > 0);
 }
 
+/* Returns UID_INVALID on error */
 uid_t
 filename2uid(const char *filename) {
 	if (filename == NULL)
-		return 0;
+		return UID_INVALID;
 
 	char *tmp = strdupa(filename);
 	char *bname = basename(tmp);
@@ -124,7 +126,7 @@ filename2uid(const char *filename) {
 	if ((strcmp(ext, ".yml") != 0) || (ext != &bname[4]))
 		return 0;
 	ext[0] = '\0';
-	uid_t uid = is_numeric(bname) ? atoi(bname) : 0;
+	uid_t uid = is_numeric(bname) ? (uid_t)atoi(bname) : UID_INVALID;
 	return uid;
 }
 
@@ -136,19 +138,22 @@ load_user_password(struct passwd *user) {
 	ssize_t line_len;
 	size_t line_cap = 0;
 
+	errno = 0;
 	if (asprintf(&filename, "%s/.password", user->pw_dir) < 0)
-		goto nopassword;
+		goto error;
 	if ((file = fopen(filename, "r")) == NULL)
 		goto nopassword;
 	// TODO: check permissions on 'file'
 	if ((line_len = getline(&line, &line_cap, file)) < 1)
-		goto nopassword;
+		goto error;
 	if (line[line_len-1] == '\n')
 		line[--line_len] = '\0';
 	free(filename);
 	fclose(file);
 	user->pw_passwd = line;
 	return 0;
+ error:
+	error(0, errno, "unexpected error in %s", __func__);
  nopassword:
 	free(filename);
 	free(line);
@@ -159,6 +164,7 @@ load_user_password(struct passwd *user) {
 #define NODE(id) yaml_document_get_node(&yaml_document, id)
 int
 load_user_yaml(const char *filename, struct passwd *user) {
+	errno = 0;
 	int ret = -1;
 	unsigned char flags = 0;
 
@@ -168,8 +174,7 @@ load_user_yaml(const char *filename, struct passwd *user) {
 	yaml_parser_t	yaml_parser;	ZERO(yaml_parser);
 	yaml_document_t	yaml_document;	ZERO(yaml_document);
 
-	uid_t uid;
-	ASSERT((uid = user->pw_uid = filename2uid(filename)) > 0);
+	ASSERT((user->pw_uid = filename2uid(filename)) != UID_INVALID);
 	flags |= PW_UID;
 
 	ASSERT((yaml_file = fopen(filename, "r")) != NULL);
@@ -191,11 +196,11 @@ load_user_yaml(const char *filename, struct passwd *user) {
 			flags |= PW_NAME | PW_DIR;
 		}
 		if (strcmp("fullname", STR_VALUE(key))==0) {
-			user->pw_gecos = strdup(STR_VALUE(val));
+			ASSERT((user->pw_gecos = strdup(STR_VALUE(val))) != NULL);
 			flags |= PW_GECOS;
 		}
 		if (strcmp("shell", STR_VALUE(key))==0) {
-			user->pw_shell = strdup(STR_VALUE(val));
+			ASSERT((user->pw_shell = strdup(STR_VALUE(val))) != NULL);
 			flags |= PW_SHELL;
 		}
 		if (strcmp("groups", STR_VALUE(key))==0) {
@@ -208,7 +213,7 @@ load_user_yaml(const char *filename, struct passwd *user) {
 				if (itemp == val->data.sequence.items.start) {
 					/* primary group */
 					char *grp_name = STR_VALUE(item);
-					ASSERT((user->pw_gid = name2gid(grp_name)) > 0);
+					ASSERT((user->pw_gid = name2gid(grp_name)) != GID_INVALID);
 					flags |= PW_GID;
 				} else {
 					/* secondary group */
@@ -226,8 +231,9 @@ load_user_yaml(const char *filename, struct passwd *user) {
 	ret = 0;
 	goto end;
  error:
+	error(0, errno, "error when parsing %s", filename);
 	PASSWD_FREE(*user);
-	user->pw_uid = uid;
+	user->pw_uid = UID_INVALID;
  end:
 	yaml_document_delete(&yaml_document);
 	yaml_parser_delete(&yaml_parser);
diff --git a/nslcd/hackers_watch.c b/nslcd/hackers_watch.c
index 1516593..7289c59 100644
--- a/nslcd/hackers_watch.c
+++ b/nslcd/hackers_watch.c
@@ -18,6 +18,8 @@
 #define _GNU_SOURCE
 
 #include <glob.h>
+#include <errno.h>
+#include <error.h>
 #include <stdio.h> /* for asprintf(3) */
 #include <unistd.h> /* for chdir(3) */
 
@@ -32,11 +34,39 @@
 #define EVENT_CHILD_MOD (IN_CLOSE_WRITE | IN_MOVED_TO)
 #define EVENT_CHILD_ANY (EVENT_CHILD_ADD | EVENT_CHILD_DEL | EVENT_CHILD_MOD)
 
-#define WATCH_HOMEDIR(session, i) \
-	session->in_user_wds[i] = \
-		inotify_add_watch(session->in_fd, \
-		                  session->users[i].pw_dir, \
-		                  EVENT_CHILD_ANY | IN_MOVE_SELF)
+#define ASSERT(expr)                                                 \
+	do {                                                         \
+		errno = 0;                                           \
+		if (!(expr)) {                                       \
+			error(0, errno, "%s:%d: ASSERT(%s) failed",  \
+			      __FILE__, __LINE__, #expr);            \
+			goto error;                                  \
+		}                                                    \
+	} while(0)
+
+#define REALLOC(ptr, size)                                                 \
+	(__extension__ ({                                                  \
+		errno = 0;                                                 \
+		void *ret = realloc(ptr, size);                            \
+		if (ret == (ptr)) {                                        \
+			error(0, errno, "could not (re)allocate memory");  \
+			goto error;                                        \
+		};                                                         \
+		ret;                                                       \
+	}))
+
+#define WATCH_HOMEDIR(session, i)                                           \
+	do {                                                                \
+		session->in_user_wds[i] =                                   \
+			inotify_add_watch(session->in_fd,                   \
+			                  session->users[i].pw_dir,         \
+			                  EVENT_CHILD_ANY | IN_MOVE_SELF);  \
+		if (session->in_user_wds[i] < 0) {                          \
+			error(0, errno, "could not watch: %s",              \
+			      session->users[i].pw_dir);                    \
+			goto error;                                         \
+		}                                                           \
+	} while(0)
 
 int
 hackers_init(const char *yamldir, struct session *sess) {
@@ -44,27 +74,26 @@ hackers_init(const char *yamldir, struct session *sess) {
 	glob_t glob_results;
 	char *filename;
 
-	sess->yamldir = strdup(yamldir);
-	pthread_rwlock_init(&(sess->lock), NULL);
-	sess->in_fd = inotify_init();
-	sess->in_wd_yaml = inotify_add_watch(sess->in_fd, yamldir, EVENT_CHILD_ANY);
-	sess->in_wd_home = inotify_add_watch(sess->in_fd, "/home" , EVENT_CHILD_ADD);
+	ASSERT((sess->yamldir = strdup(yamldir)) != NULL);
+	ASSERT((errno = pthread_rwlock_init(&(sess->lock), NULL)) >= 0);
+	ASSERT((sess->in_fd = inotify_init()) >= 0);
+	ASSERT((sess->in_wd_yaml = inotify_add_watch(sess->in_fd, yamldir, EVENT_CHILD_ANY)) >= 0);
+	ASSERT((sess->in_wd_home = inotify_add_watch(sess->in_fd, "/home" , EVENT_CHILD_ADD)) >= 0);
 
-	if (asprintf(&glob_pattern, "%s/*.yml", yamldir) < 0)
-		return -1;
-	glob(glob_pattern, 0, NULL, &glob_results);
+	ASSERT(asprintf(&glob_pattern, "%s/*.yml", yamldir) > 0);
+	ASSERT(glob(glob_pattern, 0, NULL, &glob_results) == 0);
 	free(glob_pattern);
 
 	sess->cnt = glob_results.gl_pathc - glob_results.gl_offs;
-	sess->users       = calloc(sess->cnt, sizeof(struct passwd));
-	sess->in_user_wds = calloc(sess->cnt, sizeof(int));
+	ASSERT((sess->users       = calloc(sess->cnt, sizeof(struct passwd))) != NULL);
+	ASSERT((sess->in_user_wds = calloc(sess->cnt, sizeof(int))) != NULL);
 
 	for (size_t i = 0; i < sess->cnt; i++) {
 		filename = glob_results.gl_pathv[glob_results.gl_offs+i];
 		if (load_user_yaml(filename, &(sess->users[i]))==0) {
 			WATCH_HOMEDIR(sess, i);
 		} else {
-			sess->users[i].pw_uid = 0;
+			sess->users[i].pw_uid = UID_INVALID;
 			sess->in_user_wds[i] = -1;
 		}
 	}
@@ -72,6 +101,8 @@ hackers_init(const char *yamldir, struct session *sess) {
 	globfree(&glob_results);
 
 	return 0;
+ error:
+	return -1;
 }
 
 static
@@ -79,7 +110,7 @@ void
 worker_watch_homedirs(struct session *sess) {
 	pthread_rwlock_wrlock(&(sess->lock));
 	for (size_t i = 0; i < sess->cnt; i++) {
-		if (sess->users[i].pw_uid > 0) {
+		if (sess->users[i].pw_uid != UID_INVALID) {
 			int oldwd = sess->in_user_wds[i];
 			WATCH_HOMEDIR(sess, i);
 			if (oldwd != sess->in_user_wds[i]) {
@@ -90,6 +121,9 @@ worker_watch_homedirs(struct session *sess) {
 		}
 	}
 	pthread_rwlock_unlock(&(sess->lock));
+	return;
+ error:
+	exit(1);
 }
 
 static
@@ -102,7 +136,7 @@ worker_handle_add_yaml(struct session *sess, struct passwd *newdata) {
 	pthread_rwlock_wrlock(&(sess->lock));
 	ssize_t spot = -1;
 	for (size_t i = 0; i < sess->cnt; i++) {
-		if (spot < 0 && sess->users[i].pw_uid == 0) {
+		if (spot < 0 && sess->users[i].pw_uid == UID_INVALID) {
 			spot = i;
 		}
 		if (sess->users[i].pw_uid == newdata->pw_uid) {
@@ -113,14 +147,17 @@ worker_handle_add_yaml(struct session *sess, struct passwd *newdata) {
 	if (spot < 0) {
 		/* must grow the array */
 		spot = sess->cnt++;
-		sess->users       = realloc(sess->users      , sess->cnt * sizeof(struct passwd));
-		sess->in_user_wds = realloc(sess->in_user_wds, sess->cnt * sizeof(int));
+		sess->users       = REALLOC(sess->users      , sess->cnt * sizeof(struct passwd));
+		sess->in_user_wds = REALLOC(sess->in_user_wds, sess->cnt * sizeof(int));
 		ZERO(sess->users[spot]);
-	} else if (sess->users[spot].pw_uid != 0) {
+	} else if (sess->users[spot].pw_uid != UID_INVALID) {
 		PASSWD_FREE(sess->users[spot]);
 	}
 	sess->users[spot] = *newdata;
 	pthread_rwlock_unlock(&(sess->lock));
+	return;
+ error:
+	exit(1);
 }
 
 static
@@ -149,7 +186,7 @@ hackers_worker(struct session *sess) {
 				if (load_user_yaml(event->name, &user)==0) {
 					/* User added/updated */
 					worker_handle_add_yaml(sess, &user);
-				} else if (user.pw_uid > 0) {
+				} else if (user.pw_uid != UID_INVALID) {
 					/* User became invalid */
 					worker_handle_del_yaml(sess,
 					                       user.pw_uid);
diff --git a/nslcd/nslcd.c b/nslcd/nslcd.c
index 9bfcdd2..4a3fd26 100644
--- a/nslcd/nslcd.c
+++ b/nslcd/nslcd.c
@@ -495,6 +495,7 @@ int main(int argc, char *argv[])
     {
       log_log(LOG_INFO, "caught signal %s (%d), refresh retries",
               signame(nslcd_receivedsignal), nslcd_receivedsignal);
+      /* TODO: force reload yamldir */
       nslcd_receivedsignal = 0;
     }
   }