Add new assert macros, convert server.c to use them

3 files changed, 88 insertions, 74 deletions

diff --git a/igmp.c b/igmp.c
index 9ada4e4..b809e4b 100644
--- a/igmp.c
+++ b/igmp.c
@@ -513,9 +513,9 @@ igmp_init(struct cfg *cfg)
 	sfd = socket(AF_PACKET, SOCK_DGRAM | SOCK_CLOEXEC, htons(ETH_P_ALL));
 	if (sfd < 0) {
 		if (errno == EACCES || errno == EPERM)
-			error("permission denied");
+			info("Unable to do IGMP snooping, permission denied");
 		else
-			error("%mn");
+			error("socket: %m");
 		goto error;
 	}
 
diff --git a/main.h b/main.h
index 9d50bc8..ccd7221 100644
--- a/main.h
+++ b/main.h
@@ -46,17 +46,47 @@ void __debug(enum debug_lvl lvl, const char *fmt, ...) __attribute__((format(pri
 	} while (0)
 
 #define debug(lvl, fmt, ...)	__ifdebug((lvl), "%s:%s:%i: " fmt,		\
-					  __func__, __FILE__, __LINE__ 		\
+					  __FILE__, __func__, __LINE__ 		\
 					  __VA_OPT__(,) __VA_ARGS__)
 #define verbose(fmt, ...)	__ifdebug(DBG_VERBOSE, fmt, __VA_ARGS__)
 #define info(fmt, ...)		__ifdebug(DBG_INFO, fmt,  __VA_ARGS__)
-#define error(fmt, ...)		__ifdebug(DBG_ERROR, "%s: " fmt,		\
-					  __func__ __VA_OPT__(,) __VA_ARGS__)
+#define error(fmt, ...)		__ifdebug(DBG_ERROR, "%s:%s:%i: " fmt,		\
+					  __FILE__, __func__, __LINE__ 		\
+					  __VA_OPT__(,) __VA_ARGS__)
 
 void __die(const char *fmt, ...) __attribute__((format(printf, 1, 2)));
 
