~emersion/mrsh-dev

mrsh_state: rename args field to frame v2 PROPOSED

Drew DeVault: 3
 mrsh_state: rename args field to frame
 mrsh_state: move loop control into call frame
 builtins: implement return

 21 files changed, 136 insertions(+), 67 deletions(-)
The whole series LGTM. Nice work!

To git.sr.ht:~emersion/mrsh
   c7c3feb5d57e..8ed33d40ce16  master -> master

Thanks!
Export patchset (mbox)
How do I use this?

Copy & paste the following snippet into your terminal to import this patchset into git:

curl -s https://lists.sr.ht/~emersion/mrsh-dev/patches/7889/mbox | git am -3
Learn more about email & git
View this thread in the archives

[PATCH v2 1/3] mrsh_state: rename args field to frame Export this patch

---
 builtin/getopts.c           |  4 ++--
 builtin/set.c               | 12 ++++++------
 builtin/shift.c             | 12 ++++++------
 include/mrsh/shell.h        |  6 +++---
 main.c                      |  4 ++--
 shell/shell.c               | 38 ++++++++++++++++++-------------------
 shell/task/simple_command.c |  4 ++--
 shell/task/word.c           | 10 +++++-----
 8 files changed, 45 insertions(+), 45 deletions(-)

diff --git a/builtin/getopts.c b/builtin/getopts.c
index 3832b86..b7b6ad6 100644
--- a/builtin/getopts.c
@@ -29,8 +29,8 @@ int builtin_getopts(struct mrsh_state *state, int argc, char *argv[]) {
		optc = argc - mrsh_optind - 2;
		optv = &argv[mrsh_optind + 2];
	} else {
		optc = state->args->argc;
		optv = state->args->argv;
		optc = state->frame->argc;
		optv = state->frame->argv;
	}
	char *optstring = argv[mrsh_optind];
	char *name = argv[mrsh_optind + 1];
diff --git a/builtin/set.c b/builtin/set.c
index 921b0c1..376c1d1 100644
--- a/builtin/set.c
@@ -191,15 +191,15 @@ static int set(struct mrsh_state *state, struct mrsh_init_args *init_args,

			init_args->command_file = argv_0;
		} else {
			argv_0 = strdup(state->args->argv[0]);
			argv_0 = strdup(state->frame->argv[0]);
		}
		argv_free(state->args->argc, state->args->argv);
		state->args->argc = argc - i + 1;
		state->args->argv = argv_dup(argv_0, state->args->argc, &argv[i]);
		argv_free(state->frame->argc, state->frame->argv);
		state->frame->argc = argc - i + 1;
		state->frame->argv = argv_dup(argv_0, state->frame->argc, &argv[i]);
	} else if (init_args != NULL) {
		// No args given, but we need to initialize state->argv
		state->args->argc = 1;
		state->args->argv = argv_dup(strdup(argv[0]), 1, argv);
		state->frame->argc = 1;
		state->frame->argv = argv_dup(strdup(argv[0]), 1, argv);
	}

	return 0;
