From ff9d60b0d5b27369073a329cd5ceb5d6c94bdf84 Mon Sep 17 00:00:00 2001 From: David Härdeman Date: Tue, 30 Jun 2020 23:19:53 +0200 Subject: Fix some of the fallout of the latest reorg --- minecproxy/misc.c | 6 +-- minecproxy/server-config.c | 2 +- minecproxy/server-proxy.c | 1 + minecproxy/server.c | 29 ++++++++--- minecproxy/signal-handler.c | 123 +++++++++++++++++++++++--------------------- shared/config-parser.c | 32 +++++++----- 6 files changed, 109 insertions(+), 84 deletions(-) diff --git a/minecproxy/misc.c b/minecproxy/misc.c index e3872c5..0f0da00 100644 --- a/minecproxy/misc.c +++ b/minecproxy/misc.c @@ -102,9 +102,6 @@ void __xfree(const char *fn, int line, void *ptr) if (!ptr) return; - free(ptr); - malloc_count--; - debug(DBG_MALLOC, "called from %s:%i - %p", fn, line, ptr); list_for_each_entry_safe(a, tmp, &malloc_list, list) { @@ -119,6 +116,9 @@ void __xfree(const char *fn, int line, void *ptr) error("Delete count is %u for ptr 0x%p", delete_count, ptr); exit(EXIT_FAILURE); } + + free(ptr); + malloc_count--; } void debug_resource_usage() diff --git a/minecproxy/server-config.c b/minecproxy/server-config.c index 6fdd73a..de9265d 100644 --- a/minecproxy/server-config.c +++ b/minecproxy/server-config.c @@ -229,7 +229,7 @@ void server_cfg_monitor_init() server = server_new(dent->d_name); if (server) - uring_openat(&server->task, server->name, scfg_open_cb); + uring_openat(&server->task, server->scfg.filename, scfg_open_cb); } closedir(dir); diff --git a/minecproxy/server-proxy.c b/minecproxy/server-proxy.c index 787934d..b8a8988 100644 --- a/minecproxy/server-proxy.c +++ b/minecproxy/server-proxy.c @@ -564,6 +564,7 @@ struct server_local *local_new(struct server *server, struct saddr *saddr) local = zmalloc(sizeof(*local)); if (!local) { error("malloc: %m"); + xfree(saddr); return NULL; } diff --git a/minecproxy/server.c b/minecproxy/server.c index e7efe5d..b06286b 100644 --- a/minecproxy/server.c +++ b/minecproxy/server.c @@ -100,6 +100,7 @@ static void server_dump(struct server *server) assert_return(server); verbose("Server %s:", server->name); + verbose(" * Filename: %s", server->scfg.filename); switch (server->scfg.type) { case SERVER_TYPE_ANNOUNCE: verbose(" * Type: announce"); @@ -428,6 +429,12 @@ bool server_commit(struct server *server) assert_return(server && server->name, false); assert_task_alive_or(DBG_SRV, &server->task, return false); + if (!list_empty(&server->scfg.dnslookups)) { + debug(DBG_SRV, "%s: called with pending DNS requests", + server->name); + return false; + } + if (server->state != SERVER_STATE_INIT) { error("called in wrong state"); return false; @@ -438,14 +445,9 @@ bool server_commit(struct server *server) return false; } - if (!list_empty(&server->scfg.dnslookups)) { - debug(DBG_SRV, "%s: called with pending DNS requests", - server->name); - return true; - } - if (!scfg_validate(&server->scfg)) { - debug(DBG_SRV, "%s: config file not valid", server->name); + error("%s: failed to validate config file", server->name); + server_delete(server); return false; } @@ -469,8 +471,19 @@ bool server_commit(struct server *server) /* FIXME: error checks */ list_del(&saddr->list); local = local_new(server, saddr); - local_open(local); + if (!local) { + error("%s: failed to create local listener", + server->name); + server_delete(server); + return false; + } list_add(&local->list, &server->listenings); + if (!local_open(local)) { + error("%s: failed to open listening port", + server->name); + server_delete(server); + return false; + } } server->state = SERVER_STATE_CFG_OK; diff --git a/minecproxy/signal-handler.c b/minecproxy/signal-handler.c index ba92382..5369670 100644 --- a/minecproxy/signal-handler.c +++ b/minecproxy/signal-handler.c @@ -33,74 +33,77 @@ static void signalfd_read(struct uring_task *task, int res) struct signal_ev *signal = container_of(task, struct signal_ev, task); struct server *server, *stmp; static unsigned count = 0; - siginfo_t *si; + siginfo_t *sia, *si; assert_return(task); assert_task_alive(DBG_SIG, task); - si = (siginfo_t *)task->tbuf->buf; - if (res != sizeof(*si)) - die("error in signalfd (%i)", res); - - switch (si->si_signo) { - case SIGUSR1: { - struct dns_async *dns; - - debug(DBG_SIG, "Got a SIGUSR1"); - if (si->si_code != SI_ASYNCNL || !si->si_ptr) { - error("SIGUSR1: unexpected values in siginfo"); - goto out; - } - - dns = si->si_ptr; - if (!dns->cb) { - error("DNS callback not set"); - goto out; + if (res == 0 || res % sizeof(*si)) + die("error in signalfd (%i vs %zu)", res, sizeof(*si)); + + sia = (siginfo_t *)(task->tbuf->buf); + for (unsigned i = 0; i < res / sizeof(*si); i++) { + si = &sia[i]; + switch (si->si_signo) { + case SIGUSR1: { + struct dns_async *dns; + + debug(DBG_SIG, "Got a SIGUSR1"); + if (si->si_code != SI_ASYNCNL || !si->si_ptr) { + error("SIGUSR1: unexpected values in siginfo"); + goto out; + } + + dns = si->si_ptr; + if (!dns->cb) { + error("DNS callback not set (%p)", dns); + goto out; + } + + debug(DBG_DNS, "DNS lookup complete, dns: %p, dns->cb: %p", dns, + dns->cb); + dns->cb(dns); + break; } - debug(DBG_DNS, "DNS lookup complete, dns: %p, dns->cb: %p", dns, - dns->cb); - dns->cb(dns); - break; - } - - case SIGUSR2: - debug(DBG_SIG, "got a SIGUSR2"); - dump_tree(); - break; - - case SIGTERM: - debug(DBG_SIG, "Got a SIGINT/SIGHUP"); - verbose("got a signal to quit"); - sd_notifyf(0, "STOPPING=1\nSTATUS=Received signal, exiting"); - exit(EXIT_SUCCESS); - break; - - case SIGINT: - case SIGHUP: - count++; - if (count > 5) { + case SIGUSR2: + debug(DBG_SIG, "got a SIGUSR2"); dump_tree(); - exit(EXIT_FAILURE); + break; + + case SIGTERM: + debug(DBG_SIG, "Got a SIGINT/SIGHUP"); + verbose("got a signal to quit"); + sd_notifyf(0, "STOPPING=1\nSTATUS=Received signal, exiting"); + exit(EXIT_SUCCESS); + break; + + case SIGINT: + case SIGHUP: + count++; + if (count > 5) { + dump_tree(); + exit(EXIT_FAILURE); + } + + verbose("got a signal to dump tree"); + sd_notifyf(0, "STOPPING=1\nSTATUS=Received signal, exiting"); + dump_tree(); + signal_delete(); + ptimer_delete(); + igmp_delete(); + announce_delete(); + idle_delete(); + server_cfg_monitor_delete(); + list_for_each_entry_safe(server, stmp, &cfg->servers, list) + server_delete(server); + uring_delete(); + return; + + default: + error("got an unknown signal: %i", si->si_signo); + break; } - - verbose("got a signal to dump tree"); - sd_notifyf(0, "STOPPING=1\nSTATUS=Received signal, exiting"); - dump_tree(); - signal_delete(); - ptimer_delete(); - igmp_delete(); - announce_delete(); - idle_delete(); - server_cfg_monitor_delete(); - list_for_each_entry_safe(server, stmp, &cfg->servers, list) - server_delete(server); - uring_delete(); - return; - - default: - error("got an unknown signal: %i", si->si_signo); - break; } out: diff --git a/shared/config-parser.c b/shared/config-parser.c index 850bae8..c238bb7 100644 --- a/shared/config-parser.c +++ b/shared/config-parser.c @@ -116,6 +116,10 @@ static void scfg_dns_rcon_cb(struct dns_async *dns) scfg_dns_cb(dns, SCFG_KEY_RCON); } +/* + * FIXME: this is brittle, the dns request has already been triggered, + * can't just cancel on error. + */ static bool handle_dns(struct server_config *scfg, enum scfg_keys type, struct cfg_value *value, notification_cb_t notification_cb) @@ -155,7 +159,7 @@ static bool handle_dns(struct server_config *scfg, enum scfg_keys type, return false; } - if (!!notification_cb) { + if (!notification_cb) { error("async DNS lookup received but not requested"); xfree(dns); return false; @@ -263,12 +267,12 @@ bool scfg_validate(struct server_config *scfg) error("%s: systemd start/stop method missing " "systemd service", scfg->filename); return false; - } else { - if (scfg->systemd_service || scfg->systemd_obj) { - error("%s: systemd service set but not used", - scfg->filename); - return false; - } + } + } else { + if (scfg->systemd_service || scfg->systemd_obj) { + error("%s: systemd service set but not used", + scfg->filename); + return false; } } @@ -313,7 +317,7 @@ bool scfg_validate(struct server_config *scfg) break; case SERVER_TYPE_PROXY: - if (scfg->announce_port == 0) { + if (scfg->announce_port != 0) { error("%s: can't set announce port for proxy server", scfg->filename); return false; @@ -581,11 +585,14 @@ bool scfg_init(struct server_config *scfg, const char *filename) if (filename) { scfg->filename = xstrdup(filename); + if (!scfg->filename) { error("strdup: %m"); return false; } - } + } else + scfg->filename = NULL; + scfg->type = SERVER_TYPE_UNDEFINED; scfg->pretty_name = NULL; scfg->announce_port = 0; @@ -1042,9 +1049,10 @@ bool config_parse_line(const char *filename, char **buf, /* sanity check */ if ((rvalue->type != kvmap[i].value_type) && - ((kvmap[i].value_type != CFG_VAL_TYPE_ASYNC_ADDRS) && - (rvalue->type != CFG_VAL_TYPE_ADDRS))) { - error("rvalue->type != kvmap->type"); + ((kvmap[i].value_type != CFG_VAL_TYPE_ADDRS) && + (rvalue->type != CFG_VAL_TYPE_ASYNC_ADDRS))) { + error("rvalue->type != kvmap->type (%i %i)", + rvalue->type, kvmap[i].value_type); goto error; } -- cgit v1.2.3