From: Andre Noll <maan@tuebingen.mpg.de>
Date: Sat, 4 Apr 2015 19:51:25 +0000 (+0000)
Subject: audiod: Document and fix command handler return values.
X-Git-Tag: v0.5.6~94^2~38
X-Git-Url: http://git.tuebingen.mpg.de/?p=paraslash.git;a=commitdiff_plain;h=cd624494dc77e7283ee87bc474c55b0b1d2c2363;hp=6aea41e619e9ec72167483465ca959715faa8e55

audiod: Document and fix command handler return values.

It has always been the case that a positive return value from a
command handler causes audiod to dump all audiod status items to all
clients. This behaviour is not documented though, and some command
handlers get it wrong. In fact, the help, stat, tasks, grab, version
commands all return positive values although those commands never
change any status items.

This comment documents the meaning of the return value and fixes up
the above command handlers.
---

diff --git a/audiod_command.c b/audiod_command.c
index 68cc15fb..8d80f0cb 100644
--- a/audiod_command.c
+++ b/audiod_command.c
@@ -42,6 +42,12 @@ static audiod_command_handler_t AUDIOD_COMMAND_HANDLERS;
 struct audiod_command {
 	const char *name;
 	/* Pointer to the function that handles the command. */
+	/*
+	 * Command handlers must never never close their file descriptor. A
+	 * positive return value tells audiod that the status items have
+	 * changed. In this case audiod will send an updated version of all
+	 * status items to to each connected stat client.
+	 */
 	audiod_command_handler_t *handler;
 	/* One-line description. */
 	const char *description;
@@ -256,10 +262,11 @@ static int com_help(int fd, int argc, char **argv)
 {
 	int i, ret;
 	char *buf;
-	const char *dflt = "No such command. Available commands:\n";
 
-	if (argc < 2)
-		return dump_commands(fd);
+	if (argc < 2) {
+		ret = dump_commands(fd);
+		goto out;
+	}
 	FOR_EACH_COMMAND(i) {
 		if (strcmp(audiod_cmds[i].name, argv[1]))
 			continue;
@@ -274,12 +281,13 @@ static int com_help(int fd, int argc, char **argv)
 		);
 		ret = client_write(fd, buf);
 		free(buf);
-		return ret;
+		goto out;
 	}
-	ret = client_write(fd, dflt);
+	ret = client_write(fd, "No such command. Available commands:\n");
 	if (ret > 0)
 		ret = dump_commands(fd);
-	return ret;
+out:
+	return ret < 0? ret : 0;
 }
 
 static int com_tasks(int fd, __a_unused int argc, __a_unused char **argv)
@@ -290,7 +298,7 @@ static int com_tasks(int fd, __a_unused int argc, __a_unused char **argv)
 	if (tl)
 		ret = client_write(fd, tl);
 	free(tl);
-	return ret;
+	return ret < 0? ret : 0;
 }
 
 static int com_stat(int fd, int argc, char **argv)
@@ -335,12 +343,13 @@ static int com_stat(int fd, int argc, char **argv)
 	if (ret >= 0)
 		ret = stat_client_add(fd, mask, parser_friendly);
 	free(b.buf);
-	return ret;
+	return ret < 0? ret : 0;
 }
 
 static int com_grab(int fd, int argc, char **argv)
 {
-	return grab_client_new(fd, argc, argv, &sched);
+	int ret = grab_client_new(fd, argc, argv, &sched);
+	return ret < 0? ret : 0;
 }
 
 static int com_term(__a_unused int fd, __a_unused int argc, __a_unused char **argv)
@@ -393,7 +402,7 @@ static int com_version(int fd, int argc, char **argv)
 		msg = make_message("%s\n", version_single_line("audiod"));
 	ret = client_write(fd, msg);
 	free(msg);
-	return ret;
+	return ret < 0? ret : 0;
 }
 
 static int check_perms(uid_t uid, uid_t *whitelist)