ctl-commands: Use slightly more helpful schemas

Let's not abuse the equality sign more than necessary. ;)

src/ctl-client.c

@@ -37,17 +37,17 @@
 #include "table-printer.h"
 
 #define LOG(level, fmt, ...) \
-	fprintf(stderr, "[%s:%d] <" level "> " fmt "\n", __FILE__, __LINE__, \
+	fprintf(stderr, level ": %s: %d: " fmt "\n", __FILE__, __LINE__, \
 		##__VA_ARGS__)
 
-#define WARN(fmt, ...) \
-	LOG("WARNING", fmt, ##__VA_ARGS__)
+#define ERROR(fmt, ...) \
+	LOG("ERROR", fmt, ##__VA_ARGS__)
 
 static bool do_debug = false;
 
 #define DEBUG(fmt, ...) \
 	if (do_debug) \
-	LOG("DEBUG", fmt, ##__VA_ARGS__)
+		LOG("DEBUG", fmt, ##__VA_ARGS__)
 
 static struct cmd_info internal_events[] = {
 	{ .name = "wayvnc-startup",
@@ -85,13 +85,14 @@ struct ctl_client* ctl_client_new(const char* socket_path, void* userdata)
 {
 	if (!socket_path)
 		socket_path = default_ctl_socket_path();
+
 	struct ctl_client* new = calloc(1, sizeof(*new));
 	new->userdata = userdata;
 	new->fd = -1;
 
 	if (strlen(socket_path) >= sizeof(new->addr.sun_path)) {
 		errno = ENAMETOOLONG;
-		WARN("Failed to create unix socket: %m");
+		ERROR("Failed to create unix socket: %m");
 		goto socket_failure;
 	}
 	strcpy(new->addr.sun_path, socket_path);
@@ -108,9 +109,10 @@ static int wait_for_socket(const char* socket_path, int timeout)
 {
 	bool needs_log = true;
 	struct stat sb;
+
 	while (stat(socket_path, &sb) != 0) {
 		if (timeout == 0) {
-			WARN("Failed to find socket path \"%s\": %m",
+			ERROR("Failed to find socket path \"%s\": %m",
 					socket_path);
 			return 1;
 		}
@@ -120,17 +122,19 @@ static int wait_for_socket(const char* socket_path, int timeout)
 					socket_path);
 		}
 		if (usleep(50000) == -1) {
-			WARN("Failed to wait for socket path: %m");
+			ERROR("Failed to wait for socket path: %m");
 			return -1;
 		}
 	}
+
 	if (S_ISSOCK(sb.st_mode)) {
 		DEBUG("Found socket \"%s\"", socket_path);
 	} else {
-		WARN("Path \"%s\" exists but is not a socket (0x%x)",
+		ERROR("Path \"%s\" exists but is not a socket (0x%x)",
 				socket_path, sb.st_mode);
 		return -1;
 	}
+
 	return 0;
 }
 
@@ -138,23 +142,27 @@ static int try_connect(struct ctl_client* self, int timeout)
 {
 	if (self->fd != -1)
 		close(self->fd);
+
 	self->fd = socket(AF_UNIX, SOCK_STREAM, 0);
 	if (self->fd < 0) {
-		WARN("Failed to create unix socket: %m");
+		ERROR("Failed to create unix socket: %m");
 		return 1;
 	}
+
 	while (connect(self->fd, (struct sockaddr*)&self->addr,
 				sizeof(self->addr)) != 0) {
 		if (timeout == 0 || errno != ENOENT) {
-			WARN("Failed to connect to unix socket \"%s\": %m",
+			ERROR("Failed to connect to unix socket \"%s\": %m",
 					self->addr.sun_path);
 			return 1;
 		}
+
 		if (usleep(50000) == -1) {
-			WARN("Failed to wait for connect to succeed: %m");
+			ERROR("Failed to wait for connect to succeed: %m");
 			return 1;
 		}
 	}
+
 	return 0;
 }
 
@@ -189,12 +197,14 @@ static struct jsonipc_request* ctl_client_parse_args(struct ctl_client* self,
 	struct jsonipc_request* request = NULL;
 	json_t* params = json_object();
 	struct cmd_info* info = ctl_command_by_type(*cmd);
+
 	if (option_parser_get_value(options, "help")) {
 		json_object_set_new(params, "command", json_string(info->name));
 		*cmd = CMD_HELP;
 		info = ctl_command_by_type(*cmd);
 		goto out;
 	}
+
 	for (int i = 0; info->params[i].name != NULL; ++i) {
 		const char* key = info->params[i].name;
 		const char* value = option_parser_get_value(options, key);
@@ -202,6 +212,7 @@ static struct jsonipc_request* ctl_client_parse_args(struct ctl_client* self,
 			continue;
 		json_object_set_new(params, key, json_string(value));
 	}
+
 out:
 	request = jsonipc_request_new(info->name, params);
 	json_decref(params);
@@ -215,20 +226,22 @@ static json_t* json_from_buffer(struct ctl_client* self)
 		errno = ENODATA;
 		return NULL;
 	}
+
 	json_error_t err;
 	json_t* root = json_loadb(self->read_buffer, self->read_len, 0, &err);
 	if (root) {
-		advance_read_buffer(&self->read_buffer, &self->read_len, err.position);
+		advance_read_buffer(&self->read_buffer, &self->read_len,
+				err.position);
 	} else if (json_error_code(&err) == json_error_premature_end_of_input) {
 		if (self->read_len == sizeof(self->read_buffer)) {
-			WARN("Response message is too long");
+			ERROR("Response message is too long");
 			errno = EMSGSIZE;
 		} else {
 			DEBUG("Awaiting more data");
 			errno = ENODATA;
 		}
 	} else {
-		WARN("Json parsing failed: %s", err.text);
+		ERROR("Json parsing failed: %s", err.text);
 		errno = EINVAL;
 	}
 	return root;
@@ -241,36 +254,43 @@ static json_t* read_one_object(struct ctl_client* self, int timeout_ms)
 		return root;
 	if (errno != ENODATA)
 		return NULL;
+
 	struct pollfd pfd = {
 		.fd = self->fd,
 		.events = POLLIN,
 		.revents = 0
 	};
-	while (root == NULL) {
+
+	while (!root) {
 		int n = poll(&pfd, 1, timeout_ms);
 		if (n == -1) {
 			if (errno == EINTR && self->wait_for_events)
 				continue;
-			WARN("Error waiting for a response: %m");
+			ERROR("Error waiting for a response: %m");
 			break;
 		} else if (n == 0) {
-			WARN("Timeout waiting for a response");
+			ERROR("Timeout waiting for a response");
 			break;
 		}
+
 		char* readptr = self->read_buffer + self->read_len;
 		size_t remainder = sizeof(self->read_buffer) - self->read_len;
+
 		n = recv(self->fd, readptr, remainder, 0);
 		if (n == -1) {
-			WARN("Read failed: %m");
+			ERROR("Read failed: %m");
 			break;
 		} else if (n == 0) {
-			WARN("Disconnected");
+			ERROR("Disconnected");
 			errno = ECONNRESET;
 			break;
 		}
+
 		DEBUG("Read %d bytes", n);
 		DEBUG("<< %.*s", n, readptr);
+
 		self->read_len += n;
+
 		root = json_from_buffer(self);
 		if (!root && errno != ENODATA)
 			break;
@@ -284,14 +304,17 @@ static struct jsonipc_response* ctl_client_wait_for_response(struct ctl_client*
 	json_t* root = read_one_object(self, 1000);
 	if (!root)
 		return NULL;
+
 	struct jsonipc_error jipc_err = JSONIPC_ERR_INIT;
+
 	struct jsonipc_response* response = jsonipc_response_parse_new(root,
 			&jipc_err);
 	if (!response) {
 		char* msg = json_dumps(jipc_err.data, JSON_EMBED);
-		WARN("Could not parse json: %s", msg);
+		ERROR("Could not parse json: %s", msg);
 		free(msg);
 	}
+
 	json_decref(root);
 	jsonipc_error_cleanup(&jipc_err);
 	return response;
@@ -299,9 +322,10 @@ static struct jsonipc_response* ctl_client_wait_for_response(struct ctl_client*
 
 static void print_error(struct jsonipc_response* response, const char* method)
 {
-	printf("Error (%d)", response->code);
+	printf("ERROR: Failed to execute command: %s", method);
 	if (!response->data)
 		goto out;
+
 	json_t* data = response->data;
 	if (json_is_string(data))
 		printf(": %s", json_string_value(data));
@@ -310,6 +334,7 @@ static void print_error(struct jsonipc_response* response, const char* method)
 		printf(": %s", json_string_value(json_object_get(data, "error")));
 	else
 		json_dumpf(response->data, stdout, JSON_INDENT(2));
+
 out:
 	printf("\n");
 }
@@ -325,29 +350,26 @@ static void pretty_version(json_t* data)
 
 static void pretty_client_list(json_t* data)
 {
-	int n = json_array_size(data);
-	printf("There %s %d VNC client%s connected%s\n", (n == 1) ? "is" : "are",
-			n, (n == 1) ? "" : "s", (n > 0) ? ":" : ".");
 	size_t i;
 	json_t* value;
 	json_array_foreach(data, i, value) {
 		char* id = NULL;
 		char* hostname = NULL;
 		char* username = NULL;
+
 		json_unpack(value, "{s:s, s?s, s?s}", "id", &id, "hostname",
 				&hostname, "username", &username);
-		printf("  client[%s]: ", id);
+		printf("  %s: ", id);
+
 		if (username)
 			printf("%s@", username);
+
 		printf("%s\n", hostname ? hostname : "<unknown>");
 	}
 }
 
 static void pretty_output_list(json_t* data)
 {
-	int n = json_array_size(data);
-	printf("There %s %d output%s%s\n", (n == 1) ? "is" : "are",
-			n, (n == 1) ? "" : "s", (n > 0) ? ":" : ".");
 	size_t i;
 	json_t* value;
 	json_array_foreach(data, i, value) {
@@ -356,16 +378,18 @@ static void pretty_output_list(json_t* data)
 		int height = -1;
 		int width = -1;
 		int captured = false;
+
 		json_unpack(value, "{s:s, s:s, s:i, s:i, s:b}", "name", &name, 
 				"description", &description,
 				"height", &height,
 				"width", &width,
 				"captured", &captured);
-		printf("%s output[%s]: %s (%dx%d)\n",
+		printf("%s %s: \"%s\" (%dx%d)\n",
 				captured ? "*" : " ", name, description, width,
 				height);
 	}
 }
+
 static void pretty_print(json_t* data,
 		struct jsonipc_request* request)
 {
@@ -405,6 +429,7 @@ static int ctl_client_print_response(struct ctl_client* self,
 		struct jsonipc_response* response)
 {
 	DEBUG("Response code: %d", response->code);
+
 	if (response->data) {
 		if (self->flags & CTL_CLIENT_PRINT_JSON)
 			print_compact_json(response->data);
@@ -413,10 +438,12 @@ static int ctl_client_print_response(struct ctl_client* self,
 		else
 			print_error(response, request->method);
 	}
+
 	return response->code;
 }
 
 static struct ctl_client* sig_target = NULL;
+
 static void stop_loop(int signal)
 {
 	sig_target->wait_for_events = false;
@@ -441,9 +468,11 @@ static bool json_has_content(json_t* root)
 {
 	if (!root)
 		return false;
+
 	size_t i;
 	const char* key;
 	json_t* value;
+
 	switch (json_typeof(root)) {
 	case JSON_NULL:
 		return false;
@@ -468,39 +497,34 @@ static bool json_has_content(json_t* root)
 	return false;
 }
 
-static void print_as_yaml(json_t* data, int level, bool needs_leading_newline)
+static void print_for_human(json_t* data, int level)
 {
 	size_t i;
 	const char* key;
 	json_t* value;
-	bool needs_indent = needs_leading_newline;
+
 	switch(json_typeof(data)) {
 	case JSON_NULL:
 		printf("<null>\n");
 		break;
 	case JSON_OBJECT:
-		if (json_object_size(data) > 0 && needs_leading_newline)
-				printf("\n");
 		json_object_foreach(data, key, value) {
 			if (!json_has_content(value))
 				continue;
-			if (needs_indent)
-				print_indent(level);
-			else
-				needs_indent = true;
+
+			print_indent(level);
 			printf("%s: ", key);
-			print_as_yaml(value, level + 1, true);
+			print_for_human(value, level + 1);
 		}
 		break;
 	case JSON_ARRAY:
-		if (json_array_size(data) > 0 && needs_leading_newline)
-			printf("\n");
 		json_array_foreach(data, i, value) {
 			if (!json_has_content(value))
 				continue;
+
 			print_indent(level);
 			printf("- ");
-			print_as_yaml(value, level + 1, json_is_array(value));
+			print_for_human(value, level + 1);
 		}
 		break;
 	case JSON_STRING:
@@ -526,11 +550,10 @@ static void print_event(struct jsonipc_request* event, unsigned flags)
 	if (flags & CTL_CLIENT_PRINT_JSON) {
 		print_compact_json(event->json);
 	} else {
-		printf("\n%s:", event->method);
+		printf("%s:\n", event->method);
 		if (event->params)
-			print_as_yaml(event->params, 1, true);
-		else
-			printf("<<null>\n");
+			print_for_human(event->params, 1);
+		printf("\n");
 	}
 	fflush(stdout);
 }
@@ -559,13 +582,15 @@ static ssize_t ctl_client_send_request(struct ctl_client* self,
 	json_error_t err;
 	json_t* packed = jsonipc_request_pack(request, &err);
 	if (!packed) {
-		WARN("Could not encode json: %s", err.text);
+		ERROR("Could not encode json: %s", err.text);
 		return -1;
 	}
+
 	char buffer[512];
 	int len = json_dumpb(packed, buffer, sizeof(buffer), JSON_COMPACT);
 	json_decref(packed);
 	DEBUG(">> %.*s", len, buffer);
+
 	return send(self->fd, buffer, len, MSG_NOSIGNAL);
 }
 
@@ -589,6 +614,7 @@ static int ctl_client_register_for_events(struct ctl_client* self,
 	jsonipc_response_destroy(response);
 	if (result == 0)
 		send_startup_event(self);
+
 	return result;
 }
 
@@ -597,6 +623,7 @@ static int ctl_client_reconnect_event_loop(struct ctl_client* self,
 {
 	if (ctl_client_connect(self, timeout) != 0)
 		return -1;
+
 	return ctl_client_register_for_events(self, request);
 }
 
@@ -628,6 +655,7 @@ static int ctl_client_event_loop(struct ctl_client* self,
 		print_event(event, self->flags);
 		jsonipc_request_destroy(event);
 	}
+
 	return 0;
 }
 
@@ -638,6 +666,7 @@ static int ctl_client_print_single_command(struct ctl_client* self,
 			request);
 	if (!response)
 		return 1;
+
 	int result = ctl_client_print_response(self, request, response);
 	jsonipc_response_destroy(response);
 	return result;
@@ -652,6 +681,7 @@ void ctl_client_print_command_list(FILE* stream)
 			continue;
 		max_namelen = MAX(max_namelen, strlen(ctl_command_list[i].name));
 	}
+
 	struct table_printer printer;
 	table_printer_init(&printer, stdout, max_namelen);
 	for (size_t i = 0; i < CMD_LIST_LEN; ++i) {
@@ -660,6 +690,7 @@ void ctl_client_print_command_list(FILE* stream)
 		table_printer_print_line(&printer, ctl_command_list[i].name,
 				ctl_command_list[i].description);
 	}
+
 	fprintf(stream, "\nRun 'wayvncctl command-name --help' for command-specific details.\n");
 }
 
@@ -683,7 +714,8 @@ static void print_event_info(const struct cmd_info* info)
 		for (int i = 0; info->params[i].name != NULL; ++i)
 			table_printer_print_fmtline(&printer,
 					info->params[i].description,
-					"%s=%s", info->params[i].name, info->params[i].schema);
+					"%s: %s", info->params[i].name,
+					info->params[i].schema);
 		printf("\n");
 	}
 }
@@ -695,13 +727,15 @@ static int print_event_details(const char* evt_name)
 		print_event_info(info);
 		return 0;
 	}
+
 	for (size_t i = 0; i < INTERNAL_EVT_LEN; ++i) {
 		if (strcmp(evt_name, internal_events[i].name) == 0) {
 			print_event_info(&internal_events[i]);
 			return 0;
 		}
 	}
-	WARN("No such event \"%s\"\n", evt_name);
+
+	ERROR("No such event \"%s\"\n", evt_name);
 	return 1;
 }
 
@@ -720,6 +754,7 @@ void ctl_client_print_event_list(FILE* stream)
 	for (size_t i = 0; i < EVT_LIST_LEN; ++i)
 		table_printer_print_line(&printer, ctl_event_list[i].name,
 				ctl_event_list[i].description);
+
 	for (size_t i = 0; i < INTERNAL_EVT_LEN; ++i)
 		table_printer_print_line(&printer, internal_events[i].name,
 				internal_events[i].description);
@@ -731,20 +766,23 @@ static int print_command_usage(struct ctl_client* self,
 		struct option_parser* parent_options)
 {
 	if (self->flags & CTL_CLIENT_PRINT_JSON) {
-		WARN("JSON output is not supported for \"help\" output");
+		ERROR("JSON output is not supported for \"help\" output");
 		return 1;
 	}
+
 	struct cmd_info* info = ctl_command_by_type(cmd);
 	if (!info) {
-		WARN("No such command");
+		ERROR("No such command");
 		return 1;
 	}
+
 	printf("Usage: wayvncctl [options] %s", info->name);
 	option_parser_print_usage(cmd_options, stdout);
 	printf("\n");
 	option_parser_print_cmd_summary(info->description, stdout);
 	if (option_parser_print_arguments(cmd_options, stdout))
 		printf("\n");
+
 	option_parser_print_options(cmd_options, stdout);
 	printf("\n");
 	option_parser_print_options(parent_options, stdout);
@@ -753,6 +791,7 @@ static int print_command_usage(struct ctl_client* self,
 		ctl_client_print_event_list(stdout);
 		printf("\n");
 	}
+
 	return 0;
 }
 
@@ -772,8 +811,9 @@ int ctl_client_init_cmd_parser(struct option_parser* parser, enum cmd_type cmd)
 	size_t alloc_count = param_count + 2;
 	if (cmd == CMD_EVENT_RECEIVE)
 		alloc_count++;
-	struct wv_option* options = calloc(alloc_count,
-			sizeof(struct wv_option));
+
+	struct wv_option* options = calloc(alloc_count, sizeof(*options));
+
 	size_t i = 0;
 	for (; i < param_count; ++i) {
 		struct wv_option* option = &options[i];
@@ -787,17 +827,20 @@ int ctl_client_init_cmd_parser(struct option_parser* parser, enum cmd_type cmd)
 			option->schema = param->schema;
 		}
 	}
+
 	if (cmd == CMD_EVENT_RECEIVE) {
 		options[i].long_opt = "show";
 		options[i].schema = "<event-name>";
 		options[i].help = "Display details about the given event";
 		i++;
 	}
+
 	options[i].long_opt = "help";
 	options[i].short_opt = 'h';
 	options[i].help = "Display this help text";
 	option_parser_init(parser, options);
 	parser->name = "Parameters";
+
 	return 0;
 }
 
@@ -816,7 +859,7 @@ int ctl_client_run_command(struct ctl_client* self,
 	const char* method = option_parser_get_value(parent_options, "command");
 	enum cmd_type cmd = ctl_command_parse_name(method);
 	if (cmd == CMD_UNKNOWN || cmd == CMD_HELP) {
-		WARN("No such command \"%s\"\n", method);
+		ERROR("No such command \"%s\"\n", method);
 		return 1;
 	}
 
@@ -833,6 +876,7 @@ int ctl_client_run_command(struct ctl_client* self,
 				&cmd_options, parent_options);
 		goto help_printed;
 	}
+
 	if (cmd == CMD_EVENT_RECEIVE && option_parser_get_value(&cmd_options, "show")) {
 		result = print_event_details(option_parser_get_value(&cmd_options, "show"));
 		goto help_printed;

src/ctl-commands.c

@@ -50,7 +50,7 @@ struct cmd_info ctl_command_list[] = {
 		{
 			{ "id",
 				"The ID of the client to disconnect",
-				"<id>", true },
+				"<integer>", true },
 			{},
 		}
 	},
@@ -67,7 +67,7 @@ struct cmd_info ctl_command_list[] = {
 		{
 			{ "output-name",
 				"The specific output name to capture",
-				"<name>", true },
+				"<string>", true },
 			{},
 		}
 	},
@@ -80,34 +80,34 @@ struct cmd_info ctl_command_list[] = {
 #define CLIENT_EVENT_PARAMS(including) \
 	{ "id", \
 		"A unique identifier for this client", \
-		"<id>" }, \
+		"<integer>" }, \
 	{ "connection_count", \
 		"The total number of connected VNC clients " including " this one.", \
-		"<count>" }, \
+		"<integer>" }, \
 	{ "hostname", \
 		"The hostname or IP address of this client (may be null)", \
 		"<name|ip>" }, \
 	{ "username", \
 		"The username used to authentice this client (may be null).", \
-		"<name>" }, \
+		"<string>" }, \
 	{},
 
 struct cmd_info ctl_event_list[] = {
 	[EVT_CAPTURE_CHANGED] = {"capture-changed",
-		"Sent by wayvnc when the catured output is changed",
+		"Sent by wayvnc when the captured output is changed",
 		{
 			{ "output-name",
 				"The name of the output now being captured",
-				"<name>" },
+				"<string>" },
 			{},
 		},
 	},
 	[EVT_CLIENT_CONNECTED] = {"client-connected",
-		"Sent by wayvnc when a new vnc client connects",
+		"Sent by wayvnc when a new VNC client connects",
 		{ CLIENT_EVENT_PARAMS("including") }
 	},
 	[EVT_CLIENT_DISCONNECTED] = {"client-disconnected",
-		"Sent by waynvc when a vnc client disconnects",
+		"Sent by waynvc when a VNC client disconnects",
 		{ CLIENT_EVENT_PARAMS("not including") }
 	},
 };

src/ctl-server.c

@@ -138,8 +138,7 @@ static struct cmd_set_output* cmd_set_output_new(json_t* args,
 {
 	const char* target = NULL;
 	if (json_unpack(args, "{s:s}", "output-name", &target) == -1) {
-		jsonipc_error_printf(err, EINVAL,
-				"required: \"output-name\"");
+		jsonipc_error_printf(err, EINVAL, "Missing output name");
 		return NULL;
 	}
 	struct cmd_set_output* cmd = calloc(1, sizeof(*cmd));
@@ -152,8 +151,7 @@ static struct cmd_disconnect_client* cmd_disconnect_client_new(json_t* args,
 {
 	const char* id = NULL;
 	if (json_unpack(args, "{s:s}", "id", &id) == -1) {
-		jsonipc_error_printf(err, EINVAL,
-				"required: \"id\"");
+		jsonipc_error_printf(err, EINVAL, "Missing client id");
 		return NULL;
 	}
 	struct cmd_disconnect_client* cmd = calloc(1, sizeof(*cmd));
@@ -202,7 +200,6 @@ static struct cmd* parse_command(struct jsonipc_request* ipc,
 	case CMD_OUTPUT_LIST:
 	case CMD_OUTPUT_CYCLE:
 	case CMD_WAYVNC_EXIT:
-		cmd = calloc(1, sizeof(*cmd));
 		break;
 	case CMD_UNKNOWN:
 		jsonipc_error_set_new(err, ENOENT,
@@ -211,9 +208,10 @@ static struct cmd* parse_command(struct jsonipc_request* ipc,
 					jprintf("Unknown command \"%s\"",
 						ipc->method),
 					"commands", list_allowed_commands()));
-		return NULL;
+		break;
 	}
-	cmd->type = cmd_type;
+	if (cmd)
+		cmd->type = cmd_type;
 	return cmd;
 }