Bring some sanity to cmd parsing

4 files changed, 220 insertions, 226 deletions

diff --git a/minecctl/minecctl-commands.h b/minecctl/minecctl-commands.h
index 17cf610..444204f 100644
--- a/minecctl/minecctl-commands.h
+++ b/minecctl/minecctl-commands.h
@@ -13,54 +13,37 @@ enum commands {
 	CMD_CONSOLE,
 };
 
-enum command_args {
-	CMD_ARG_INVALID = 0,
-	CMD_ARG_NONE,
-	CMD_ARG_ONE_OPTIONAL,
-	CMD_ARG_AT_LEAST_ONE,
-};
-
 static struct command_list {
 	const char *name;
 	enum commands cmd;
-	enum command_args args;
 } command_list[] = {
 	{
 		.name = "list",
 		.cmd = CMD_LIST,
-		.args = CMD_ARG_NONE,
 	}, {
 		.name = "status",
 		.cmd = CMD_STATUS,
-		.args = CMD_ARG_ONE_OPTIONAL,
 	}, {
 		.name = "ping",
 		.cmd = CMD_PING,
-		.args = CMD_ARG_ONE_OPTIONAL,
 	}, {
 		.name = "stop",
 		.cmd = CMD_STOP,
-		.args = CMD_ARG_ONE_OPTIONAL,
 	}, {
 		.name = "stopall",
 		.cmd = CMD_STOPALL,
-		.args = CMD_ARG_NONE,
 	}, {
 		.name = "pcount",
 		.cmd = CMD_PCOUNT,
-		.args = CMD_ARG_ONE_OPTIONAL,
 	}, {
 		.name = "cmd",
 		.cmd = CMD_COMMAND,
-		.args = CMD_ARG_AT_LEAST_ONE,
 	}, {
 		.name = "console",
 		.cmd = CMD_CONSOLE,
-		.args = CMD_ARG_ONE_OPTIONAL,
 	}, {
 		.name = NULL,
 		.cmd = CMD_INVALID,
-		.args = CMD_ARG_INVALID,
 	}
 };
 
diff --git a/minecctl/minecctl-rcon.c b/minecctl/minecctl-rcon.c
index 6ee8b50..fd880f5 100644
--- a/minecctl/minecctl-rcon.c
+++ b/minecctl/minecctl-rcon.c
@@ -293,6 +293,21 @@ do_ping(_unused_ struct cfg *cfg) {
 	die("Not implemented");
 }
 
+static bool
+get_player_count(int fd, unsigned *current, unsigned *max)
+{
+	char buf[4096];
+	const char *reply;
+
+	if (get_one_status(fd, buf, sizeof(buf), "list", 2,
+			   "There are %u of a max %u players online",
+			   &reply, current, max))
+		return true;
+
+	else
+		return false;
+}
+
 void
 do_stop(struct cfg *cfg) {
 	int fd;
@@ -300,22 +315,48 @@ do_stop(struct cfg *cfg) {
 	assert_die(cfg, "invalid arguments");
 
 	fd = rcon_login(cfg);
+	if (cfg->force_stop) {
+		unsigned current, _unused_ max;
+
+		/* FIXME: args optional */
+		if (!get_player_count(fd, &current, &max)) {
+			error("Unable to get player count");
+			return;
+		} else if (current > 0) {
+			error("Not stopping server, there are active players"
+			      " (use -f to force stop)");
+			return;
+		}
+	}
+
 	send_cmd(fd, "stop");
 }
 
 void
 do_stop_all(_unused_ struct cfg *cfg) {
+	/*
+	struct server *server;
+	int fd;
+
+	list_for_each_entry(server, &cfg->known_servers, list) {
+		read_server_config(server->filename);
+		*/
+	
 	die("Not implemented");
 }
 
 void
 do_pcount(struct cfg *cfg) {
 	int fd;
+	unsigned current, max;
 
 	assert_die(cfg, "invalid arguments");
 
 	fd = rcon_login(cfg);
-	send_cmd(fd, "list");
+	if (get_player_count(fd, &current, &max))
+		info("Players: %u/%u", current, max);
+	else
+		die("Failed to get player count");
 }
 
 void
diff --git a/minecctl/minecctl.c b/minecctl/minecctl.c
index ed91fdf..6d60a32 100644
--- a/minecctl/minecctl.c
+++ b/minecctl/minecctl.c
@@ -137,7 +137,7 @@ dump_config()
 	assert_die(cfg, "cfg not set");
 
 	info("Configuration:");
-	info("pwd           : %s", cfg->password);
+	info("password      : %s", cfg->password);
 	info("cfgdir        : %s", cfg->cfgdir);
 	info("addrstr       : %s", cfg->addrstr);
 	info("cmdstr        : %s", cfg->cmdstr);
@@ -150,12 +150,9 @@ dump_config()
 }
 
 static void
