[PATCH 1/2] lttng: Fix /proc/ltt parser

The parser for /proc/ltt (*) is borken, at least the describing comment
and the actual implementation diverge and the latter is unhandy. Find a
rework of ltt-marker-control.c below. It implements the following
interface:

connect <marker name> [<probe name> [compact|dynamic [<channel>]]]
disconnect <marker name> [<probe name>]
set_id <marker name> [<probe name>] compact|dynamic
set_channel <marker name> [<probe name>] <channel>

If <probe name> is not given, the default probe is assumed.

The patch also removes a few pointless "inline" attributes. The compiler
should be able to decide about the right way.

Signed-off-by: Jan Kiszka <[email protected]>


(*) Is /proc the right place for this thing at all? Shouldn't this be
debugfs as well?

---
 ltt/ltt-marker-control.c |  128 ++++++++++++++++++++---------------------------
 1 file changed, 57 insertions(+), 71 deletions(-)

Index: b/ltt/ltt-marker-control.c
===================================================================
--- a/ltt/ltt-marker-control.c
+++ b/ltt/ltt-marker-control.c
@@ -87,11 +87,13 @@ static struct id_name_info {
 
 static struct file_operations ltt_fops;
 
-static inline struct ltt_available_probe *get_probe_from_name(const char *pname)
+static struct ltt_available_probe *get_probe_from_name(const char *pname)
 {
 	struct ltt_available_probe *iter;
 	int comparison, found = 0;
 
+	if (!pname)
+		pname = DEFAULT_PROBE;
 	list_for_each_entry(iter, &probes_registered_list, node) {
 		comparison = strcmp(pname, iter->name);
 		if (!comparison)
@@ -105,7 +107,7 @@ static inline struct ltt_available_probe
 		return NULL;
 }
 
-static inline int get_channel_index_from_name(const char *name)
+static int get_channel_index_from_name(const char *name)
 {
 	struct chan_name_info *info;
 
@@ -119,7 +121,7 @@ static inline int get_channel_index_from
 	return -ENOENT;
 }
 
-static inline enum marker_id get_id_from_name(const char *name)
+static enum marker_id get_id_from_name(const char *name)
 {
 	struct id_name_info *info;
 
@@ -132,14 +134,14 @@ static inline enum marker_id get_id_from
 	return -ENOENT;
 }
 
-static inline char *skip_spaces(char *buf)
+static char *skip_spaces(char *buf)
 {
 	while (*buf != '\0' && isspace(*buf))
 		buf++;
 	return buf;
 }
 
-static inline char *skip_nonspaces(char *buf)
+static char *skip_nonspaces(char *buf)
 {
 	while (*buf != '\0' && !isspace(*buf))
 		buf++;
@@ -609,12 +611,12 @@ EXPORT_SYMBOL_GPL(ltt_dump_marker_state)
 /*
  * function handling proc entry write.
  *
- * connect marker_name [probe_name] [id type (compact/dynamic)] [channel]
- * disconnect marker_name
+ * connect <marker name> [<probe name> [compact|dynamic [<channel>]]]
+ * disconnect <marker name> [<probe name>]
  *
  * Actions can be done on a connected marker:
- * set_id marker_name compact/dynamic
- * set_channel marker_name channel
+ * set_id <marker name> [<probe name>] compact|dynamic
+ * set_channel <marker name> [<probe name>] <channel>
  *
  * note : when a core probe registers, it directly establishes the connexion.
  * Cannot be disconnected by users.
@@ -623,11 +625,11 @@ static ssize_t ltt_write(struct file *fi
 			   size_t count, loff_t *offset)
 {
 	char *kbuf;
-	char *iter, *marker_action, *marker_name = NULL, *probe_name = NULL,
-			*id = NULL, *channel = NULL;
+	char *iter, *marker_action, *arg[4];
 	ssize_t ret;
 	int channel_index;
 	enum marker_id n_id;
+	int i;
 
 	if (!count)
 		return -EINVAL;
@@ -644,88 +646,72 @@ static ssize_t ltt_write(struct file *fi
 		ret = -EINVAL;
 		goto end;
 	}
-	if (iter < kbuf + count) {
-		iter++;			/* skip the added '\0' */
-		get_marker_string(iter, &marker_name, &iter);
-		if (marker_name == iter)
-			marker_name = NULL;
-	}
-	if (iter < kbuf + count) {
-		iter++;			/* skip the added '\0' */
-		get_marker_string(iter, &probe_name, &iter);
-		if (probe_name == iter)
-			probe_name = DEFAULT_PROBE;
-	}
-	if (iter < kbuf + count) {
-		iter++;			/* skip the added '\0' */
-		get_marker_string(iter, &id, &iter);
-		if (id == iter)
-			id = NULL;
-	}
-	if (iter < kbuf + count) {
-		iter++;			/* skip the added '\0' */
-		get_marker_string(iter, &channel, &iter);
-		if (channel == iter)
-			channel = NULL;
+	for (i = 0; i < 4; i++) {
+		arg[i] = NULL;
+		if (iter < kbuf + count) {
+			iter++;			/* skip the added '\0' */
+			get_marker_string(iter, &arg[i], &iter);
+			if (arg[i] == iter)
+				arg[i] = NULL;
+		}
 	}
 
-	printk(KERN_DEBUG "Marker control : '%s' '%s' id: %s channel: %s\n",
-			marker_action, marker_name, id, channel);
-
-	if (!marker_name) {
+	if (!arg[0]) {
 		ret = -EINVAL;
 		goto end;
 	}
 
 	if (!strcmp(marker_action, "connect")) {
-		channel_index = get_channel_index_from_name(channel);
-		if (channel_index < 0) {
-			ret = channel_index;
-			goto end;
-		}
-		n_id = get_id_from_name(id);
+		n_id = get_id_from_name(arg[2]);
 		if (n_id < 0) {
 			ret = n_id;
 			goto end;
 		}
-		ret = ltt_marker_connect(marker_name, probe_name, n_id,
-					(uint16_t)channel_index, 1, 1);
+		channel_index = get_channel_index_from_name(arg[3]);
+		if (channel_index < 0) {
+			ret = channel_index;
+			goto end;
+		}
+		ret = ltt_marker_connect(arg[0], arg[1], n_id,
+					 (uint16_t)channel_index, 1, 1);
 		if (ret)
 			goto end;
 	} else if (!strcmp(marker_action, "disconnect")) {
-		ret = ltt_marker_disconnect(marker_name, probe_name, 1);
+		ret = ltt_marker_disconnect(arg[0], arg[1], 1);
 		if (ret)
 			goto end;
-	} else if (!strcmp(marker_action, "set_id")) {
-		if (!id) {
-			ret = -EINVAL;
-			goto end;
-		}
-		n_id = get_id_from_name(id);
-		if (n_id < 0) {
-			ret = n_id;
-			goto end;
+	} else {
+		if (!arg[2]) {
+			arg[2] = arg[1];
+			arg[1] = DEFAULT_PROBE;
 		}
-		ret = do_set_id(marker_name, probe_name, n_id, 1);
-		if (ret)
-			goto end;
-	} else if (!strcmp(marker_action, "set_channel")) {
-		if (!channel) {
+		if (!arg[2]) {
 			ret = -EINVAL;
 			goto end;
 		}
-		channel_index = get_channel_index_from_name(channel);
-		if (channel_index < 0) {
-			ret = channel_index;
+		if (!strcmp(marker_action, "set_id")) {
+			n_id = get_id_from_name(arg[2]);
+			if (n_id < 0) {
+				ret = n_id;
+				goto end;
+			}
+			ret = do_set_id(arg[0], arg[1], n_id, 1);
+			if (ret)
+				goto end;
+		} else if (!strcmp(marker_action, "set_channel")) {
+			channel_index = get_channel_index_from_name(arg[2]);
+			if (channel_index < 0) {
+				ret = channel_index;
+				goto end;
+			}
+			ret = do_set_channel(arg[0], arg[1],
+					     (uint16_t)channel_index, 1);
+			if (ret)
+				goto end;
+		} else {
+			ret = -EINVAL;
 			goto end;
 		}
-		ret = do_set_channel(marker_name, probe_name,
-			(uint16_t)channel_index, 1);
-		if (ret)
-			goto end;
-	} else {
-		ret = -EINVAL;
-		goto end;
 	}
 	ret = count;
 end: