---
 builtin/set.c        | 84 +++++++++++++++++++++++++++++++++++++-------
 include/builtin.h    |  1 +
 include/mrsh/shell.h |  4 +++
 main.c               | 40 +++------------------
 shell/shell.c        |  1 +
 shell/task_token.c   | 22 ++++++++++--
 6 files changed, 102 insertions(+), 50 deletions(-)

diff --git a/builtin/set.c b/builtin/set.c
index 3d6f873..a0c410d 100644
--- a/builtin/set.c
@@ -1,3 +1,5 @@
+#define _POSIX_C_SOURCE 200809L
+#include <errno.h>
 #include <mrsh/builtin.h>
 #include <mrsh/shell.h>
 #include <stdbool.h>
@@ -50,8 +52,24 @@ static const struct option_map *find_long_option(const char *opt) {
 	return NULL;
 }
 
-int builtin_set(struct mrsh_state *state, int argc, char *argv[]) {
-	if (argc == 1) {
+static char **argv_dup(char *argv_0, int argc, char *argv[]) {
+	char **_argv = malloc((argc + 1) * sizeof(char *));
+	_argv[0] = argv_0;
+	for (int i = 1; i < argc; ++i) {
+		_argv[i] = strdup(argv[i - 1]);
+	}
+	return _argv;
+}
+
+static void argv_free(int argc, char **argv) {
+	for (int i = 0; i < argc; ++i) {
+		free(argv[i]);
+	}
+	free(argv);
+}
+
+int set(struct mrsh_state *state, int argc, char *argv[], bool cmdline) {
+	if (argc == 1 && !cmdline) {
 		// TODO: Print all shell variables
 		return EXIT_FAILURE;
 	}
@@ -61,6 +79,7 @@ int builtin_set(struct mrsh_state *state, int argc, char *argv[]) {
 	for (i = 1; i < argc; ++i) {
 		if (strcmp(argv[i], "--") == 0) {
 			force_positional = true;
+			++i;
 			break;
 		}
 		if (argv[i][0] != '-' && argv[i][0] != '+') {
@@ -71,7 +90,8 @@ int builtin_set(struct mrsh_state *state, int argc, char *argv[]) {
 			return EXIT_FAILURE;
 		}
 		const struct option_map *option;
-		if (argv[i][1] == 'o') {
+		switch (argv[i][1]) {
+		case 'o':
 			if (i + 1 == argc) {
 				fprintf(stderr, set_usage);
 				return EXIT_FAILURE;
@@ -88,25 +108,63 @@ int builtin_set(struct mrsh_state *state, int argc, char *argv[]) {
 			}
 			++i;
 			continue;
-		}
-		for (int j = 1; argv[i][j]; ++j) {
-			option = find_option(argv[i][j]);
-			if (!option) {
+		case 'c':
+			if (!cmdline) {
 				fprintf(stderr, set_usage);
 				return EXIT_FAILURE;
 			}
-			if (argv[i][0] == '-') {
-				state->options |= option->value;
-			} else {
-				state->options &= ~option->value;
+			state->input = fmemopen(argv[i + 1], strlen(argv[i + 1]), "r");
+			++i;
+			if (!state->input) {
+				fprintf(stderr, "fmemopen failed: %s", strerror(errno));
+				return EXIT_FAILURE;
 			}
+			break;
+		case 's':
+			state->input = stdin;
+			break;
+		default:
+			for (int j = 1; argv[i][j]; ++j) {
+				option = find_option(argv[i][j]);
+				if (!option) {
+					fprintf(stderr, set_usage);
+					return EXIT_FAILURE;
+				}
+				if (argv[i][0] == '-') {
+					state->options |= option->value;
+				} else {
+					state->options &= ~option->value;
+				}
+			}
+			break;
 		}
 	}
 
 	if (i != argc || force_positional) {
-		// TODO: Assign remaining arguments to positional parameters, and set $#
-		return EXIT_FAILURE;
+		char *argv_0;
+		if (cmdline) {
+			argv_0 = strdup(argv[i++]);
+			state->input = fopen(argv_0, "r");
+			if (!state->input) {
+				fprintf(stderr, "could not open %s for reading: %s",
+						argv_0, strerror(errno));
+				return EXIT_FAILURE;
+			}
+		} else {
+			argv_0 = strdup(state->argv[0]);
+		}
+		argv_free(state->argc, state->argv);
+		state->argc = argc - i + 1;
+		state->argv = argv_dup(argv_0, state->argc, &argv[i]);
+	} else if (cmdline) {
+		// No args given, but we need to initialize state->argv
+		state->argc = 1;
+		state->argv = argv_dup(strdup(argv[0]), 1, argv);
 	}
 
 	return EXIT_SUCCESS;
 }
+
+int builtin_set(struct mrsh_state *state, int argc, char *argv[]) {
+	return set(state, argc, argv, false);
+}
diff --git a/include/builtin.h b/include/builtin.h
index 3c4a84b..00e7ef7 100644
--- a/include/builtin.h
+++ b/include/builtin.h
@@ -6,6 +6,7 @@ typedef int (*mrsh_builtin_func_t)(struct mrsh_state *state,
 
 int builtin_colon(struct mrsh_state *state, int argc, char *argv[]);
 int builtin_exit(struct mrsh_state *state, int argc, char *argv[]);
+int set(struct mrsh_state *state, int argc, char *argv[], bool cmdline);
 int builtin_set(struct mrsh_state *state, int argc, char *argv[]);
 int builtin_times(struct mrsh_state *state, int argc, char *argv[]);
 
diff --git a/include/mrsh/shell.h b/include/mrsh/shell.h
index fc18467..f946836 100644
--- a/include/mrsh/shell.h
+++ b/include/mrsh/shell.h
@@ -4,6 +4,7 @@
 #include <mrsh/ast.h>
 #include <mrsh/hashtable.h>
 #include <stdint.h>
+#include <stdio.h>
 
 enum mrsh_option {
 	// -a: When this option is on, the export attribute shall be set for each
@@ -59,6 +60,9 @@ enum mrsh_option {
 struct mrsh_state {
 	int exit;
 	uint32_t options; // enum mrsh_option
+	FILE *input;
+	char **argv;
+	int argc;
 	bool interactive;
 	struct mrsh_hashtable variables; // char *
 };
diff --git a/main.c b/main.c
index b4e729a..c5e80ea 100644
--- a/main.c
+++ b/main.c
@@ -7,47 +7,17 @@
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
+#include "builtin.h"
 
 int main(int argc, char *argv[]) {
 	struct mrsh_state state = {0};
 	mrsh_state_init(&state);
 
-	FILE *input = stdin;
-
-	// TODO: argument parsing should be handled by the set builtin
-
-	int opt;
-	while ((opt = getopt(argc, argv, "c:ns")) != -1) {
-		switch (opt) {
-		case 'n':
-			state.options |= MRSH_OPT_NOEXEC;
-			break;
-		case 'c':
-			input = fmemopen(optarg, strlen(optarg), "r");
-			if (!input) {
-				fprintf(stderr, "fmemopen failed: %s", strerror(errno));
-				return EXIT_FAILURE;
-			}
-			break;
-		case 's':
-			input = stdin;
-			break;
-		default:
-			return EXIT_FAILURE;
-		}
-	}
-
-	if (optind < argc) {
-		input = fopen(argv[optind], "r");
-		if (!input) {
-			fprintf(stderr, "could not open %s for reading: %s",
-					argv[optind], strerror(errno));
-			return EXIT_FAILURE;
-		}
-		assert(optind + 1 >= argc && "additional args not yet supported");
+	if (set(&state, argc, argv, true) != EXIT_SUCCESS) {
+		return EXIT_FAILURE;
 	}
 
-	struct mrsh_parser *parser = mrsh_parser_create(input);
+	struct mrsh_parser *parser = mrsh_parser_create(state.input);
 	while (state.exit == -1) {
 		struct mrsh_program *prog = mrsh_parse_line(parser);
 		if (prog == NULL) {
@@ -64,6 +34,6 @@ int main(int argc, char *argv[]) {
 
 	mrsh_parser_destroy(parser);
 	mrsh_state_finish(&state);
-	fclose(input);
+	fclose(state.input);
 	return state.exit;
 }
diff --git a/shell/shell.c b/shell/shell.c
index 7d0bef6..bf871d9 100644
--- a/shell/shell.c
+++ b/shell/shell.c
@@ -7,6 +7,7 @@ void mrsh_state_init(struct mrsh_state *state) {
 	state->exit = -1;
 	state->interactive = isatty(STDIN_FILENO);
 	state->options = state->interactive ? MRSH_OPT_INTERACTIVE : 0;
+	state->input = stdin;
 }
 
 static void state_variable_finish_iterator(const char *key, void *value,
diff --git a/shell/task_token.c b/shell/task_token.c
index 9628a3b..0f244a6 100644
--- a/shell/task_token.c
+++ b/shell/task_token.c
@@ -1,6 +1,7 @@
 #define _POSIX_C_SOURCE 200809L
 #include <assert.h>
 #include <stdlib.h>
+#include <stdio.h>
 #include <string.h>
 
 #include "shell.h"
@@ -16,6 +17,24 @@ static void task_token_swap(struct task_token *tt,
 	*tt->token_ptr = new_token;
 }
 
+static const char *parameter_get_value(struct mrsh_state *state, char *name) {
+	static char value[16];
+	char *_;
+	long lvalue = strtol(name, &_, 10);
+	// Special cases
+	if (strcmp(name, "#") == 0) {
+		sprintf(value, "%d", state->argc);
+		return value;
+	} else if (!_[0]) {
+		if (state->argc < lvalue) {
+			return NULL;
+		}
+		return state->argv[lvalue];
+	}
+	// User-set cases
+	return (const char *)mrsh_hashtable_get(&state->variables, name);
+}
+
 static int task_token_poll(struct task *task, struct context *ctx) {
 	struct task_token *tt = (struct task_token *)task;
 	struct mrsh_token *token = *tt->token_ptr;
@@ -26,8 +45,7 @@ static int task_token_poll(struct task *task, struct context *ctx) {
 	case MRSH_TOKEN_PARAMETER:;
 		struct mrsh_token_parameter *tp = mrsh_token_get_parameter(token);
 		assert(tp != NULL);
-		const char *value =
-			mrsh_hashtable_get(&ctx->state->variables, tp->name);
+		const char *value = parameter_get_value(ctx->state, tp->name);
 		if (value == NULL) {
 			value = "";
 		}
-- 
2.18.0

LGTM overall!

I was about to apply this patch, but it seems like there's a conflict
with my last commit. Can your rebase?

On August 4, 2018 5:02 PM, Drew DeVault <sir@cmpwn.com> wrote:
> -int builtin_set(struct mrsh_state *state, int argc, char *argv[]) {
> -	if (argc == 1) {
> +static char **argv_dup(char *argv_0, int argc, char *argv[]) {

It's not immediately clear what this function is duplicating and what's
not. Is it possible to add `const` to `argv` to make this a little bit
clearer?

Maybe this could also duplicate `argv_0`? (Which could then become
`const`)

> +static const char *parameter_get_value(struct mrsh_state *state, char *name) {
> +	static char value[16];
> +	char *_;

Hmm, I'm not a fan of this name. Could be renamed to `end`?

On 2018-08-04  5:19 PM, Simon Ser wrote:
> > -int builtin_set(struct mrsh_state *state, int argc, char *argv[]) {
> > -	if (argc == 1) {
> > +static char **argv_dup(char *argv_0, int argc, char *argv[]) {
> 
> It's not immediately clear what this function is duplicating and what's
> not. Is it possible to add `const` to `argv` to make this a little bit
> clearer?

Not without making argv const across the whole project. I wouldn't mind
doing this in a separate patch if you want.

> Maybe this could also duplicate `argv_0`? (Which could then become
> `const`)

I don't want it to duplicate argv_0, because of the way the logic flows
at line 156.

Side note: when you trim messages, you should try to make the context
more obvious, e.g prefer this:

>+++ b/builtin/set.c
> -%<-
>+static char **argv_dup(char *argv_0, int argc, char *argv[]) {
>+	char **_argv = malloc((argc + 1) * sizeof(char *));

Over this:

>+static char **argv_dup(char *argv_0, int argc, char *argv[]) {
>+	char **_argv = malloc((argc + 1) * sizeof(char *));

---
 builtin/set.c        | 84 +++++++++++++++++++++++++++++++++++++-------
 include/builtin.h    |  1 +
 include/mrsh/shell.h |  4 +++
 main.c               | 40 +++------------------
 shell/shell.c        |  1 +
 shell/task_token.c   | 22 ++++++++++--
 6 files changed, 102 insertions(+), 50 deletions(-)

diff --git a/builtin/set.c b/builtin/set.c
index 3d6f873..a0c410d 100644
--- a/builtin/set.c
@@ -1,3 +1,5 @@
+#define _POSIX_C_SOURCE 200809L
+#include <errno.h>
 #include <mrsh/builtin.h>
 #include <mrsh/shell.h>
 #include <stdbool.h>
@@ -50,8 +52,24 @@ static const struct option_map *find_long_option(const char *opt) {
 	return NULL;
 }
 
-int builtin_set(struct mrsh_state *state, int argc, char *argv[]) {
-	if (argc == 1) {
+static char **argv_dup(char *argv_0, int argc, char *argv[]) {
+	char **_argv = malloc((argc + 1) * sizeof(char *));
+	_argv[0] = argv_0;
+	for (int i = 1; i < argc; ++i) {
+		_argv[i] = strdup(argv[i - 1]);
+	}
+	return _argv;
+}
+
+static void argv_free(int argc, char **argv) {
+	for (int i = 0; i < argc; ++i) {
+		free(argv[i]);
+	}
+	free(argv);
+}
+
+int set(struct mrsh_state *state, int argc, char *argv[], bool cmdline) {
+	if (argc == 1 && !cmdline) {
 		// TODO: Print all shell variables
 		return EXIT_FAILURE;
 	}
@@ -61,6 +79,7 @@ int builtin_set(struct mrsh_state *state, int argc, char *argv[]) {
 	for (i = 1; i < argc; ++i) {
 		if (strcmp(argv[i], "--") == 0) {
 			force_positional = true;
+			++i;
 			break;
 		}
 		if (argv[i][0] != '-' && argv[i][0] != '+') {
@@ -71,7 +90,8 @@ int builtin_set(struct mrsh_state *state, int argc, char *argv[]) {
 			return EXIT_FAILURE;
 		}
 		const struct option_map *option;
-		if (argv[i][1] == 'o') {
+		switch (argv[i][1]) {
+		case 'o':
 			if (i + 1 == argc) {
 				fprintf(stderr, set_usage);
 				return EXIT_FAILURE;
@@ -88,25 +108,63 @@ int builtin_set(struct mrsh_state *state, int argc, char *argv[]) {
 			}
 			++i;
 			continue;
-		}
-		for (int j = 1; argv[i][j]; ++j) {
-			option = find_option(argv[i][j]);
-			if (!option) {
+		case 'c':
+			if (!cmdline) {
 				fprintf(stderr, set_usage);
 				return EXIT_FAILURE;
 			}
-			if (argv[i][0] == '-') {
-				state->options |= option->value;
-			} else {
-				state->options &= ~option->value;
+			state->input = fmemopen(argv[i + 1], strlen(argv[i + 1]), "r");
+			++i;
+			if (!state->input) {
+				fprintf(stderr, "fmemopen failed: %s", strerror(errno));
+				return EXIT_FAILURE;
 			}
+			break;
+		case 's':
+			state->input = stdin;
+			break;
+		default:
+			for (int j = 1; argv[i][j]; ++j) {
+				option = find_option(argv[i][j]);
+				if (!option) {
+					fprintf(stderr, set_usage);
+					return EXIT_FAILURE;
+				}
+				if (argv[i][0] == '-') {
+					state->options |= option->value;
+				} else {
+					state->options &= ~option->value;
+				}
+			}
+			break;
 		}
 	}
 
 	if (i != argc || force_positional) {
-		// TODO: Assign remaining arguments to positional parameters, and set $#
-		return EXIT_FAILURE;
+		char *argv_0;
+		if (cmdline) {
+			argv_0 = strdup(argv[i++]);
+			state->input = fopen(argv_0, "r");
+			if (!state->input) {
+				fprintf(stderr, "could not open %s for reading: %s",
+						argv_0, strerror(errno));
+				return EXIT_FAILURE;
+			}
+		} else {
+			argv_0 = strdup(state->argv[0]);
+		}
+		argv_free(state->argc, state->argv);
+		state->argc = argc - i + 1;
+		state->argv = argv_dup(argv_0, state->argc, &argv[i]);
+	} else if (cmdline) {
+		// No args given, but we need to initialize state->argv
+		state->argc = 1;
+		state->argv = argv_dup(strdup(argv[0]), 1, argv);
 	}
 
 	return EXIT_SUCCESS;
 }
+
+int builtin_set(struct mrsh_state *state, int argc, char *argv[]) {
+	return set(state, argc, argv, false);
+}
diff --git a/include/builtin.h b/include/builtin.h
index 3c4a84b..00e7ef7 100644
--- a/include/builtin.h
+++ b/include/builtin.h
@@ -6,6 +6,7 @@ typedef int (*mrsh_builtin_func_t)(struct mrsh_state *state,
 
 int builtin_colon(struct mrsh_state *state, int argc, char *argv[]);
 int builtin_exit(struct mrsh_state *state, int argc, char *argv[]);
+int set(struct mrsh_state *state, int argc, char *argv[], bool cmdline);
 int builtin_set(struct mrsh_state *state, int argc, char *argv[]);
 int builtin_times(struct mrsh_state *state, int argc, char *argv[]);
 
diff --git a/include/mrsh/shell.h b/include/mrsh/shell.h
index fc18467..f946836 100644
--- a/include/mrsh/shell.h
+++ b/include/mrsh/shell.h
@@ -4,6 +4,7 @@
 #include <mrsh/ast.h>
 #include <mrsh/hashtable.h>
 #include <stdint.h>
+#include <stdio.h>
 
 enum mrsh_option {
 	// -a: When this option is on, the export attribute shall be set for each
@@ -59,6 +60,9 @@ enum mrsh_option {
 struct mrsh_state {
 	int exit;
 	uint32_t options; // enum mrsh_option
+	FILE *input;
+	char **argv;
+	int argc;
 	bool interactive;
 	struct mrsh_hashtable variables; // char *
 };
diff --git a/main.c b/main.c
index b4e729a..c5e80ea 100644
--- a/main.c
+++ b/main.c
@@ -7,47 +7,17 @@
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
+#include "builtin.h"
 
 int main(int argc, char *argv[]) {
 	struct mrsh_state state = {0};
 	mrsh_state_init(&state);
 
-	FILE *input = stdin;
-
-	// TODO: argument parsing should be handled by the set builtin
-
-	int opt;
-	while ((opt = getopt(argc, argv, "c:ns")) != -1) {
-		switch (opt) {
-		case 'n':
-			state.options |= MRSH_OPT_NOEXEC;
-			break;
-		case 'c':
-			input = fmemopen(optarg, strlen(optarg), "r");
-			if (!input) {
-				fprintf(stderr, "fmemopen failed: %s", strerror(errno));
-				return EXIT_FAILURE;
-			}
-			break;
-		case 's':
-			input = stdin;
-			break;
-		default:
-			return EXIT_FAILURE;
-		}
-	}
-
-	if (optind < argc) {
-		input = fopen(argv[optind], "r");
-		if (!input) {
-			fprintf(stderr, "could not open %s for reading: %s",
-					argv[optind], strerror(errno));
-			return EXIT_FAILURE;
-		}
-		assert(optind + 1 >= argc && "additional args not yet supported");
+	if (set(&state, argc, argv, true) != EXIT_SUCCESS) {
+		return EXIT_FAILURE;
 	}
 
-	struct mrsh_parser *parser = mrsh_parser_create(input);
+	struct mrsh_parser *parser = mrsh_parser_create(state.input);
 	while (state.exit == -1) {
 		struct mrsh_program *prog = mrsh_parse_line(parser);
 		if (prog == NULL) {
@@ -64,6 +34,6 @@ int main(int argc, char *argv[]) {
 
 	mrsh_parser_destroy(parser);
 	mrsh_state_finish(&state);
-	fclose(input);
+	fclose(state.input);
 	return state.exit;
 }
diff --git a/shell/shell.c b/shell/shell.c
index 7d0bef6..bf871d9 100644
--- a/shell/shell.c
+++ b/shell/shell.c
@@ -7,6 +7,7 @@ void mrsh_state_init(struct mrsh_state *state) {
 	state->exit = -1;
 	state->interactive = isatty(STDIN_FILENO);
 	state->options = state->interactive ? MRSH_OPT_INTERACTIVE : 0;
+	state->input = stdin;
 }
 
 static void state_variable_finish_iterator(const char *key, void *value,
diff --git a/shell/task_token.c b/shell/task_token.c
index 1c0e846..052a5c1 100644
--- a/shell/task_token.c
+++ b/shell/task_token.c
@@ -1,6 +1,7 @@
 #define _POSIX_C_SOURCE 200809L
 #include <assert.h>
 #include <stdlib.h>
+#include <stdio.h>
 #include <string.h>
 #include <unistd.h>
 #include <errno.h>
@@ -104,6 +105,24 @@ static bool task_token_command_start(struct task_token *tt,
 	}
 }
 
+static const char *parameter_get_value(struct mrsh_state *state, char *name) {
+	static char value[16];
+	char *end;
+	long lvalue = strtol(name, &end, 10);
+	// Special cases
+	if (strcmp(name, "#") == 0) {
+		sprintf(value, "%d", state->argc);
+		return value;
+	} else if (!end[0]) {
+		if (state->argc < lvalue) {
+			return NULL;
+		}
+		return state->argv[lvalue];
+	}
+	// User-set cases
+	return (const char *)mrsh_hashtable_get(&state->variables, name);
+}
+
 static int task_token_poll(struct task *task, struct context *ctx) {
 	struct task_token *tt = (struct task_token *)task;
 	struct mrsh_token *token = *tt->token_ptr;
@@ -114,8 +133,7 @@ static int task_token_poll(struct task *task, struct context *ctx) {
 	case MRSH_TOKEN_PARAMETER:;
 		struct mrsh_token_parameter *tp = mrsh_token_get_parameter(token);
 		assert(tp != NULL);
-		const char *value =
-			mrsh_hashtable_get(&ctx->state->variables, tp->name);
+		const char *value = parameter_get_value(ctx->state, tp->name);
 		if (value == NULL) {
 			value = "";
 		}
-- 
2.18.0

Thanks!

To git.sr.ht:~emersion/mrsh
   6c1f3c6..8e1b34a  master -> master