-parse_server_config(char *buf, const char *filename,
-		    struct list_head *addrs)
+parse_server_config(char *buf, const char *filename)
 {
-	list_init(addrs);
-
-	assert_die(buf && filename && addrs && cfg, "invalid arguments");
+	assert_die(buf && filename && cfg, "invalid arguments");
 
 	if (!config_parse_header(SERVER_CFG_HEADER, &buf))
 		die("Unable to parse %s: invalid/missing header", filename);
@@ -172,25 +169,36 @@ parse_server_config(char *buf, const char *filename,
 
 		switch (key) {
 		case SCFG_KEY_RCON:
-			list_replace(&value.saddrs, addrs);
+			if (!list_empty(&cfg->addrs))
+				die("rcon address defined twice in %s", filename);
+			list_replace(&value.saddrs, &cfg->addrs);
 			break;
 		case SCFG_KEY_RCON_PASSWORD:
 			if (!cfg->password)
 				cfg->password = xstrdup(value.str);
 			break;
+		case SCFG_KEY_REMOTE:
+			if (!list_empty(&cfg->mcaddrs))
+				die("rcon address defined twice in %s", filename);
+			list_replace(&value.saddrs, &cfg->mcaddrs);
 		default:
 			continue;
 		}
 
-		if (cfg->password && !list_empty(addrs))
+		if (cfg->password &&
+		    !list_empty(&cfg->addrs) &&
+		    !list_empty(&cfg->mcaddrs))
 			break;
 	}
 
 	if (!cfg->password)
 		verbose("rcon password not found in %s", filename);
 
-	if (list_empty(addrs))
-		die("rcon address not found in %s", filename);
+	if (list_empty(&cfg->addrs))
+		verbose("rcon address not found in %s", filename);
+
+	if (list_empty(&cfg->mcaddrs))
+		verbose("mc server address not found in %s", filename);
 }
 
 static void
@@ -229,79 +237,74 @@ read_server_config()
 	buf[off] = '\0';
 	close(fd);
 
-	parse_server_config(buf, cfg->server->filename, &cfg->addrs);
+	parse_server_config(buf, cfg->server->filename);
+	dump_config();
 }
 
 _noreturn_ static void
