From c5f4c471f0f0b60837e5dc8093aa639e59633b29 Mon Sep 17 00:00:00 2001
From: Alberto Bertogli <albertito@blitiri.com.ar>
Date: Thu, 29 Mar 2012 22:57:17 +0100
Subject: [PATCH 08/13] Simplify remote control commands

The remote control commands were specified in many different places, with
different ad-hoc parsing and passing to get them from the user to the remote
control thread.

This patch rewrites how remote control commands get parsed and passed around,
unifiying the parsing in the library itself, with a much more extensible and
reasonable format.

The command line tools are also updated to get the commands directly from the
user, and passing them without any need for parsing. The older flags for
specifying commands are marked as deprecated and will be removed in future
releases.

Signed-off-by: Alberto Bertogli <albertito@blitiri.com.ar>
---
 libfiu/fiu-control.h   |   13 ++++
 libfiu/fiu-rc.c        |  165 +++++++++++++++++++++++++++++++-----------------
 preload/run/fiu-run.in |  101 +++++++++++++++++++----------
 preload/run/run.c      |  125 +++++++-----------------------------
 utils/fiu-ctrl         |  101 ++++++++++++++++++------------
 5 files changed, 272 insertions(+), 233 deletions(-)

diff --git a/libfiu/fiu-control.h b/libfiu/fiu-control.h
index cb2b0a4..7e54ce3 100644
--- a/libfiu/fiu-control.h
+++ b/libfiu/fiu-control.h
@@ -130,6 +130,19 @@ int fiu_disable(const char *name);
  * @returns  0 on success, -1 on errors. */
 int fiu_rc_fifo(const char *basename);
 
+/** Applies a remote control command given via a string.
+ *
+ * The format of the string is not stable and is still subject to change.
+ * At the moment, this function is exported for consumption by the libfiu
+ * utilities.
+ *
+ * @param command:  A zero-terminated string with the command to apply.
+ * @param error:  In case of an error, it will point to a human-readable error
+ *			message.
+ * @returns  0 if success, < 0 otherwise.
+ */
+int fiu_rc_string(const char *cmd, char ** const error);
+
 
 #ifdef __cplusplus
 }
diff --git a/libfiu/fiu-rc.c b/libfiu/fiu-rc.c
index 3a9151b..cd45086 100644
--- a/libfiu/fiu-rc.c
+++ b/libfiu/fiu-rc.c
@@ -24,6 +24,7 @@
 /* Max length of a line containing a control directive */
 #define MAX_LINE 512
 
+
 /*
  * Generic remote control
  */
@@ -64,77 +65,127 @@ static int read_line(int fd, char *buf)
 }
 
 /* Remote control command processing.
+ *
  * Supported commands:
- *  - disable <name>
- *  - enable <name> <failnum> <failinfo> [flags]
- *  - enable_random <name> <failnum> <failinfo> <probability> [flags]
+ *  - disable name=N
+ *  - enable name=N,failnum=F,failinfo=I
+ *  - enable_random <same as enable>,probability=P
+ *  - enable_stack_by_name <same as enable>,func_name=F,pos_in_stack=P
+ *
+ * All enable* commands can also take an additional "onetime" parameter,
+ * indicating that this should only fail once (analogous to the FIU_ONETIME
+ * flag).
  *
- * flags can be one of:
- *  - "one": same as FIU_ONETIME
+ * This function is ugly, but we aim for simplicity and ease to extend for
+ * future commands.
  */
