Drew DeVault: 2 Store and expand positional arguments None 12 files changed, 204 insertions(+), 100 deletions(-)
Over this:
Thanks! To git.sr.ht:~emersion/mrsh 6c1f3c6..8e1b34a master -> master
Copy & paste the following snippet into your terminal to import this patchset into git:
curl -s https://lists.sr.ht/~emersion/mrsh-dev/patches/3449/mbox | git am -3Learn more about email & git
--- 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 *));
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`)
+ _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);
Hmm, I'm not a fan of this name. Could be renamed to `end`?
+ // 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?
--- 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