-usage(bool invalid)
+usage(bool no_error)
 {
-	if (invalid)
-		error("Invalid option(s)");
-
 	info("Usage: %s [OPTION...] COMMAND\n"
 	     "\n"
 	     "Valid options:\n"
-	     "  -p, --password=PWD    use PWD as the rcon password\n"
-	     "                        (or use environment variable RCON_PASSWORD)\n"
-	     "  -a, --address=ADDR    connect to rcon at ADDR\n"
-	     "                        (or use environment variable RCON_ADDRESS)\n"
-	     "  -A, --mcaddress=ADDR  connect to Minecraft server at ADDR\n"
-	     "                        (potentially used for ping, status, pcount)\n"
-	     "  -c, --cfgdir=DIR      look for server configurations in DIR\n"
-	     "                        (default: %s)\n"
-	     "  -f, --force           stop server even if it has players\n"
-	     "  -v, --verbose         enable extra logging\n"
-	     "  -h, --help            print this information\n"
+	     "  -p, --password=PWD       use PWD as the rcon password\n"
+	     "                           (or use environment variable RCON_PASSWORD)\n"
+	     "  -a, --rcon-address=ADDR  connect to rcon at ADDR\n"
+	     "                           (or use environment variable RCON_ADDRESS)\n"
+	     "  -m, --mc-address=ADDR    connect to Minecraft server at ADDR\n"
+	     "                           (only relevant for some commands, can also\n"
+	     "                            use environment variable MC_ADDRESS)\n"
+	     "  -c, --cfgdir=DIR         look for server configurations in DIR\n"
+	     "                           (default: %s)\n"
+	     "  -f, --force              stop server even if it has players\n"
+	     "  -v, --verbose            enable extra logging\n"
+	     "  -h, --help               print this information\n"
 	     "\n"
 	     "Valid commands:\n"
-	     "  list                  list known servers\n"
-	     "  status [SERVER]       show status of SERVER (or all known servers)\n"
-	     "  ping [SERVER]         check if SERVER is running\n"
-	     "  stop [SERVER]         stop SERVER\n"
-	     "  stopall               stop all known servers\n"
-	     "  pcount [SERVER]       get player count for SERVER\n"
-	     "  console [SERVER]      provide an interactive command line for SERVER\n"
-	     "  cmd [SERVER] CMD      send CMD to SERVER\n"
-	     "  [SERVER] CMD          shorthand for \"cmd [SERVER] CMD\"\n"
-	     "  [SERVER]              shorthand for \"console [SERVER]\"\n"
+	     "  list                     list known servers\n"
+	     "  status [SERVER]          show status of SERVER (or all known servers)\n"
+	     "  ping [SERVER]            check if SERVER is running\n"
+	     "  stop [SERVER]            stop SERVER\n"
+	     "  stopall                  stop all known servers\n"
+	     "  pcount [SERVER]          get player count for SERVER\n"
+	     "  console [SERVER]         interactive command line for SERVER\n"
+	     "  cmd [SERVER] CMD         send CMD to SERVER\n"
+	     "  [SERVER] CMD             shorthand for \"cmd [SERVER] CMD\"\n"
+	     "  [SERVER]                 shorthand for \"console [SERVER]\"\n"
 	     "\n"
 	     "Note: if ADDR is given as an option, SERVER must be omitted from\n"
-	     "      the command and vice versa.\n"
-	     "\n",
+	     "      the command and vice versa.\n",
 	     program_invocation_short_name, DEFAULT_CFG_DIR);
 
-	exit(invalid ? EXIT_FAILURE : EXIT_SUCCESS);
+	exit(no_error ? EXIT_FAILURE : EXIT_SUCCESS);
 }
 
 static char *
-combine_args(int index, int remain, char **argv)
+strv_join(char * const *strv)
 {
 	size_t len = 0;
-	char *result, *pos;
-
-	if (index < 0 || remain < 0)
-		die("Internal error parsing arguments");
+	char *r, *to;
 
-	for (int i = index; i < index + remain; i++)
-		len += strlen(argv[i]) + 1;
+	for (unsigned i = 0; strv[i]; i++)
+		len += strlen(strv[i]) + 1;
 
 	if (len == 0)
-		die("Internal error parsing arguments");
+		return NULL;
 
-	len++;
-	result = zmalloc(len);
-	pos = result;
-	for (int i = index; i < index + remain; i++)
-		pos += sprintf(pos, "%s ", argv[i]);
+	r = zmalloc(len);
+	to = r;
 
-	pos--;
-	*pos = '\0';
+	for (unsigned i = 0; strv[i]; i++) {
+		if (i > 0)
+			*(to++) = ' ';
+		to = stpcpy(to, strv[i]);
+	}
 
-	return result;
+	return r;
 }
 
 static void