-static int rc_process_cmd(char *cmd)
+int fiu_rc_string(const char *cmd, char ** const error)
 {
-	char *tok, *state;
-	char fp_name[MAX_LINE];
-	int failnum;
-	void *failinfo;
-	float probability;
-	unsigned int flags;
+	char m_cmd[MAX_LINE];
+	char command[MAX_LINE], parameters[MAX_LINE];
 
-	state = NULL;
-
-	tok = strtok_r(cmd, " ", &state);
-	if (tok == NULL)
-		return -1;
+	/* We need a version of cmd we can write to for parsing */
+	strncpy(m_cmd, cmd, MAX_LINE);
 
-	if (strcmp(tok, "disable") == 0) {
-		tok = strtok_r(NULL, " ", &state);
-		if (tok == NULL)
-			return -1;
-		return fiu_disable(tok);
-
-	} else if (strcmp(tok, "enable") == 0
-			|| strcmp(tok, "enable_random") == 0) {
-
-		tok = strtok_r(NULL, " ", &state);
-		if (tok == NULL)
-			return -1;
-		strncpy(fp_name, tok, MAX_LINE);
+	/* Separate command and parameters */
+	{
+		char *tok = NULL, *state = NULL;
 
-		tok = strtok_r(NULL, " ", &state);
-		if (tok == NULL)
+		tok = strtok_r(m_cmd, " \t", &state);
+		if (tok == NULL) {
+			*error = "Cannot get command";
 			return -1;
-		failnum = atoi(tok);
+		}
+		strncpy(command, tok, MAX_LINE);
 
-		tok = strtok_r(NULL, " ", &state);
-		if (tok == NULL)
+		tok = strtok_r(NULL, " \t", &state);
+		if (tok == NULL) {
+			*error = "Cannot get parameters";
 			return -1;
-		failinfo = (void *) strtoul(tok, NULL, 10);
-
-		probability = -1;
-		if (strcmp(tok, "enable_random") == 0) {
-			tok = strtok_r(NULL, " ", &state);
-			if (tok == NULL)
-				return -1;
-			probability = strtod(tok, NULL);
-			if (probability < 0 || probability > 1)
-				return -1;
 		}
+		strncpy(parameters, tok, MAX_LINE);
+	}
 
-		flags = 0;
-		tok = strtok_r(NULL, " ", &state);
-		if (tok != NULL) {
-			if (strcmp(tok, "one") == 0)
+	/* Parsing of parameters.
+	 *
+	 * To simplify the code, we parse the command parameters here. Not all
+	 * commands use all the parameters, but since they're not ambiguous it
+	 * makes it easier to do it this way. */
+	char *fp_name = NULL;
+	int failnum = 1;
+	void *failinfo = NULL;
+	unsigned int flags = 0;
+	double probability = -1;
+	char *func_name = NULL;
+	int func_pos_in_stack = -1;
+
+	{
+		/* Different tokens that we accept as parameters */
+		enum {
+			OPT_NAME = 0,
+			OPT_FAILNUM,
+			OPT_FAILINFO,
+			OPT_PROBABILITY,
+			OPT_FUNC_NAME,
+			OPT_POS_IN_STACK,
+			FLAG_ONETIME,
+		};
+		char * const token[] = {
+			[OPT_NAME] = "name",
+			[OPT_FAILNUM] = "failnum",
+			[OPT_FAILINFO] = "failinfo",
+			[OPT_PROBABILITY] = "probability",
+			[OPT_FUNC_NAME] = "func_name",
+			[OPT_POS_IN_STACK] = "pos_in_stack",
+			[FLAG_ONETIME] = "onetime",
+			NULL
+		};
+
+		char *value;
+		char *opts = parameters;
+		while (*opts != '\0') {
+			switch (getsubopt(&opts, token, &value)) {
+			case OPT_NAME:
+				fp_name = value;
+				break;
+			case OPT_FAILNUM:
+				failnum = atoi(value);
+				break;
+			case OPT_FAILINFO:
+				failinfo = (void *) strtoul(value, NULL, 10);
+				break;
+			case OPT_PROBABILITY:
+				probability = strtod(value, NULL);
+				break;
+			case OPT_FUNC_NAME:
+				func_name = value;
+				break;
+			case OPT_POS_IN_STACK:
+				func_pos_in_stack = atoi(value);
+				break;
+			case FLAG_ONETIME:
 				flags |= FIU_ONETIME;
+				break;
+			default:
+				*error = "Unknown parameter";
+				return -1;
+			}
 		}
+	}
 
-		if (probability >= 0) {
-			return fiu_enable_random(fp_name, failnum, failinfo,
-					probability, flags);
-		} else {
-			return fiu_enable(fp_name, failnum, failinfo, flags);
-		}
+	/* Excecute the command */
+	if (strcmp(command, "disable") == 0) {
+		return fiu_disable(fp_name);
+	} else if (strcmp(command, "enable") == 0) {
+		return fiu_enable(fp_name, failnum, failinfo, flags);
+	} else if (strcmp(command, "enable_random") == 0) {
+		return fiu_enable_random(fp_name, failnum, failinfo,
+				flags, probability);
+	} else if (strcmp(command, "enable_stack_by_name") == 0) {
+		return fiu_enable_stack_by_name(fp_name, failnum, failinfo,
+				flags, func_name, func_pos_in_stack);
 	} else {
+		*error = "Unknown command";
 		return -1;
 	}
 }
@@ -146,12 +197,13 @@ static int rc_do_command(int fdr, int fdw)
 {
 	int len, r, reply_len;
 	char buf[MAX_LINE], reply[MAX_LINE];
+	char *error;
 
 	len = read_line(fdr, buf);
 	if (len <= 0)
 		return len;
 
-	r = rc_process_cmd(buf);
+	r = fiu_rc_string(buf, &error);
 
 	reply_len = snprintf(reply, MAX_LINE, "%d\n", r);
 	r = write(fdw, reply, reply_len);
@@ -282,4 +334,3 @@ int fiu_rc_fifo(const char *basename)
 	return r;
 }
 
-
diff --git a/preload/run/fiu-run.in b/preload/run/fiu-run.in
index db33777..37ef881 100644
--- a/preload/run/fiu-run.in
+++ b/preload/run/fiu-run.in
@@ -25,27 +25,39 @@ Usage: fiu-run [options] program [arguments]
 
 The following options are supported:
 
-  -e fpname	Enable the given failure point name.
-  -p prob	... with the given probability (defaults to 100%).
-  -u failnum	... and this failnum (must be != 0) (defaults to 1).
-  -i failinfo	... and this failinfo (defaults to 0).
   -x		Use POSIX libfiu preload library, allows simulate failures in
 		the POSIX and C standard library functions.
+  -c command	Run the given libfiu remote control command before executing
+		the program (see below for reference).
   -f ctrlpath	Enable remote control over named pipes with the given path as
 		base name, the process id will be appended (defaults to
 		\"$FIFO_PREFIX\", set to \"\" to disable).
   -l path	Path where to find the libfiu preload libraries, defaults to
 		$PLIBPATH (which is usually correct).
 
-The -p, -u and -i options must come after the -e they affect.
+Remote control commands are of the form 'command param1=value1,param2=value2'.
+Valid commands are:
 
-For example:
+ - 'enable name=NAME'
+     Enables the NAME failure point unconditionally.
+ - 'enable_random name=NAME,probability=P'
+     Enables the NAME failure point with a probability of P.
 
-  fiu-run -x -e 'posix/io/*' -p 25 -e libc/mm/malloc -p 5 ls -l
+All of them can also optionally take 'failnum' and 'failinfo' parameters,
+analogous to the ones taken by the C functions.
 
-will run \"ls -l\" enabling all failure points that begin with 'posix/io/'
-with a 25% of probability to fail, and the failure point libc/mm/malloc with a
-5% of probability to fail.
+The following options existed in the past but are deprecated and WILL BE
+REMOVED in future releases: -e, -p, -u and -i.
+
+
+Example:
+
+  fiu-run -x -c 'enable_random name=posix/io/*,probability=0.25' \\
+             -c 'enable_random name=libc/mm/malloc,probability=0.05' ls -l
+
+Run \"ls -l\" enabling all failure points that begin with 'posix/io/' with a
+25% of probability to fail, and the failure point libc/mm/malloc with a 5% of
+probability to fail.
 "
 
 
@@ -61,31 +73,35 @@ fi
 function opts_reset() {
 	# variables to store what we know so far; after a new name is found
 	# the old one is added to $ENABLE
-	NAME=""
-	PROB=-1
-	FAILNUM=1
-	FAILINFO=0
+	DEP_NAME=""
+	DEP_PROB=-1
+	DEP_FAILNUM=1
+	DEP_FAILINFO=0
+}
+
+function add_deprecated_enable() {
+	if [ "$NAME" == "" ]; then
+		return
+	fi;
+
+	PARAMS="name=$DEP_NAME,failnum=$DEP_FAILNUM,failinfo=$DEP_FAILINFO"
+	if [ $PROB -ge 0 ]; then
+		C="enable_random $PARAMS,probability=$PROB"
+	else
+		C="enable $PARAMS"
+	fi
+
+	ENABLE="$ENABLE
+		$C"
 }
 
 opts_reset;
-while getopts "+e:p:u:i:f:l:xh" opt; do
+while getopts "+c:f:l:xe:p:u:i:h" opt; do
 	case $opt in
-	e)
-		# add the current one, if any
-		if [ "$NAME" != "" ]; then
-			ENABLE="$ENABLE:$NAME,$PROB,$FAILNUM,$FAILINFO"
-			opts_reset;
-		fi
-		NAME="$OPTARG"
-		;;
-	p)
-		PROB="$OPTARG"
-		;;
-	u)
-		FAILNUM="$OPTARG"
-		;;
-	i)
-		FAILINFO="$OPTARG"
+	c)
+		# Note we use the newline as a command separator.
+		ENABLE="$ENABLE
+			$OPTARG"
 		;;
 	f)
 		FIFO_PREFIX="$OPTARG"