-#define die(fmt, ...)		__die("%s:%i: " fmt "\n", __func__,		\
-				      __LINE__  __VA_OPT__(,) __VA_ARGS__)
+#define die(fmt, ...)								\
+	__die("%s:%s:%i: " fmt "\n",						\
+	      __FILE__, __func__, __LINE__					\
+	      __VA_OPT__(,) __VA_ARGS__)
+
+#define assert_log(expr, msg) 							\
+	((expr) ?								\
+	 (true) :								\
+	 (__debug(DBG_ERROR, "%s:%s:%i: assertion \"" msg "\" failed\n",	\
+		  __FILE__, __func__, __LINE__), false))
+
+#define assert_return(expr, ...)						\
+	do {									\
+		if (!assert_log(expr, #expr))					\
+			return __VA_ARGS__;					\
+	} while (0)
+
+#define assert_task_alive_or(lvl, t, cmd) 	\
+do {						\
+	if (!(t)) {				\
+		error("invalid task\n");	\
+		cmd;				\
+	}					\
+						\
+	if ((t)->dead) {			\
+		debug((lvl), "task dead\n");	\
+		cmd;				\
+	}					\
+} while(0)
+
+#define assert_task_alive(lvl, t) assert_task_alive_or((lvl), (t), return)
 
 struct uring_task;
 
@@ -95,21 +125,6 @@ struct uring_task {
 	void *priv;
 };
 
-#define assert_task_alive_or(lvl, t, cmd) 	\
-do {						\
-	if (!(t)) {				\
-		error("invalid task\n");	\
-		cmd;				\
-	}					\
-						\
-	if ((t)->dead) {			\
-		debug((lvl), "task dead\n");	\
-		cmd;				\
-	}					\
-} while(0)
-
-#define assert_task_alive(lvl, t) assert_task_alive_or((lvl), (t), return)
-
 struct cfg {
 	uid_t uid;
 	gid_t gid;
diff --git a/server.c b/server.c
index 8f5cd7d..04275d9 100644
--- a/server.c
+++ b/server.c
@@ -34,8 +34,7 @@ struct server_local {
 static bool
 set_property(struct cfg *cfg, struct server *server, char **property, const char *value)
 {
-	if (!cfg || !server || empty_str(value) || *property)
-		return false;
+	assert_return(cfg && server && !*property && !empty_str(value), false);
 
 	*property = xstrdup(value);
 	if (!*property) {
@@ -52,6 +51,8 @@ server_refdump(struct server *server)
 	struct server_local *local;
 	struct server_proxy *proxy;
 
+	assert_return(server);
+
 	uring_task_refdump(&server->task);
 	list_for_each_entry(local, &server->locals, list)
 		uring_task_refdump(&local->task);
@@ -66,6 +67,8 @@ server_free(struct uring_task *task)
 {
 	struct server *server = container_of(task, struct server, task);
 
+	assert_return(task);
+
 	debug(DBG_SRV, "freeing server %s (%p)", server->name, server);
 	list_del(&server->list);
 	xfree(server->pretty_name);
@@ -88,6 +91,8 @@ server_delete(struct cfg *cfg, struct server *server)
 	struct saddr *tmp;
 	struct dns_async *dns, *dtmp;
 
+	assert_return(cfg && server);
+
 	verbose("Removing server %s", server->name);
 
 	idle_delete(cfg, server);
@@ -122,8 +127,7 @@ server_delete_by_name(struct cfg *cfg, const char *name)
 {
 	struct server *server;
 
-	if (!cfg || empty_str(name))
-		return;
+	assert_return(cfg && !empty_str(name));
 
 	list_for_each_entry(server, &cfg->servers, list) {
 		if (!strcmp(server->name, name)) {
@@ -140,6 +144,8 @@ server_dump(struct server *server)
 	struct saddr *remote;
 	struct saddr *rcon;
 
+	assert_return(server);
+
 	verbose("Server %s:", server->name);
 	switch (server->type) {
 	case SERVER_TYPE_ANNOUNCE:
@@ -181,6 +187,8 @@ server_local_free(struct uring_task *task)
 {
 	struct server_local *local = container_of(task, struct server_local, task);
 
+	assert_return(task);
+
 	debug(DBG_SRV, "task %p, local %p", task, local);
 	list_del(&local->list);
 	xfree(local);
@@ -193,10 +201,11 @@ server_local_accept(struct cfg *cfg, struct uring_task *task, int res)
 	struct server *server = container_of(task->parent, struct server, task);
 	struct server_proxy *proxy;
 
-	debug(DBG_SRV, "task %p, res %i, server %s", task, res, server->name);
-
+	assert_return(cfg && task);
 	assert_task_alive(DBG_SRV, task);
 
+	debug(DBG_SRV, "task %p, res %i, server %s", task, res, server->name);
+
 	if (res < 0) {
 		error("result was %i", res);
 		goto out;
@@ -229,6 +238,8 @@ server_local_open(struct cfg *cfg, struct server *server, struct server_local *l
 	int option;
 	int r;
 
+	assert_return(cfg && server && local, false);
+
 	sfd = socket(local->local.storage.ss_family, SOCK_STREAM | SOCK_CLOEXEC, 0);
 	if (sfd < 0) {
 		error("socket: %m");
@@ -279,7 +290,7 @@ error:
 static void
 server_exec_free(struct uring_task *task)
 {
-	//struct server *server = container_of(task, struct server, exec_task);
+	assert_return(task);
 
 	debug(DBG_SRV, "called");
 }
@@ -296,8 +307,9 @@ server_exec_done(struct cfg *cfg, struct uring_task *task, int res)
 	int r;
 	siginfo_t info;
 
-	/* Should we leave child processes running? */
+	assert_return(cfg && task);
 	assert_task_alive_or(DBG_SRV, task, goto out);
+	/* Should we leave child processes running? */
 
 	if (!(res & POLLIN)) {
 		error("unexpected result: %i", res);
@@ -324,6 +336,8 @@ server_exec_child(void *ptr)
 {
 	const char *cmd = ptr;
 
+	assert_return(ptr, EINVAL);
+
 	execl(cmd, cmd, NULL);
 	return errno;
 }
@@ -339,16 +353,12 @@ server_exec(struct cfg *cfg, struct server *server, const char *cmd)
 	int pidfd;
 	int r;
 
-	if (!cfg || !server || !cmd)
-		return false;
-
-	if (server->exec_task.fd >= 0)
-		return false;
+	assert_return(cfg && server && cmd && server->exec_task.fd < 1, false);
 
         r = clone(server_exec_child, stack + sizeof(stack),
                   CLONE_VM | CLONE_VFORK | CLONE_PIDFD | SIGCHLD,
                   (void *)cmd, &pidfd);
-	if (r < 0) {
+	if (r != 0) {
 		error("clone: %m");
 		return false;
 	}
@@ -361,6 +371,8 @@ server_exec(struct cfg *cfg, struct server *server, const char *cmd)
 static bool
 server_check_running(struct cfg *cfg, struct server *server)
 {
+	assert_return(cfg && server, false);
+
 	/* FIXME: other methods, rcon? */
 	if (server->systemd_service) {
 		verbose("%s: checking if systemd service is running", server->name);
@@ -379,9 +391,7 @@ server_check_running(struct cfg *cfg, struct server *server)
 bool
 server_start(struct cfg *cfg, struct server *server)
 {
-	if (!cfg || !server)
-		return false;
-
+	assert_return(cfg && server, false);
 	assert_task_alive_or(DBG_SRV, &server->task, return false);
 
 	switch (server->start_method) {
@@ -411,9 +421,7 @@ server_start(struct cfg *cfg, struct server *server)
 bool
 server_stop(struct cfg *cfg, struct server *server)
 {
-	if (!cfg || !server)
-		return false;
-
+	assert_return(cfg && server, false);
 	assert_task_alive_or(DBG_SRV, &server->task, return false);
 
 	switch (server->stop_method) {
@@ -450,11 +458,7 @@ server_commit(struct cfg *cfg, struct server *server)
 	struct server_local *local;
 	uint16_t port;
 
-	if (!server || !server->name) {
-		error("called with invalid parameters");
-		return false;
-	}
-
+	assert_return(cfg && server && server->name, false);
 	assert_task_alive_or(DBG_SRV, &server->task, return false);
 
 	if (server->state != SERVER_STATE_INIT) {
@@ -614,9 +618,7 @@ server_commit(struct cfg *cfg, struct server *server)
 bool
 server_add_remote(struct cfg *cfg, struct server *server, struct saddr *remote)
 {
-	if (!server || !remote)
-		return false;
-
+	assert_return(cfg && server && remote, false);
 	assert_task_alive_or(DBG_SRV, &server->task, return false);
 
 	debug(DBG_SRV, "adding remote: %s", remote->addrstr);
@@ -629,11 +631,7 @@ server_add_local(struct cfg *cfg, struct server *server, struct saddr *saddr)
 {
 	struct server_local *local;
 
-	if (!server || !saddr) {
-		error("missing arguments");
-		return false;
-	}
-
+	assert_return(cfg && server && saddr, false);
 	assert_task_alive_or(DBG_SRV, &server->task, return false);
 
 	local = zmalloc(sizeof(*local));
@@ -642,10 +640,9 @@ server_add_local(struct cfg *cfg, struct server *server, struct saddr *saddr)
 		return false;
 	}
 
+	debug(DBG_SRV, "adding local: %s", saddr->addrstr);
 	local->local = *saddr;
 	uring_task_init(&local->task, "local", &server->task, server_local_free);
-
-	debug(DBG_SRV, "adding local: %s", saddr->addrstr);
 	list_add(&local->list, &server->locals);
 	xfree(saddr);
 	return true;
@@ -654,9 +651,7 @@ server_add_local(struct cfg *cfg, struct server *server, struct saddr *saddr)
 bool
 server_add_rcon(struct cfg *cfg, struct server *server, struct saddr *rcon)
 {
-	if (!server || !rcon)
-		return false;
-
+	assert_return(cfg && server && rcon, false);
 	assert_task_alive_or(DBG_SRV, &server->task, return false);
 
 	debug(DBG_SRV, "adding rcon: %s", rcon->addrstr);
@@ -668,6 +663,8 @@ bool
 server_set_rcon_password(struct cfg *cfg, struct server *server,
 			 const char *password)
 {
+	assert_return(cfg && server && !empty_str(password), false);
+
 	return set_property(cfg, server, &server->rcon_password, password);
 }
 
@@ -678,8 +675,7 @@ server_set_systemd_service(struct cfg *cfg, struct server *server,
 	const char *suffix;
 	char *tmp;
 
-	if (!cfg || !server || empty_str(service) || server->systemd_service)
-		return false;
+	assert_return(cfg && server && !empty_str(service) && !server->systemd_service, false);
 
 	suffix = strrchr(service, '.');
 	if (!suffix || strcmp(suffix, ".service")) {
@@ -702,9 +698,8 @@ bool
 server_set_stop_method(struct cfg *cfg, struct server *server,
 		       enum server_stop_method stop_method)
 {
-	if (server->stop_method != SERVER_STOP_METHOD_UNDEFINED ||
-	    stop_method == SERVER_STOP_METHOD_UNDEFINED)
-		return false;
+	assert_return(server->stop_method == SERVER_STOP_METHOD_UNDEFINED &&
+		      stop_method != SERVER_STOP_METHOD_UNDEFINED, false);
 
 	server->stop_method = stop_method;
 	return true;
@@ -714,9 +709,8 @@ bool
 server_set_start_method(struct cfg *cfg, struct server *server,
 			enum server_start_method start_method)
 {
-	if (server->start_method != SERVER_START_METHOD_UNDEFINED ||
-	    start_method == SERVER_START_METHOD_UNDEFINED)
-		return false;
+	assert_return(server->start_method == SERVER_START_METHOD_UNDEFINED &&
+		      start_method != SERVER_START_METHOD_UNDEFINED, false);
 
 	server->start_method = start_method;
 	return true;
@@ -725,20 +719,23 @@ server_set_start_method(struct cfg *cfg, struct server *server,
 bool
 server_set_stop_exec(struct cfg *cfg, struct server *server, const char *cmd)
 {
+	assert_return(cfg && server && !empty_str(cmd), false);
+
 	return set_property(cfg, server, &server->stop_exec, cmd);
 }
 
 bool
 server_set_start_exec(struct cfg *cfg, struct server *server, const char *cmd)
 {
+	assert_return(cfg && server && !empty_str(cmd), false);
+
 	return set_property(cfg, server, &server->start_exec, cmd);
 }
 
 bool
 server_set_idle_timeout(struct cfg *cfg, struct server *server, uint16_t timeout)
 {
-	if (!server || server->idle_timeout != 0)
-		return false;
+	assert_return(cfg && server && timeout > 0 && server->idle_timeout == 0, false);
 
 	server->idle_timeout = timeout;
 	return true;
@@ -747,8 +744,7 @@ server_set_idle_timeout(struct cfg *cfg, struct server *server, uint16_t timeout
 bool
 server_set_port(struct cfg *cfg, struct server *server, uint16_t port)
 {
-	if (!server || server->announce_port != 0)
-		return false;
+	assert_return(cfg && server && port > 0 && server->announce_port == 0, false);
 
 	server->announce_port = htons(port);
 	return true;
@@ -757,8 +753,7 @@ server_set_port(struct cfg *cfg, struct server *server, uint16_t port)
 bool
 server_set_type(struct cfg *cfg, struct server *server, enum server_type type)
 {
-	if (!server || server->type != SERVER_TYPE_UNDEFINED)
-		return false;
+	assert_return(cfg && server && type != SERVER_TYPE_UNDEFINED, false);
 
 	switch (type) {
 	case SERVER_TYPE_ANNOUNCE:
@@ -777,6 +772,8 @@ server_set_type(struct cfg *cfg, struct server *server, enum server_type type)
 bool
 server_set_pretty_name(struct cfg *cfg, struct server *server, const char *pretty_name)
 {
+	assert_return(cfg && server && !empty_str(pretty_name), false);
+
 	return set_property(cfg, server, &server->pretty_name, pretty_name);
 }
 
@@ -785,6 +782,8 @@ server_new(struct cfg *cfg, const char *name)
 {
 	struct server *server;
 
+	assert_return(cfg && !empty_str(name), NULL);
+
 	list_for_each_entry(server, &cfg->servers, list) {
 		if (strcmp(name, server->name))
 			continue;