Only check for password if/when required

5 files changed, 128 insertions, 83 deletions

diff --git a/minecctl/minecctl-rcon.c b/minecctl/minecctl-rcon.c
index f5a9bb5..6ee8b50 100644
--- a/minecctl/minecctl-rcon.c
+++ b/minecctl/minecctl-rcon.c
@@ -100,16 +100,25 @@ send_msg(int sfd, char *buf, size_t len, enum rcon_packet_type type,
 	rcon_packet_id++;
 }
 
-static void
-send_login(struct cfg *cfg)
+static int
+rcon_login(struct cfg *cfg)
 {
 	char buf[4096];
 	int32_t rtype;
 	const char *reply;
+	int fd;
+
+	assert_die(cfg, "invalid arguments");
+
+	if (!cfg->password)
+		cfg->password = ask_password();
 
-	assert_die(cfg && cfg->fd >= 0 && cfg->password, "invalid arguments");
+	if (!cfg->password)
+		die("Can't login - no password");
 
-	send_msg(cfg->fd, buf, sizeof(buf), RCON_PACKET_LOGIN, cfg->password,
+	fd = connect_any(&cfg->addrs, false);
+
+	send_msg(fd, buf, sizeof(buf), RCON_PACKET_LOGIN, cfg->password,
 		 &rtype, &reply);
 
 	/* An rcon password isn't exactly super-secret, but can't hurt */
@@ -124,6 +133,8 @@ send_login(struct cfg *cfg)
 		die("Login failure, invalid password?");
 	else
 		die("Invalid return code: %" PRIi32, rtype);
+
+	return fd;
 }
 
 static void
@@ -195,13 +206,13 @@ mctime_mm(unsigned mctime) {
 }
 
 static bool
-get_one_status(struct cfg *cfg, char *buf, size_t len, const char *cmd,
+get_one_status(int fd, char *buf, size_t len, const char *cmd,
 	       size_t argc, const char *replyscan, const char **reply, ...)
 {
 	va_list ap;
 	int r;
 
-	get_info(cfg->fd, buf, len, cmd, reply);
+	get_info(fd, buf, len, cmd, reply);
 
 	va_start(ap, reply);
 	r = vsscanf(*reply, replyscan, ap);
@@ -221,53 +232,56 @@ do_status(struct cfg *cfg) {
 	unsigned cplayers, maxplayers, gtime;
 	unsigned epacks, apacks;
 	unsigned bannedplayers, bannedips;
+	int fd;
 
-	send_login(cfg);
+	assert_die(cfg, "invalid arguments");
 
-	if (get_one_status(cfg, buf, sizeof(buf), "seed", 1,
+	fd = rcon_login(cfg);
+
+	if (get_one_status(fd, buf, sizeof(buf), "seed", 1,
 			   "Seed : [ %[^]]]", &reply, tbuf))
 		info("Seed: %s", tbuf);
 		       
-	if (get_one_status(cfg, buf, sizeof(buf), "difficulty", 1,
+	if (get_one_status(fd, buf, sizeof(buf), "difficulty", 1,
 			   "The difficulty is %s", &reply, tbuf))
 		info("Difficulty: %s", tbuf);
 
-	if (get_one_status(cfg, buf, sizeof(buf), "list", 2,
+	if (get_one_status(fd, buf, sizeof(buf), "list", 2,
 			   "There are %u of a max %u players online",
 			   &reply, &cplayers, &maxplayers))
 		info("Players: %u/%u", cplayers, maxplayers);
 
-	if (get_one_status(cfg, buf, sizeof(buf), "time query day", 1,
+	if (get_one_status(fd, buf, sizeof(buf), "time query day", 1,
 			   "The time is %u", &reply, &gtime))
 		info("In-game days: %u", gtime);
 
-	if (get_one_status(cfg, buf, sizeof(buf), "time query gametime", 1,
+	if (get_one_status(fd, buf, sizeof(buf), "time query gametime", 1,
 			   "The time is %u", &reply, &gtime))
 		info("World age: %ud:%02uh:%02um",
 		     mctime_days(gtime), mctime_hh(gtime), mctime_mm(gtime));
 
-	if (get_one_status(cfg, buf, sizeof(buf), "time query daytime", 1,
+	if (get_one_status(fd, buf, sizeof(buf), "time query daytime", 1,
 			   "The time is %u", &reply, &gtime))
 		info("Current in-game time: %02uh:%02um",
 		     mctime_hh(gtime + MCTIME_OFFSET), mctime_mm(gtime + MCTIME_OFFSET));
 
-	if (get_one_status(cfg, buf, sizeof(buf), "datapack list enabled", 2,
+	if (get_one_status(fd, buf, sizeof(buf), "datapack list enabled", 2,
 			   "There are %u data packs enabled: %[^\n]", &reply, &epacks, tbuf))
 		info("Enabled data packs (%u): %s", epacks, tbuf);
 
-	if (get_one_status(cfg, buf, sizeof(buf), "datapack list available", 2,
+	if (get_one_status(fd, buf, sizeof(buf), "datapack list available", 2,
 			   "There are %u data packs available : %[^\n]", &reply, &apacks, tbuf))
 		info("Available data packs (%u): %s", apacks, tbuf);
 	else if (streq(reply, "There are no more data packs available"))
 		info("Available data packs: none");
 
-	if (get_one_status(cfg, buf, sizeof(buf), "banlist players", 1,
+	if (get_one_status(fd, buf, sizeof(buf), "banlist players", 1,
 			   "There are %u bans", &reply, &bannedplayers))
 		info("Banned players: %u", bannedplayers);
 	else if (streq(reply, "There are no bans"))
 		info("Banned players: 0");
 
-	if (get_one_status(cfg, buf, sizeof(buf), "banlist ips", 1,
+	if (get_one_status(fd, buf, sizeof(buf), "banlist ips", 1,
 			   "There are %u bans", &reply, &bannedips))
 		info("Banned IPs: %u", bannedips);
 	else if (streq(reply, "There are no bans"))
@@ -280,12 +294,13 @@ do_ping(_unused_ struct cfg *cfg) {
 }
 
 void
-do_stop(_unused_ struct cfg *cfg) {
-	assert_die(cfg && cfg->fd >= 0, "invalid arguments");
+do_stop(struct cfg *cfg) {
+	int fd;
 
-	send_login(cfg);
+	assert_die(cfg, "invalid arguments");
 
-	send_cmd(cfg->fd, "stop");
+	fd = rcon_login(cfg);
+	send_cmd(fd, "stop");
 }
 
 void
@@ -294,10 +309,13 @@ do_stop_all(_unused_ struct cfg *cfg) {
 }
 
 void
-do_pcount(_unused_ struct cfg *cfg) {
-	send_login(cfg);
+do_pcount(struct cfg *cfg) {
+	int fd;
+
+	assert_die(cfg, "invalid arguments");
 
-	send_cmd(cfg->fd, "list");
+	fd = rcon_login(cfg);
+	send_cmd(fd, "list");
 }
 
 void
@@ -306,8 +324,11 @@ do_console(struct cfg *cfg)
 	char *prompt;
 	char *cmd;
 	const char *sname;
+	int fd;
 
-	assert_die(cfg && cfg->fd >= 0, "invalid arguments");
+	assert_die(cfg, "invalid arguments");
+
+	fd = rcon_login(cfg);
 
 	if (cfg->server)
 		sname = cfg->server->shortname;
@@ -320,8 +341,6 @@ do_console(struct cfg *cfg)
 			STRLEN(" (") + strlen(sname) + STRLEN("): ") + 1);
 	sprintf(prompt, "%s (%s): ", program_invocation_short_name, sname);
 
-	send_login(cfg);
-
 	while (true) {
 		char *tmp;
 
@@ -340,7 +359,7 @@ do_console(struct cfg *cfg)
 		    streq(tmp, "/q") || streq(tmp, "/quit"))
 			break;
 
-		send_cmd(cfg->fd, tmp);
+		send_cmd(fd, tmp);
 
 		if (streq(tmp, "stop") || streq(tmp, "/stop"))
 			/* The server waits for us to close the connection */
@@ -353,11 +372,12 @@ do_console(struct cfg *cfg)
 }
 
 void
-do_command(_unused_ struct cfg *cfg) {
-	assert_die(cfg && cfg->fd >= 0, "invalid arguments");
+do_command(struct cfg *cfg) {
+	int fd;
 
-	send_login(cfg);
+	assert_die(cfg, "invalid arguments");
 
-	send_cmd(cfg->fd, cfg->cmdstr);
+	fd = rcon_login(cfg);
+	send_cmd(fd, cfg->cmdstr);
 }
 
diff --git a/minecctl/minecctl.c b/minecctl/minecctl.c
index e71a1cb..ed91fdf 100644
--- a/minecctl/minecctl.c
+++ b/minecctl/minecctl.c
@@ -57,9 +57,19 @@ __debug(_unused_ enum debug_lvl lvl, const char *fmt, ...)
 {
 	va_list ap;
 
+	if (use_colors) {
+		if (lvl & DBG_ERROR)
+			fprintf(stderr, ANSI_RED);
+		else if (!(lvl & (DBG_INFO | DBG_VERBOSE)))
+			fprintf(stderr, ANSI_GREY);
+	}
+
 	va_start(ap, fmt);
 	vfprintf(stderr, fmt, ap);
 	va_end(ap);
+
+	if (use_colors && !(lvl & (DBG_INFO | DBG_VERBOSE)))
+		fprintf(stderr, ANSI_NORMAL);
 }
 
 _noreturn_ void
@@ -133,39 +143,12 @@ dump_config()
 	info("cmdstr        : %s", cfg->cmdstr);
 	info("server        : %p", cfg->server);
 	info("cmd           : %p", cfg->cmd);
+	info("force stop    : %s", cfg->force_stop ? "yes" : "no");
 	info("addrs         : %sempty", list_empty(&cfg->addrs) ? "" : "not ");
+	info("mcaddrs       : %sempty", list_empty(&cfg->mcaddrs) ? "" : "not ");
 	info("known_servers : %sempty", list_empty(&cfg->addrs) ? "" : "not ");
 }
 
-static int
-connect_any(struct list_head *addrs)
-{
-	struct saddr *saddr;
-	bool connected = false;
-	int sfd;
-
-	list_for_each_entry(saddr, addrs, list) {
-		sfd = socket(saddr->storage.ss_family, SOCK_STREAM | SOCK_CLOEXEC, 0);
-		if (sfd < 0)
-			die("socket: %m");
-
-		socket_set_low_latency(sfd, true, true, true);
-
-		if (connect(sfd, (struct sockaddr *)&saddr->storage, saddr->addrlen) < 0) {
-			close(sfd);
-			continue;
-		}
-
-		connected = true;
-		break;
-	}
-
-	if (!connected)
-		die("Failed to connect to remote host");
-
-	return sfd;
-}
-
 static void
 parse_server_config(char *buf, const char *filename,
 		    struct list_head *addrs)
@@ -253,7 +236,7 @@ _noreturn_ static void
 usage(bool invalid)
 {
 	if (invalid)
-		info("Invalid option(s)");
+		error("Invalid option(s)");
 
 	info("Usage: %s [OPTION...] COMMAND\n"
 	     "\n"
@@ -262,8 +245,11 @@ usage(bool invalid)
 	     "                        (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"
 	     "\n"
@@ -400,8 +386,8 @@ set_server(const char *name)
 	usage(true);
 }
 
-static char *
-prompt_password()
+char *
+ask_password()
 {
 	struct termios old, new;
 	char *password = NULL;
@@ -440,13 +426,49 @@ prompt_password()
 	return password;
 }
 
+int
+connect_any(struct list_head *addrs, bool may_fail)
+{
+	struct saddr *saddr;
+	bool connected = false;
+	int sfd;
+
+	if (list_empty(addrs))
+		die("No address to connect to");
+
+	list_for_each_entry(saddr, addrs, list) {
+		sfd = socket(saddr->storage.ss_family, SOCK_STREAM | SOCK_CLOEXEC, 0);
+		if (sfd < 0)
+			die("socket: %m");
+
+		socket_set_low_latency(sfd, true, true, true);
+
+		if (connect(sfd, (struct sockaddr *)&saddr->storage, saddr->addrlen) < 0) {
+			close(sfd);
+			continue;
+		}
+
+		connected = true;
+		break;
+	}
+
+	if (!connected) {
+		if (may_fail)
+			sfd = -1;
+		else
+			die("Failed to connect to remote host");
+	}
+
+	return sfd;
+}
+
 static void
 parse_verb(int index, int remain, char **argv)
 {
 	enum command_args args;
 	enum commands cmd;
 
-	assert_die(index >= 0 && remain >= 0 && argv && cfg, "invalid arguments");
+	assert_die(index >= 0 && remain >= 0 && argv, "invalid arguments");
 
 	cmd = CMD_INVALID;
 
@@ -609,13 +631,15 @@ parse_cmdline(int argc, char **argv)
 		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  }
 		};
 
-		c = getopt_long(argc, argv, ":p:a:c:vh",
+		c = getopt_long(argc, argv, ":p:a:A:c:fvh",
 				long_options, &option_index);
 
 		if (c == -1)
@@ -628,9 +652,15 @@ parse_cmdline(int argc, char **argv)
 		case 'a':
 			cfg->addrstr = xstrdup(optarg);
 			break;
+		case 'A':
+			cfg->mcaddrstr = xstrdup(optarg);
+			break;
 		case 'c':
 			cfg->cfgdir = optarg;
 			break;
+		case 'f':
+			cfg->force_stop = true;
+			break;
 		case 'v':
 			debug_mask |= DBG_VERBOSE;
 			break;
@@ -669,8 +699,8 @@ main(int argc, char **argv)
 	set_use_colors();
 
 	cfg = zmalloc(sizeof(*cfg));
-	cfg->fd = -1;
 	list_init(&cfg->addrs);
+	list_init(&cfg->mcaddrs);
 	list_init(&cfg->known_servers);
 
 	parse_cmdline(argc, argv);
@@ -688,8 +718,9 @@ main(int argc, char **argv)
 	} else if (cfg->addrstr) {
 		struct cfg_value value;
 
+		/* FIXME: create a struct server here, fill in details like name */
 		if (!strtosockaddrs(cfg->addrstr, &value, false))
-			die("Unable to connect");
+			die("Unable to parse address: %s", cfg->addrstr);
 
 		if (value.type != CFG_VAL_TYPE_ADDRS)
 			die("Unexpected return value from strtosockaddrs");
@@ -700,18 +731,6 @@ main(int argc, char **argv)
 		list_replace(&value.saddrs, &cfg->addrs);
 	}
 
-	info("here %p", cfg->password);
-	if (!cfg->password)
-		cfg->password = prompt_password();
-	info("here %p", cfg->password);
-
-	dump_config();
-
-	if (list_empty(&cfg->addrs))
-		die("Remote address not found");
-
-	cfg->fd = connect_any(&cfg->addrs);
-
 	cfg->cmd(cfg);
 
 	xfree(cfg);
diff --git a/minecctl/minecctl.h b/minecctl/minecctl.h
index 4918ca0..17dc9c6 100644
--- a/minecctl/minecctl.h
+++ b/minecctl/minecctl.h
@@ -11,15 +11,21 @@ struct cfg {
 	char *password;
 	const char *cfgdir;
 	const char *addrstr;
+	const char *mcaddrstr;
 	char *cmdstr;
 	struct server *server;
 	void (*cmd)(struct cfg *cfg);
+	bool force_stop;
 	struct list_head addrs;
+	struct list_head mcaddrs;
 	struct list_head known_servers;
-	int fd;
 };
 
 extern bool use_colors;
 
+char *ask_password();
+
+int connect_any(struct list_head *addrs, bool may_fail);
+
 #endif
 
diff --git a/shared/config-parser.c b/shared/config-parser.c
index 1f44db4..a8e949d 100644
--- a/shared/config-parser.c
+++ b/shared/config-parser.c
@@ -281,7 +281,6 @@ strtosockaddrs(const char *str, struct cfg_value *rvalue, bool async)
 
 	} else {
 		/* Unknown */
-		error("unable to parse address: %s", str);
 		goto error;
 	}
 
diff --git a/shared/utils.c b/shared/utils.c
index 1284a73..c71f1f7 100644
--- a/shared/utils.c
+++ b/shared/utils.c
@@ -142,7 +142,8 @@ strtou16_strict(const char *str, uint16_t *result)
 	char *end;
 	long val;
 
-	//assert_return(!empty_str(str) && result, -EINVAL);
+	if (empty_str(str))
+		return -EINVAL;
 
 	errno = 0;
 	val = strtol(str, &end, 10);