@@ -96,6 +112,23 @@ while getopts "+e:p:u:i:f:l:xh" opt; do
 	x)
 		USE_POSIX_PRELOAD=1
 		;;
+
+	# Deprecated options
+	e)
+		add_deprecated_enable
+		opts_reset
+		DEP_NAME="$OPTARG"
+		;;
+	p)
+		DEP_PROB="$OPTARG"
+		;;
+	u)
+		DEP_FAILNUM="$OPTARG"
+		;;
+	i)
+		DEP_FAILINFO="$OPTARG"
+		;;
+
 	h|*)
 		echo "$HELP_MSG"
 		exit 1
@@ -104,10 +137,8 @@ while getopts "+e:p:u:i:f:l:xh" opt; do
 done
 
 # add leftovers
-if [ "$NAME" != "" ]; then
-	ENABLE="$ENABLE:$NAME,$PROB,$FAILNUM,$FAILINFO"
-	opts_reset;
-fi
+add_deprecated_enable
+opts_reset;
 
 # eat the parameters we already processed
 shift $(( $OPTIND - 1 ))
diff --git a/preload/run/run.c b/preload/run/run.c
index 8687b32..74ca75b 100644
--- a/preload/run/run.c
+++ b/preload/run/run.c
@@ -8,123 +8,46 @@
 #include <fiu.h>
 #include <fiu-control.h>
 