diff --git a/builtin/shift.c b/builtin/shift.c
index 8a9ce47..5c582fc 100644
--- a/builtin/shift.c
@@ -33,20 +33,20 @@ int builtin_shift(struct mrsh_state *state, int argc, char *argv[]) {
			state->exit = 1;
		}
		return 1;
	} else if (n > state->args->argc - 1) {
	} else if (n > state->frame->argc - 1) {
		fprintf(stderr, "shift: [n] must be less than $#\n");
		if (!state->interactive) {
			state->exit = 1;
		}
		return 1;
	}
	for (int i = 1, j = n + 1; j < state->args->argc; ++i, ++j) {
		if (j <= state->args->argc - n) {
			state->args->argv[i] = state->args->argv[j];
	for (int i = 1, j = n + 1; j < state->frame->argc; ++i, ++j) {
		if (j <= state->frame->argc - n) {
			state->frame->argv[i] = state->frame->argv[j];
		} else {
			free(state->args->argv[i]);
			free(state->frame->argv[i]);
		}
	}
	state->args->argc -= n;
	state->frame->argc -= n;
	return 0;
}
diff --git a/include/mrsh/shell.h b/include/mrsh/shell.h
index 92cb44f..3d5b560 100644
--- a/include/mrsh/shell.h
+++ b/include/mrsh/shell.h
@@ -94,7 +94,7 @@ struct mrsh_state {
	int exit;
	int fd;
	uint32_t options; // enum mrsh_option
	struct mrsh_call_frame *args;
	struct mrsh_call_frame *frame;
	bool interactive;
	struct mrsh_hashtable variables; // struct mrsh_variable *
	struct mrsh_hashtable aliases; // char *
@@ -127,8 +127,8 @@ void mrsh_env_set(struct mrsh_state *state,
void mrsh_env_unset(struct mrsh_state *state, const char *key);
const char *mrsh_env_get(struct mrsh_state *state,
	const char *key, uint32_t *attribs);
void mrsh_push_args(struct mrsh_state *state, int argc, const char *argv[]);
void mrsh_pop_args(struct mrsh_state *state);
void mrsh_push_frame(struct mrsh_state *state, int argc, const char *argv[]);
void mrsh_pop_frame(struct mrsh_state *state);
int mrsh_run_program(struct mrsh_state *state, struct mrsh_program *prog);
int mrsh_run_word(struct mrsh_state *state, struct mrsh_word **word);
bool mrsh_run_arithm_expr(struct mrsh_arithm_expr *expr, long *result);
diff --git a/main.c b/main.c
index 49847da..15ff946 100644
--- a/main.c
+++ b/main.c
@@ -32,7 +32,7 @@ int main(int argc, char *argv[]) {

	if (!(state.options & MRSH_OPT_NOEXEC)) {
		// If argv[0] begins with `-`, it's a login shell
		if (state.args->argv[0][0] == '-') {
		if (state.frame->argv[0][0] == '-') {
			mrsh_source_profile(&state);
		}
		if (state.interactive) {
@@ -112,7 +112,7 @@ int main(int argc, char *argv[]) {
			if (err_msg != NULL) {
				mrsh_buffer_finish(&read_buffer);
				fprintf(stderr, "%s:%d:%d: syntax error: %s\n",
					state.args->argv[0], err_pos.line, err_pos.column, err_msg);
					state.frame->argv[0], err_pos.line, err_pos.column, err_msg);
				if (state.interactive) {
					continue;
				} else {
diff --git a/shell/shell.c b/shell/shell.c
index a9cfadf..ffb6878 100644
--- a/shell/shell.c
+++ b/shell/shell.c
@@ -24,7 +24,7 @@ void mrsh_state_init(struct mrsh_state *state) {
	state->nloops = 0;
	state->interactive = isatty(STDIN_FILENO);
	state->options = state->interactive ? MRSH_OPT_INTERACTIVE : 0;
	state->args = calloc(1, sizeof(struct mrsh_call_frame));
	state->frame = calloc(1, sizeof(struct mrsh_call_frame));
}

static const char *get_alias(const char *name, void *data) {
@@ -59,12 +59,12 @@ static void state_fn_finish_iterator(const char *key, void *value, void *_) {
	mrsh_function_destroy((struct mrsh_function *)value);
}

static void call_frame_destroy(struct mrsh_call_frame *args) {
	for (int i = 0; i < args->argc; ++i) {
		free(args->argv[i]);
static void call_frame_destroy(struct mrsh_call_frame *frame) {
	for (int i = 0; i < frame->argc; ++i) {
		free(frame->argv[i]);
	}
	free(args->argv);
	free(args);
	free(frame->argv);
	free(frame);
}

void mrsh_state_finish(struct mrsh_state *state) {
@@ -83,11 +83,11 @@ void mrsh_state_finish(struct mrsh_state *state) {
		process_destroy(state->processes.data[state->processes.len - 1]);
	}
	mrsh_array_finish(&state->processes);
	struct mrsh_call_frame *args = state->args;
	while (args) {
		struct mrsh_call_frame *prev = args->prev;
		call_frame_destroy(args);
		args = prev;
	struct mrsh_call_frame *frame = state->frame;
	while (frame) {
		struct mrsh_call_frame *prev = frame->prev;
		call_frame_destroy(frame);
		frame = prev;
	}
}

@@ -117,20 +117,20 @@ const char *mrsh_env_get(struct mrsh_state *state,
	return var ? var->value : NULL;
}

void mrsh_push_args(struct mrsh_state *state, int argc, const char *argv[]) {
void mrsh_push_frame(struct mrsh_state *state, int argc, const char *argv[]) {
	struct mrsh_call_frame *next = calloc(1, sizeof(struct mrsh_call_frame));
	next->argc = argc;
	next->argv = malloc(sizeof(char *) * argc);
	for (int i = 0; i < argc; ++i) {
		next->argv[i] = strdup(argv[i]);
	}
	next->prev = state->args;
	state->args = next;
	next->prev = state->frame;
	state->frame = next;
}

void mrsh_pop_args(struct mrsh_state *state) {
	struct mrsh_call_frame *args = state->args;
	assert(args->prev != NULL);
	state->args = args->prev;
	call_frame_destroy(args);
void mrsh_pop_frame(struct mrsh_state *state) {
	struct mrsh_call_frame *frame = state->frame;
	assert(frame->prev != NULL);
	state->frame = frame->prev;
	call_frame_destroy(frame);
}
diff --git a/shell/task/simple_command.c b/shell/task/simple_command.c
index 30118a8..867f6d6 100644
--- a/shell/task/simple_command.c
+++ b/shell/task/simple_command.c
@@ -329,13 +329,13 @@ int run_simple_command(struct context *ctx, struct mrsh_simple_command *sc) {
	const struct mrsh_function *fn_def =
		mrsh_hashtable_get(&ctx->state->functions, argv_0);
	if (fn_def != NULL) {
		mrsh_push_args(ctx->state, argc, (const char **)argv);
		mrsh_push_frame(ctx->state, argc, (const char **)argv);
		// fn_def may be free'd during run_command when overwritten with another
		// function, so we need to copy it.
		struct mrsh_command *body = mrsh_command_copy(fn_def->body);
		ret = run_command(ctx, body);
		mrsh_command_destroy(body);
		mrsh_pop_args(ctx->state);
		mrsh_pop_frame(ctx->state);
	} else if (mrsh_has_builtin(argv_0)) {
		ret = run_builtin(ctx, sc, argc, argv);
	} else {
diff --git a/shell/task/word.c b/shell/task/word.c
index a81f4c0..37958e1 100644
--- a/shell/task/word.c
+++ b/shell/task/word.c
@@ -105,7 +105,7 @@ static const char *parameter_get_value(struct mrsh_state *state, char *name) {
	} else if (strcmp(name, "*") == 0) {
		// TODO
	} else if (strcmp(name, "#") == 0) {
		sprintf(value, "%d", state->args->argc - 1);
		sprintf(value, "%d", state->frame->argc - 1);
		return value;
	} else if (strcmp(name, "?") == 0) {
		sprintf(value, "%d", state->last_status);
@@ -129,10 +129,10 @@ static const char *parameter_get_value(struct mrsh_state *state, char *name) {
		/* Standard is unclear on what to do in this case, mimic dash */
		return "";
	} else if (end[0] == '\0') {
		if (lvalue >= state->args->argc) {
		if (lvalue >= state->frame->argc) {
			return NULL;
		}
		return state->args->argv[lvalue];
		return state->frame->argv[lvalue];
	}
	// User-set cases
	return mrsh_env_get(state, name, NULL);
@@ -166,7 +166,7 @@ int run_word(struct context *ctx, struct mrsh_word **word_ptr,
		if (value == NULL) {
			if ((ctx->state->options & MRSH_OPT_NOUNSET)) {
				fprintf(stderr, "%s: %s: unbound variable\n",
						ctx->state->args->argv[0], wp->name);
						ctx->state->frame->argv[0], wp->name);
				return TASK_STATUS_ERROR;
			}
			value = "";
@@ -197,7 +197,7 @@ int run_word(struct context *ctx, struct mrsh_word **word_ptr,
			if (err_msg != NULL) {
				// TODO: improve error line/column
				fprintf(stderr, "%s (arithmetic %d:%d): %s\n",
					ctx->state->args->argv[0], err_pos.line,
					ctx->state->frame->argv[0], err_pos.line,
					err_pos.column, err_msg);
			} else {
				fprintf(stderr, "expected an arithmetic expression\n");
-- 
2.23.0

[PATCH v2 2/3] mrsh_state: move loop control into call frame Export this patch

---
 builtin/break.c      |  8 ++++----
 builtin/exit.c       |  2 +-
 include/mrsh/shell.h | 17 +++++++++--------
 shell/shell.c        |  2 +-
 shell/task/task.c    | 16 ++++++++--------
 5 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/builtin/break.c b/builtin/break.c
index f88cd01..dc0a991 100644
--- a/builtin/break.c
@@ -25,12 +25,12 @@ int builtin_break(struct mrsh_state *state, int argc, char *argv[]) {
		}
	}

	if (n > state->nloops) {
		n = state->nloops;
	if (n > state->frame->nloops) {
		n = state->frame->nloops;
	}

	state->nloops -= n - 1;
	state->branch_control =
	state->frame->nloops -= n - 1;
	state->frame->branch_control =
		strcmp(argv[0], "break") == 0 ? MRSH_BRANCH_BREAK : MRSH_BRANCH_CONTINUE;
	return TASK_STATUS_INTERRUPTED;
}
diff --git a/builtin/exit.c b/builtin/exit.c
index 0cd051c..04b2301 100644
--- a/builtin/exit.c
@@ -27,6 +27,6 @@ int builtin_exit(struct mrsh_state *state, int argc, char *argv[]) {
	}

	state->exit = status;
	state->branch_control = MRSH_BRANCH_EXIT;
	state->frame->branch_control = MRSH_BRANCH_EXIT;
	return TASK_STATUS_INTERRUPTED;
}
diff --git a/include/mrsh/shell.h b/include/mrsh/shell.h
index 3d5b560..d5ac51a 100644
--- a/include/mrsh/shell.h
+++ b/include/mrsh/shell.h
@@ -75,12 +75,6 @@ struct mrsh_function {
	struct mrsh_command *body;
};

struct mrsh_call_frame {
	char **argv;
	int argc;
	struct mrsh_call_frame *prev;
};

struct mrsh_job;

enum mrsh_branch_control {
@@ -90,6 +84,15 @@ enum mrsh_branch_control {
	MRSH_BRANCH_EXIT,
};

struct mrsh_call_frame {
	char **argv;
	int argc;
	struct mrsh_call_frame *prev;

	enum mrsh_branch_control branch_control;
	int nloops;
};

struct mrsh_state {
	int exit;
	int fd;
@@ -109,8 +112,6 @@ struct mrsh_state {
	struct mrsh_job *foreground_job;

	// TODO: move this to context
	enum mrsh_branch_control branch_control;
	int nloops;
	bool child; // true if we're not the main shell process
};

diff --git a/shell/shell.c b/shell/shell.c
index ffb6878..e0a3809 100644
--- a/shell/shell.c
+++ b/shell/shell.c
@@ -21,7 +21,6 @@ void mrsh_function_destroy(struct mrsh_function *fn) {
void mrsh_state_init(struct mrsh_state *state) {
	state->exit = -1;
	state->fd = -1;
	state->nloops = 0;
	state->interactive = isatty(STDIN_FILENO);
	state->options = state->interactive ? MRSH_OPT_INTERACTIVE : 0;
	state->frame = calloc(1, sizeof(struct mrsh_call_frame));
@@ -124,6 +123,7 @@ void mrsh_push_frame(struct mrsh_state *state, int argc, const char *argv[]) {
	for (int i = 0; i < argc; ++i) {
		next->argv[i] = strdup(argv[i]);
	}
	next->nloops = 0;
	next->prev = state->frame;
	state->frame = next;
}
diff --git a/shell/task/task.c b/shell/task/task.c
index b0823a3..7639b70 100644
--- a/shell/task/task.c
+++ b/shell/task/task.c
@@ -63,7 +63,7 @@ static int run_if_clause(struct context *ctx, struct mrsh_if_clause *ic) {
}

static int run_loop_clause(struct context *ctx, struct mrsh_loop_clause *lc) {
	int loop_num = ++ctx->state->nloops;
	int loop_num = ++ctx->state->frame->nloops;

	int loop_ret = 0;
	while (ctx->state->exit == -1) {
@@ -97,11 +97,11 @@ static int run_loop_clause(struct context *ctx, struct mrsh_loop_clause *lc) {
		continue;

interrupt:
		if (ctx->state->nloops < loop_num) {
		if (ctx->state->frame->nloops < loop_num) {
			loop_ret = TASK_STATUS_INTERRUPTED; // break to parent loop
			break;
		}
		switch (ctx->state->branch_control) {
		switch (ctx->state->frame->branch_control) {
		case MRSH_BRANCH_BREAK:
		case MRSH_BRANCH_RETURN:
		case MRSH_BRANCH_EXIT:
@@ -116,12 +116,12 @@ interrupt:
		}
	}

	--ctx->state->nloops;
	--ctx->state->frame->nloops;
	return loop_ret;
}

static int run_for_clause(struct context *ctx, struct mrsh_for_clause *fc) {
	int loop_num = ++ctx->state->nloops;
	int loop_num = ++ctx->state->frame->nloops;

	struct mrsh_array word_fields = {0};
	for (size_t i = 0; i < fc->word_list.len; i++) {
@@ -167,12 +167,12 @@ static int run_for_clause(struct context *ctx, struct mrsh_for_clause *fc) {
		continue;

interrupt:
		if (ctx->state->nloops < loop_num) {
		if (ctx->state->frame->nloops < loop_num) {
			loop_ret = TASK_STATUS_INTERRUPTED; // break to parent loop
			break;
		}
		bool break_loop = false;
		switch (ctx->state->branch_control) {
		switch (ctx->state->frame->branch_control) {
		case MRSH_BRANCH_BREAK:
		case MRSH_BRANCH_RETURN:
		case MRSH_BRANCH_EXIT:
@@ -192,7 +192,7 @@ interrupt:
	}
	mrsh_array_finish(&expanded_fields);

	--ctx->state->nloops;
	--ctx->state->frame->nloops;
	return loop_ret;
}

-- 
2.23.0

[PATCH v2 3/3] builtins: implement return Export this patch

---
Fixes a copy/paste error with a variable name.

 Makefile          |  1 +
 builtin/builtin.c |  1 +
 builtin/return.c  | 30 ++++++++++++++++++++++++++++++
 configure         |  1 +
 include/builtin.h |  1 +
 meson.build       |  1 +
 test/meson.build  |  1 +
 test/return.sh    | 32 ++++++++++++++++++++++++++++++++
 8 files changed, 68 insertions(+)
 create mode 100644 builtin/return.c
 create mode 100644 test/return.sh

diff --git a/Makefile b/Makefile
index 1e84e1f..b5db765 100644
--- a/Makefile
+++ b/Makefile
@@ -27,6 +27,7 @@ tests=\
		test/function.sh \
		test/loop.sh \
		test/pipeline.sh \
		test/return.sh \
		test/subshell.sh \
		test/syntax.sh \
		test/ulimit.sh \
diff --git a/builtin/builtin.c b/builtin/builtin.c
index 47907e1..3d010e5 100644
--- a/builtin/builtin.c
@@ -32,6 +32,7 @@ static const struct builtin builtins[] = {
	{ "pwd", builtin_pwd, false },
	{ "read", builtin_read, false },
	{ "readonly", builtin_export, true },
	{ "return", builtin_return, true },
	{ "set", builtin_set, true },
	{ "shift", builtin_shift, true },
	{ "times", builtin_times, true },
diff --git a/builtin/return.c b/builtin/return.c
new file mode 100644
index 0000000..d0436fa
--- /dev/null
@@ -0,0 +1,30 @@
#include <mrsh/builtin.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include "builtin.h"
#include "shell/task.h"

static const char return_usage[] = "usage: %s [n]\n";

int builtin_return(struct mrsh_state *state, int argc, char *argv[]) {
	if (argc > 2) {
		fprintf(stderr, return_usage, argv[0]);
		return 1;
	}

	int n = 0;
	if (argc == 2) {
		char *end;
		n = strtol(argv[1], &end, 10);
		if (end[0] != '\0' || argv[0][0] == '\0' || n < 0 || n > 255) {
			fprintf(stderr, "%s: invalid return number '%s'\n", argv[0], argv[1]);
			return 1;
		}
	}

	state->frame->nloops = 0;
	state->frame->branch_control = MRSH_BRANCH_RETURN;
	state->last_status = n;
	return TASK_STATUS_INTERRUPTED;
}
diff --git a/configure b/configure
index 8039055..9df78c1 100755
--- a/configure
+++ b/configure
@@ -50,6 +50,7 @@ libmrsh() {
		'builtin/jobs.c' \
		'builtin/pwd.c' \
		'builtin/read.c' \
		'builtin/return.c' \
		'builtin/set.c' \
		'builtin/shift.c' \
		'builtin/times.c' \
diff --git a/include/builtin.h b/include/builtin.h
index 5c37f3b..e4d0ecf 100644
--- a/include/builtin.h
+++ b/include/builtin.h
@@ -24,6 +24,7 @@ int builtin_getopts(struct mrsh_state *state, int argc, char *argv[]);
int builtin_jobs(struct mrsh_state *state, int argc, char *argv[]);
int builtin_pwd(struct mrsh_state *state, int argc, char *argv[]);
int builtin_read(struct mrsh_state *state, int argc, char *argv[]);
int builtin_return(struct mrsh_state *state, int argc, char *argv[]);
int builtin_set(struct mrsh_state *state, int argc, char *argv[]);
int builtin_shift(struct mrsh_state *state, int argc, char *argv[]);
int builtin_times(struct mrsh_state *state, int argc, char *argv[]);
diff --git a/meson.build b/meson.build
index 6126521..597ce94 100644
--- a/meson.build
+++ b/meson.build
@@ -89,6 +89,7 @@ lib_mrsh = library(
		'builtin/jobs.c',
		'builtin/pwd.c',
		'builtin/read.c',
		'builtin/return.c',
		'builtin/set.c',
		'builtin/shift.c',
		'builtin/times.c',
diff --git a/test/meson.build b/test/meson.build
index 30eec8e..dc94b9c 100644
--- a/test/meson.build
+++ b/test/meson.build
@@ -12,6 +12,7 @@ test_files = [
	'function.sh',
	'loop.sh',
	'pipeline.sh',
	'return.sh',
	'subshell.sh',
	'syntax.sh',
	'ulimit.sh',
diff --git a/test/return.sh b/test/return.sh
new file mode 100644
index 0000000..8b70fdf
--- /dev/null
+++ b/test/return.sh
@@ -0,0 +1,32 @@
#!/bin/sh -e
func_a() {
	echo func a
	return
	echo func a post-return
}

func_b() {
	echo func b
	return 1
	echo func b post-return
}

func_c() {
	echo func c
	while :
	do
		echo func c loop
		return
		echo func c loop post-return
	done
	echo func c post loop
}

func_a

if func_b
then
	exit 1
fi

func_c
-- 
2.23.0
The whole series LGTM. Nice work!

To git.sr.ht:~emersion/mrsh
   c7c3feb5d57e..8ed33d40ce16  master -> master

Thanks!