From ab51ac11e68ce0b075688bf17fc89e0ba645b2ed Mon Sep 17 00:00:00 2001 From: David Härdeman Date: Sat, 20 Jun 2020 19:34:58 +0200 Subject: Add new assert macros, convert server.c to use them --- igmp.c | 4 +-- main.h | 55 +++++++++++++++++++++------------- server.c | 103 +++++++++++++++++++++++++++++++-------------------------------- 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; -- cgit v1.2.3