-get_servers()
+get_known_servers()
 {
 	static bool first = true;
 	struct dirent *dent;
@@ -344,28 +347,10 @@ do_list(struct cfg *cfg)
 {
 	struct server *server;
 
-	get_servers();
-
 	list_for_each_entry(server, &cfg->known_servers, list)
 		info("%s", server->shortname);
 }
 
-static bool
-is_known_server(const char *name)
-{
-	struct server *server;
-
-	assert_die(cfg && name, "invalid arguments");
-
-	get_servers();
-	list_for_each_entry(server, &cfg->known_servers, list) {
-		if (streq(name, server->shortname))
-			return true;
-	}
-
-	return false;
-}
-
 static void
 set_server(const char *name)
 {
@@ -374,7 +359,6 @@ set_server(const char *name)
 	assert_die(cfg, "invalid arguments");
 	assert_die(!cfg->server, "can't set server twice");
 
-	get_servers();
 	list_for_each_entry(server, &cfg->known_servers, list) {
 		if (streq(name, server->shortname)) {
 			cfg->server = server;
@@ -383,7 +367,7 @@ set_server(const char *name)
 	}
 
 	error("%s is not a known server", name);
-	usage(true);
+	usage(false);
 }
 
 char *
@@ -454,7 +438,7 @@ connect_any(struct list_head *addrs, bool may_fail)
 
 	if (!connected) {
 		if (may_fail)
-			sfd = -1;
+			return -1;
 		else
 			die("Failed to connect to remote host");
 	}
@@ -462,166 +446,138 @@ connect_any(struct list_head *addrs, bool may_fail)
 	return sfd;
 }
 
-static void
-parse_verb(int index, int remain, char **argv)
+static inline void
+get_optional_server_arg(char * const **argv, bool more)
 {
-	enum command_args args;
-	enum commands cmd;
+	if (!cfg->addrstr) {
+		if (!**argv) {
+			error("Missing argument");
+			usage(false);
+		}
+		set_server(**argv);
+		(*argv)++;
+	}
 
-	assert_die(index >= 0 && remain >= 0 && argv, "invalid arguments");
+	if (!more && **argv) {
+		error("Too many arguments");
+		usage(false);
+	}
+}
 
-	cmd = CMD_INVALID;
+static void
+parse_command(unsigned argc, char * const *argv)
+{
+	enum commands cmd;
 
-	info("start verb, remain %i, index %i", remain, index);
-	for (int i = index; i < index + remain; i++)
-		info("arg[%i]: %s", i, argv[i]);
+	assert_die(argv, "invalid arguments");
 
-	if (remain == 0) {
-		/* shorthand for console if address is set */
-		if (!cfg->addrstr)
-			usage(true);
+	/* Shorthand for console if address is set */
+	if (argc == 0) {
+		if (!cfg->addrstr) {
+			error("Missing options");
+			usage(false);
+		}
 
-		cmd = CMD_CONSOLE;
-		goto out;
+		cfg->cmd = do_console;
+		return;
 	}
 
-	args = CMD_ARG_INVALID;
-	for (int i = 0; command_list[i].name; i++) {
-		if (streq(argv[index], command_list[i].name)) {
+       	cmd = CMD_INVALID;
+	for (unsigned i = 0; command_list[i].name; i++) {
+		if (streq(*argv, command_list[i].name)) {
 			cmd = command_list[i].cmd;
-			args = command_list[i].args;
+			argv++;
 			break;
 		}
 	}
 
-	if (cmd == CMD_INVALID) {
-		/* maybe shorthand for console: [SERVER] CMD */
-		if (cfg->addrstr && is_known_server(argv[index])) {
-			error("Ambigous command, address set and server specified");
-			usage(true);
-		} else if (cfg->addrstr) {
-			/* CMD */
-			cmd = CMD_COMMAND;
-			cfg->cmdstr = combine_args(index, remain, argv);
-			goto out;
-		} else {
-			/* SERVER [CMD] */
-			set_server(argv[index]);
-			index++;
-			remain--;
-
-			if (remain < 1)
-				cmd = CMD_CONSOLE;
-			else {
-				cmd = CMD_COMMAND;
-				cfg->cmdstr = combine_args(index, remain, argv);
-			}
-			goto out;
-		} 
-	}
-
-	index++;
-	remain--;
-
-	info("here: index %i, remain %i, args %i", index, remain, args);
-
-	switch (args) {
-	case CMD_ARG_NONE:
-		info("here: index %i, remain %i, args %i", index, remain, args);
-		if (remain != 0)
-			usage(true);
-		info("here: index %i, remain %i, args %i", index, remain, args);
-		break;
-
-	case CMD_ARG_ONE_OPTIONAL:
-		info("hereY: index %i, remain %i, args %i", index, remain, args);
-		if (remain == 0 && cfg->addrstr)
-			break;
-		else if (remain == 1 && !cfg->addrstr) {
-			set_server(argv[index]);
-			index++;
-			remain--;
-			break;
-		}
-		usage(true);
+	switch (cmd) {
+	case CMD_INVALID:
+		/* shorthand notation? */
+		if (!cfg->addrstr)
+			/* SERVER [CMD...] */
+			set_server(*argv++);
 
-	case CMD_ARG_AT_LEAST_ONE:
-		info("hereX: index %i, remain %i, args %i", index, remain, args);
-		if (remain > 0 && cfg->addrstr)
-			break;
-		else if (remain > 1 && !cfg->addrstr) {
-			set_server(argv[index]);
-			index++;
-			remain--;
+		/* !CMD = console */
+		if (!*argv) {
+			cfg->cmd = do_console;
 			break;
 		}
-		usage(true);
-
-	case CMD_ARG_INVALID:
-		info("hereZ: index %i, remain %i, args %i", index, remain, args);
-		_fallthrough_;
-	default:
-		info("hereT: index %i, remain %i, args %i", index, remain, args);
-		die("Internal cmd parsing error");
-	}
 
-out:
-	switch (cmd) {
+		/* CMD... */
+		cfg->cmdstr = strv_join(argv);
+		cfg->cmd = do_command;
+		break;
 	case CMD_LIST:
+		if (*argv) {
+			error("Too many options");
+			usage(false);
+		}
+		cfg->cmd = do_list;
+		break;
+	case CMD_STOPALL:
+		if (*argv) {
+			error("Too many options");
+			usage(false);
+		}
 		cfg->cmd = do_list;
 		break;
 	case CMD_STATUS:
+		get_optional_server_arg(&argv, false);
 		cfg->cmd = do_status;
 		break;
 	case CMD_PING:
+		get_optional_server_arg(&argv, false);
 		cfg->cmd = do_ping;
 		break;
 	case CMD_STOP:
+		get_optional_server_arg(&argv, false);
 		cfg->cmd = do_stop;
 		break;
-	case CMD_STOPALL:
-		cfg->cmd = do_stop_all;
-		break;
 	case CMD_PCOUNT:
+		get_optional_server_arg(&argv, false);
 		cfg->cmd = do_pcount;
 		break;
 	case CMD_CONSOLE:
+		get_optional_server_arg(&argv, false);
 		cfg->cmd = do_console;
 		break;
 	case CMD_COMMAND:
+		if (!*argv) {
+			error("Missing options");
+			usage(false);
+		}
+
+		if (!cfg->addrstr)
+			set_server(*argv++);
+
+		if (!*argv) {
+			error("Missing options");
+			usage(false);
+		}
+
+		cfg->cmdstr = strv_join(argv);
 		cfg->cmd = do_command;
-	       	cfg->cmdstr = combine_args(index, remain, argv);
-		remain = 0;
 		break;
 	default:
-		die("Internal cmd parsing error");
+		die("Unreachable");
 	}
 
-	if (args == CMD_ARG_NONE && !cfg->server && !cfg->cmdstr)
-		return;
-
-	if (cfg->addrstr && cfg->server)
-		usage(true);
-
 	dump_config();
-
-	info("here2: index %i, remain %i, args %i", index, remain, args);
-	if ((!cfg->addrstr && !cfg->server) ||
-	    (cfg->cmdstr && cmd != CMD_COMMAND) ||
-	    (!cfg->cmdstr && cmd == CMD_COMMAND) ||
-	    (remain != 0) || (cmd == CMD_INVALID))
-		die("Internal cmd parsing error");
 }
 
 static void
-parse_cmdline(int argc, char **argv)
+parse_cmdline(int argc, char * const *argv)
 {
 	int c;
+	char *e;
 
 	assert_die(argc && argv && cfg, "invalid arguments");
 
-	if (argc < 2)
-		usage(true);
+	if (argc < 2) {
+		error("Missing options");
+		usage(false);
+	}
 
 	list_init(&cfg->addrs);
 	cfg->cfgdir = DEFAULT_CFG_DIR;
@@ -629,17 +585,17 @@ parse_cmdline(int argc, char **argv)
 	while (true) {
 		int option_index = 0;
 		static struct option long_options[] = {
-			{ "password",	required_argument,	0, 'p' },
-			{ "address",	required_argument,	0, 'a' },
-			{ "mcaddress",	required_argument,	0, 'A' },
-			{ "cfgdir",	required_argument,	0, 'c' },
-			{ "verbose",	no_argument,		0, 'v' },
-			{ "force",	no_argument,		0, 'f' },
-			{ "help",	no_argument,		0, 'h' },
-			{ 0,		0,			0,  0  }
+			{ "password",	  required_argument,	0, 'p' },
+			{ "rcon-address", required_argument,	0, 'a' },
+			{ "mc-address",   required_argument,	0, 'A' },
+			{ "cfgdir",	  required_argument,	0, 'c' },
+			{ "verbose",	  no_argument,		0, 'v' },
+			{ "force",	  no_argument,		0, 'f' },
+			{ "help",	  no_argument,		0, 'h' },
+			{ 0,		  0,			0,  0  }
 		};
 
-		c = getopt_long(argc, argv, ":p:a:A:c:fvh",
+		c = getopt_long(argc, argv, ":p:a:m:c:fvh",
 				long_options, &option_index);
 
 		if (c == -1)
@@ -652,7 +608,7 @@ parse_cmdline(int argc, char **argv)
 		case 'a':
 			cfg->addrstr = xstrdup(optarg);
 			break;
-		case 'A':
+		case 'm':
 			cfg->mcaddrstr = xstrdup(optarg);
 			break;
 		case 'c':
@@ -665,30 +621,30 @@ parse_cmdline(int argc, char **argv)
 			debug_mask |= DBG_VERBOSE;
 			break;
 		case 'h':
-			usage(false);
-		default:
 			usage(true);
+		default:
+			error("Invalid options");
+			usage(false);
 		}
 	}
 
 	if (!cfg->password) {
-		char *e;
-	
 		e = getenv("RCON_PASSWORD");
 		if (e)
 			cfg->password = xstrdup(e);
 	}
 
 	if (!cfg->addrstr) {
-		char *e;
-
 		e = getenv("RCON_ADDRESS");
 		if (e)
 			cfg->addrstr = xstrdup(e);
 	}
 
-	parse_verb(optind, argc - optind, argv);
-
+	if (!cfg->mcaddrstr) {
+		e = getenv("MC_ADDRESS");
+		if (e)
+			cfg->mcaddrstr = xstrdup(e);
+	}
 }
 
 int
@@ -704,7 +660,15 @@ main(int argc, char **argv)
 	list_init(&cfg->known_servers);
 
 	parse_cmdline(argc, argv);
+	info("cmdline parsed");
+	dump_config();
 
+	get_known_servers();
+	info("known servers loaded");
+	dump_config();
+
+	parse_command(argc - optind, &argv[optind]);
+	info("command parsed");
 	dump_config();
 
 	if (!cfg->cmd)
@@ -733,6 +697,12 @@ main(int argc, char **argv)
 
 	cfg->cmd(cfg);
 
+	if (cfg->password)
+		explicit_bzero(cfg->password, strlen(cfg->password));
+
+	xfree(cfg->password);
+	xfree(cfg->addrstr);
+	xfree(cfg->mcaddrstr);
 	xfree(cfg);
 	exit(EXIT_SUCCESS);
 }
diff --git a/minecctl/minecctl.h b/minecctl/minecctl.h
index 17dc9c6..405a217 100644
--- a/minecctl/minecctl.h
+++ b/minecctl/minecctl.h
@@ -9,9 +9,9 @@ struct server {
 
 struct cfg {
 	char *password;
-	const char *cfgdir;
-	const char *addrstr;
-	const char *mcaddrstr;
+	char *cfgdir;
+	char *addrstr;
+	char *mcaddrstr;
 	char *cmdstr;
 	struct server *server;
 	void (*cmd)(struct cfg *cfg);