-/* Maximum size of parameters to the enable options */
-#define MAX_ENOPT 128
-
-enum fp_type {
-	FP_ALWAYS = 1,
-	FP_PROB = 2,
-};
-
-struct enable_option {
-	char buf[MAX_ENOPT];
-	enum fp_type type;
-	char *name;
-	int failnum;
-	unsigned long failinfo;
-	float probability;
-};
-
-/* Fills the enopt structure taking the values from the given string, which is
- * assumed to be in of the form:
- *
- *   name,probability,failnum,failinfo
- *
- * All fields are optional, except for the name. On error, enopt->name will be
- * set to NULL. Probability is in percent, -1 indicates it should always be
- * enabled. */
-static void parse_enable(const char *s, struct enable_option *enopt)
-{
-	char *tok;
-	char *state;
-
-	enopt->name = NULL;
-	enopt->type = FP_ALWAYS;
-	enopt->failnum = 1;
-	enopt->failinfo = 0;
-	enopt->probability = 1;
-
-	memset(enopt->buf, 0, MAX_ENOPT);
-	strncpy(enopt->buf, s, MAX_ENOPT);
-
-	tok = strtok_r(enopt->buf, ",", &state);
-	if (tok == NULL)
-		return;
-	enopt->name = tok;
-
-	tok = strtok_r(NULL, ",", &state);
-	if (tok == NULL)
-		return;
-	if (atoi(tok) >= 0) {
-		enopt->type = FP_PROB;
-		enopt->probability = atof(tok) / 100.0;
-	}
-
-	tok = strtok_r(NULL, ",", &state);
-	if (tok == NULL)
-		return;
-	enopt->failnum = atoi(tok);
-
-	tok = strtok_r(NULL, ",", &state);
-	if (tok == NULL)
-		return;
-	enopt->failinfo = atol(tok);
-}
 
 static void __attribute__((constructor)) fiu_run_init(void)
 {
-	int r;
-	struct enable_option enopt;
-	char *fiu_enable_env, *fiu_enable_s, *fiu_fifo_env;
-	char *tok, *state;
+	char *fiu_fifo_env, *fiu_enable_env;
 
 	fiu_init(0);
 
 	fiu_fifo_env = getenv("FIU_CTRL_FIFO");
-	if (fiu_fifo_env != NULL && *fiu_fifo_env != '\0') {
+	if (fiu_fifo_env && *fiu_fifo_env != '\0') {
 		if (fiu_rc_fifo(fiu_fifo_env) < 0) {
 			perror("fiu_run_preload: Error opening RC fifo");
 		}
 	}
 
 	fiu_enable_env = getenv("FIU_ENABLE");
-	if (fiu_enable_env == NULL)
-		return;
-
-	/* copy fiu_enable_env to fiu_enable_s so we can strtok() it */
-	fiu_enable_s = malloc(strlen(fiu_enable_env) + 1);
-	strcpy(fiu_enable_s, fiu_enable_env);
-
-	state = NULL;
-	tok = strtok_r(fiu_enable_s, ":", &state);
-	while (tok != NULL) {
-		parse_enable(tok, &enopt);
-		if (enopt.name == NULL) {
-			fprintf(stderr, "fiu-run.so: "
-					"ignoring enable without name");
-			tok = strtok_r(NULL, ":", &state);
-			continue;
+	if (fiu_enable_env && fiu_enable_env != '\0') {
+		/* FIU_ENABLE can contain more than one command, separated by
+		 * a newline, so we split them and call fiu_rc_string()
+		 * accordingly. */
+		char *tok, *state;
+		char *env_copy;
+		char *rc_error;
+
+		env_copy = strdup(fiu_enable_env);
+		if (env_copy == NULL) {
+			perror("fiu_run_preload: Error in strdup()");
+			return;
 		}
 
-		if (enopt.type == FP_ALWAYS) {
-			r = fiu_enable(enopt.name, enopt.failnum,
-					(void *) enopt.failinfo, 0);
-		} else {
-			r = fiu_enable_random(enopt.name, enopt.failnum,
-					(void *) enopt.failinfo, 0,
-					enopt.probability);
+		tok = strtok_r(env_copy, "\n", &state);
+		while (tok) {
+			if (fiu_rc_string(tok, &rc_error) != 0) {
+				fprintf(stderr,
+					"fiu_run_preload: Error applying "
+						"FIU_ENABLE commands: %s\n",
+						rc_error);
+				return;
+			}
+			tok = strtok_r(NULL, "\n", &state);
 		}
-
-		if (r < 0) {
-			fprintf(stderr, "fiu-run.so: "
-					"couldn't enable failure %s\n",
-					enopt.name);
-			continue;
-		}
-
-		tok = strtok_r(NULL, ":", &state);
 	}
-
-	free(fiu_enable_s);
 }
 
diff --git a/utils/fiu-ctrl b/utils/fiu-ctrl
index a08e06d..aad946b 100755
--- a/utils/fiu-ctrl
+++ b/utils/fiu-ctrl
@@ -18,28 +18,43 @@ Usage: fiu-ctrl [options] PID [PID ...]
 
 The following options are supported:
 
-  -e fpname	Enable the given failure point name.
-  -p prob	... with the given probability (defaults to 100%).
-  -u failnum	... and this failnum (must be != 0) (defaults to 1).
-  -i failinfo	... and this failinfo (defaults to 0).
-  -d fpname	Disable the given failure point name.
-  -f ctrlpath	Set the default prefix for remote control over named pipes.
-		(defaults to \"$FIFO_PREFIX\", which is usually correct if
-		the program was run using fiu-run(1)).
+  -c command	Run the given libfiu remote control command before executing
+		the program (see below for reference).
+  -f ctrlpath	Enable remote control over named pipes with the given path as
+		base name, the process id will be appended (defaults to
+		\"$FIFO_PREFIX\", set to \"\" to disable).
+  -l path	Path where to find the libfiu preload libraries, defaults to
+		$PLIBPATH (which is usually correct).
 
-The -p, -u and -i options must come after the -e they affect.
+Remote control commands are of the form 'command param1=value1,param2=value2'.
+Valid commands are:
 
-For example:
+ - 'enable name=NAME'
+     Enables the NAME failure point unconditionally.
+ - 'enable_random name=NAME,probability=P'
+     Enables the NAME failure point with a probability of P.
+ - 'disable name=NAME'
+     Disables the NAME failure point.
 
-  fiu-ctrl -e posix/io/read -p 25 -e libc/mm/malloc -p 5 12345
+All of the enable\* can also optionally take 'failnum' and 'failinfo'
+parameters, analogous to the ones taken by the C functions.
 
-will tell the process with pid 12345 to enable the failure point
-'posix/io/read' with a 25% of probability to fail, and the failure point
-'libc/mm/malloc' with a 5% of probability to fail. And:
+The following options existed in the past but are deprecated and WILL BE
+REMOVED in future releases: -e, -p, -u and -i.
 
-  fiu-ctrl -d posix/io/read 12345
 
-will tell the same process to disable the previously enabled failure point.
+Examples:
+
+  fiu-ctrl -c 'enable_random name=posix/io/*,probability=0.25' \\
+           -c 'enable_random name=libc/mm/malloc,probability=0.05' 12345
+
+Tell the process with pid 12345 to enable the failure point 'posix/io/read'
+with a 25% of probability to fail, and the failure point 'libc/mm/malloc' with
+a 5% of probability to fail.
+
+  fiu-ctrl -c 'disable name=posix/io/read' 12345
+
+Tell the same process to disable the previously enabled failure point.
 
 You can control multiple processes at once by specifiying more than one
 process ID.
@@ -57,48 +72,54 @@ fi
 
 function opts_reset() {
 	# variables to store what we know so far; after a new name is found
-	# the old one is added to $ENABLE
-	NAME=""
-	PROB=-1
-	FAILNUM=1
-	FAILINFO=0
+	# the old one is added to $CMD
+	DEP_NAME=""
+	DEP_PROB=-1
+	DEP_FAILNUM=1
+	DEP_FAILINFO=0
 }
 
-function add_cmd() {
-	if [ "$NAME" != "" ]; then
-		if [ $PROB -ge 0 ]; then
-			C="enable_random $NAME $PROB $FAILNUM $FAILINFO"
-			CMDS[${#CMDS[*]}]="$C"
-		else
-			CMDS[${#CMDS[*]}]="enable $NAME $FAILNUM $FAILINFO"
-		fi
-		opts_reset;
+function add_deprecated_enable() {
+	if [ "$NAME" == "" ]; then
+		return
+	fi;
+
+	PARAMS="name=$DEP_NAME,failnum=$DEP_FAILNUM,failinfo=$DEP_FAILINFO"
+	if [ $PROB -ge 0 ]; then
+		C="enable_random $PARAMS,probability=$PROB"
+	else
+		C="enable $PARAMS"
 	fi
+
+	CMDS[${#CMDS[*]}]="$C"
 }
 
 opts_reset;
-while getopts "+e:p:u:i:d:f:h" opt; do
+while getopts "+c:e:p:u:i:d:f:h" opt; do
 	case $opt in
+	c)
+		CMDS[${#CMDS[*]}]="$OPTARG"
+		;;
 	e)
 		# add the current one, if any
-		add_cmd;
+		add_deprecated_enable;
 		opts_reset;
-		NAME="$OPTARG"
+		DEP_NAME="$OPTARG"
 		;;
 	p)
-		PROB="$OPTARG"
+		DEP_PROB="$OPTARG"
 		;;
 	u)
-		FAILNUM="$OPTARG"
+		DEP_FAILNUM="$OPTARG"
 		;;
 	i)
-		FAILINFO="$OPTARG"
+		DEP_FAILINFO="$OPTARG"
 		;;
 	f)
 		FIFO_PREFIX="$OPTARG"
 		;;
 	d)
-		CMDS[${#CMDS[*]}]="disable $OPTARG"
+		CMDS[${#CMDS[*]}]="disable name=$OPTARG"
 		opts_reset;
 		;;
 	h|*)
@@ -142,9 +163,9 @@ function send_cmd_fifo() {
 	P=$1
 	shift 1
 	echo "$@" > $P.in
-	R="`cat $P.out`"
-	if [ "$R" -eq -1 ]; then
-		echo "$P: Command returned error"
+	REPLY=$(cat "$P.out")
+	if [ "$REPLY" == "-1" ]; then
+		echo "$P: Command '$@' returned error ($REPLY)"
 	fi
 }
 
-- 
1.6.2.2.646.gb214

