[PATCH hare] os: add setenv and unsetenv

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch

2 2

[PATCH hare] os: add setenv and unsetenv

Ember Sawady <ecs@d2evs.net>

Details

Message ID: <20230606161213.30583-1-ecs@d2evs.net>
DKIM signature: missing

Download raw message

1 year, 4 months ago

Patch: +78 -84

Signed-off-by: Ember Sawady <ecs@d2evs.net>
---
wanted this for module3, have also wanted it a fair bit in the past and
have been confused about it not existing. also deduped the
platform-independent env handling while i was here
 os/+freebsd/dirfdfs.ha                        |  1 -
 .../{environ.ha => platform_environ.ha}       | 39 ++------------
 os/+linux/dirfdfs.ha                          |  1 -
 os/+linux/{environ.ha => platform_environ.ha} | 39 ++------------
 os/environ.ha                                 | 52 +++++++++++++++++++
 scripts/gen-stdlib                            | 10 ++--
 stdlib.mk                                     | 20 ++++---
 7 files changed, 78 insertions(+), 84 deletions(-)
 rename os/+freebsd/{environ.ha => platform_environ.ha} (72%)
 rename os/+linux/{environ.ha => platform_environ.ha} (71%)
 create mode 100644 os/environ.ha

diff --git a/os/+freebsd/dirfdfs.ha b/os/+freebsd/dirfdfs.ha
index 13903709..50c41c83 100644
--- a/os/+freebsd/dirfdfs.ha
+++ b/os/+freebsd/dirfdfs.ha
@@ -2,7 +2,6 @@
 // (c) 2021-2022 Drew DeVault <sir@cmpwn.com>
 // (c) 2021 Ember Sawady <ecs@d2evs.net>
 // (c) 2022 Jon Eskin <eskinjp@gmail.com>
-use bytes;
 use errors;
 use encoding::utf8;
 use fs;
diff --git a/os/+freebsd/environ.ha b/os/+freebsd/platform_environ.ha
similarity index 72%
rename from os/+freebsd/environ.ha
rename to os/+freebsd/platform_environ.ha
index 9dd81a94..931a49b2 100644
--- a/os/+freebsd/environ.ha
+++ b/os/+freebsd/platform_environ.ha
@@ -1,7 +1,6 @@
 // License: MPL-2.0
 // (c) 2021 Drew DeVault <sir@cmpwn.com>
 // (c) 2021 Ember Sawady <ecs@d2evs.net>
-use bytes;
 use errors;
 use rt;
 use strings;
@@ -15,7 +14,7 @@ export let args: []str = [];
 // syscall if we don't need it.
 let args_static: [32]str = [""...];
 
-@init fn init_environ() void = {
+@init fn args() void = {
 	if (rt::argc < len(args_static)) {
 		args = args_static[..rt::argc];
 		for (let i = 0z; i < rt::argc; i += 1) {
@@ -30,50 +29,20 @@ let args_static: [32]str = [""...];
 
 };
 
-@fini fn fini_environ() void = {
+@fini fn args() void = {
 	if (rt::argc >= len(args_static)) {
 		free(args);
 	};
-	free(envp);
 };
 
-// Looks up an environment variable and returns its value, or void if unset.
-export fn getenv(name: const str) (str | void) = {
-	const name_b = strings::toutf8(name);
-	for (let i = 0z; rt::envp[i] != null; i += 1) {
-		const item = rt::envp[i]: *[*]u8;
-		const ln = c::strlen(item: *c::char);
-		const eq: size = match (bytes::index(item[..ln], '=')) {
-		case void =>
-			abort("Environment violates System-V invariants");
-		case let i: size =>
-			yield i;
-		};
-		if (bytes::equal(name_b, item[..eq])) {
-			const ln = c::strlen(item: *const c::char);
-			return strings::fromutf8(item[eq+1..ln])!;
-		};
-	};
-};
-
-// Looks up an environment variable and returns its value, or a default value if
-// unset.
-export fn tryenv(name: const str, default: str) str = match (getenv(name)) {
-case let s: str =>
-	yield s;
-case void =>
-	yield default;
-};
-
-let envp: []str = [];
-
 // Returns a slice of the environment strings in the form KEY=VALUE.
 export fn getenvs() []str = {
 	if (len(envp) != 0) {
 		return envp;
 	};
 	for (let i = 0z; rt::envp[i] != null; i += 1) {
-		append(envp, c::tostr(rt::envp[i]: *const c::char)!);
+		let s = c::tostr(rt::envp[i]: *const c::char)!;
+		append(envp, strings::dup(s));
 	};
 	return envp;
 };
diff --git a/os/+linux/dirfdfs.ha b/os/+linux/dirfdfs.ha
index c7fcd451..bd658100 100644
--- a/os/+linux/dirfdfs.ha
+++ b/os/+linux/dirfdfs.ha
@@ -4,7 +4,6 @@
 // (c) 2021-2022 Drew DeVault <sir@cmpwn.com>
 // (c) 2021 Ember Sawady <ecs@d2evs.net>
 // (c) 2022 Jon Eskin <eskinjp@gmail.com>
-use bytes;
 use errors;
 use encoding::utf8;
 use fs;
diff --git a/os/+linux/environ.ha b/os/+linux/platform_environ.ha
similarity index 71%
rename from os/+linux/environ.ha
rename to os/+linux/platform_environ.ha
index 6acd0507..c14fb914 100644
--- a/os/+linux/environ.ha
+++ b/os/+linux/platform_environ.ha
@@ -1,7 +1,6 @@
 // License: MPL-2.0
 // (c) 2021 Drew DeVault <sir@cmpwn.com>
 // (c) 2021 Ember Sawady <ecs@d2evs.net>
-use bytes;
 use errors;
 use math;
 use rt;
@@ -16,7 +15,7 @@ export let args: []str = [];
 // syscall if we don't need it.
 let args_static: [32]str = [""...];
 
-@init fn init_environ() void = {
+@init fn args() void = {
 	if (rt::argc < len(args_static)) {
 		args = args_static[..rt::argc];
 		for (let i = 0z; i < rt::argc; i += 1) {
@@ -31,50 +30,20 @@ let args_static: [32]str = [""...];
 
 };
 
-@fini fn fini_environ() void = {
+@fini fn args() void = {
 	if (rt::argc >= len(args_static)) {
 		free(args);
 	};
-	free(envp);
 };
 
-// Looks up an environment variable and returns its value, or void if unset.
-export fn getenv(name: const str) (str | void) = {
-	const name_b = strings::toutf8(name);
-	for (let i = 0z; rt::envp[i] != null; i += 1) {
-		const item = rt::envp[i]: *[*]u8;
-		const ln = c::strlen(item: *c::char);
-		const eq: size = match (bytes::index(item[..ln], '=')) {
-		case void =>
-			abort("Environment violates System-V invariants");
-		case let i: size =>
-			yield i;
-		};
-		if (bytes::equal(name_b, item[..eq])) {
-			const ln = c::strlen(item: *const c::char);
-			return strings::fromutf8(item[eq+1..ln])!;
-		};
-	};
-};
-
-// Looks up an environment variable and returns its value, or a default value if
-// unset.
-export fn tryenv(name: const str, default: str) str = match (getenv(name)) {
-case let s: str =>
-	yield s;
-case void =>
-	yield default;
-};
-
-let envp: []str = [];
-
 // Returns a slice of the environment strings in the form KEY=VALUE.
 export fn getenvs() []str = {
 	if (len(envp) != 0) {
 		return envp;
 	};
 	for (let i = 0z; rt::envp[i] != null; i += 1) {
-		append(envp, c::tostr(rt::envp[i]: *const c::char)!);
+		let s = c::tostr(rt::envp[i]: *const c::char)!;
+		append(envp, strings::dup(s));
 	};
 	return envp;
 };
diff --git a/os/environ.ha b/os/environ.ha
new file mode 100644
index 00000000..45765aa7
--- /dev/null
+++ b/os/environ.ha
@@ -0,0 +1,52 @@
+use errors;
+use strings;
+
+let envp: []str = [];
+
+@fini fn envp() void = strings::freeall(envp);
+
+// Looks up an environment variable and returns its value, or void if unset.
+export fn getenv(name: const str) (str | void) = {
+	getenvs(); // populate envp
+	const name_b = strings::toutf8(name);
+	let full = strings::concat(name, "=");
+	defer free(full);
+	for (let i = 0z; i < len(envp); i += 1) {
+		if (strings::hasprefix(envp[i], full)) {
+			return strings::cut(envp[i], "=").1;
+		};
+	};
+	return void;
+};
+
+// Looks up an environment variable and returns its value, or a default value if
+// unset.
+export fn tryenv(name: const str, default: str) str = match (getenv(name)) {
+case let s: str =>
+	yield s;
+case void =>
+	yield default;
+};
+
+// Sets an environment variable. The name may not contain '=' or '\0'.
+export fn setenv(name: const str, value: const str) (void | errors::invalid) = {
+	unsetenv(name)?;
+	append(envp, strings::join("=", name, value));
+};
+
+// Unsets an environment variable. The name may not contain '=' or '\0'.
+export fn unsetenv(name: const str) (void | errors::invalid) = {
+	if (strings::contains(name, '=', '\0')) return errors::invalid;
+	getenvs(); // populate envp
+
+	let full = strings::concat(name, "=");
+	defer free(full);
+	for (let i = 0z; i < len(envp); i += 1) {
+		if (strings::hasprefix(envp[i], full)) {
+			free(envp[i]);
+			delete(envp[i]);
+			break;
+		};
+	};
+};
+
diff --git a/scripts/gen-stdlib b/scripts/gen-stdlib
index 65d626b8..6665731c 100755
--- a/scripts/gen-stdlib
+++ b/scripts/gen-stdlib
@@ -1165,25 +1165,27 @@ os() {
 
 	gen_srcs -plinux os \
 		+linux/dirfdfs.ha \
-		+linux/environ.ha \
+		+linux/platform_environ.ha \
 		+linux/$exit \
 		+linux/fs.ha \
 		+linux/memory.ha \
 		+linux/status.ha \
 		+linux/stdfd.ha \
+		environ.ha \
 		os.ha
-	gen_ssa -plinux os io strings fs encoding::utf8 bytes bufio \
+	gen_ssa -plinux os io strings fs encoding::utf8 bufio \
 		errors math types::c
 
 	gen_srcs -pfreebsd os \
-		+freebsd/environ.ha \
+		+freebsd/platform_environ.ha \
 		+freebsd/$exit \
 		+freebsd/dirfdfs.ha \
 		+freebsd/status.ha \
 		+freebsd/stdfd.ha \
 		+freebsd/fs.ha \
+		environ.ha \
 		os.ha
-	gen_ssa -pfreebsd os io strings fs encoding::utf8 bytes bufio \
+	gen_ssa -pfreebsd os io strings fs encoding::utf8 bufio \
 		errors types::c
 }
 
diff --git a/stdlib.mk b/stdlib.mk
index f4854924..955ad750 100644
--- a/stdlib.mk
+++ b/stdlib.mk
@@ -1881,15 +1881,16 @@ $(HARECACHE)/net/uri/net_uri-any.ssa: $(stdlib_net_uri_any_srcs) $(stdlib_rt) $(
 # os (+linux)
 stdlib_os_linux_srcs = \
 	$(STDLIB)/os/+linux/dirfdfs.ha \
-	$(STDLIB)/os/+linux/environ.ha \
+	$(STDLIB)/os/+linux/platform_environ.ha \
 	$(STDLIB)/os/+linux/exit.ha \
 	$(STDLIB)/os/+linux/fs.ha \
 	$(STDLIB)/os/+linux/memory.ha \
 	$(STDLIB)/os/+linux/status.ha \
 	$(STDLIB)/os/+linux/stdfd.ha \
+	$(STDLIB)/os/environ.ha \
 	$(STDLIB)/os/os.ha
 
-$(HARECACHE)/os/os-linux.ssa: $(stdlib_os_linux_srcs) $(stdlib_rt) $(stdlib_io_$(PLATFORM)) $(stdlib_strings_$(PLATFORM)) $(stdlib_fs_$(PLATFORM)) $(stdlib_encoding_utf8_$(PLATFORM)) $(stdlib_bytes_$(PLATFORM)) $(stdlib_bufio_$(PLATFORM)) $(stdlib_errors_$(PLATFORM)) $(stdlib_math_$(PLATFORM)) $(stdlib_types_c_$(PLATFORM))
+$(HARECACHE)/os/os-linux.ssa: $(stdlib_os_linux_srcs) $(stdlib_rt) $(stdlib_io_$(PLATFORM)) $(stdlib_strings_$(PLATFORM)) $(stdlib_fs_$(PLATFORM)) $(stdlib_encoding_utf8_$(PLATFORM)) $(stdlib_bufio_$(PLATFORM)) $(stdlib_errors_$(PLATFORM)) $(stdlib_math_$(PLATFORM)) $(stdlib_types_c_$(PLATFORM))
 	@printf 'HAREC \t$@\n'
 	@mkdir -p $(HARECACHE)/os
 	@$(stdlib_env) $(HAREC) $(HAREFLAGS) -o $@ -Nos \
@@ -1897,15 +1898,16 @@ $(HARECACHE)/os/os-linux.ssa: $(stdlib_os_linux_srcs) $(stdlib_rt) $(stdlib_io_$
 
 # os (+freebsd)
 stdlib_os_freebsd_srcs = \
-	$(STDLIB)/os/+freebsd/environ.ha \
+	$(STDLIB)/os/+freebsd/platform_environ.ha \
 	$(STDLIB)/os/+freebsd/exit.ha \
 	$(STDLIB)/os/+freebsd/dirfdfs.ha \
 	$(STDLIB)/os/+freebsd/status.ha \
 	$(STDLIB)/os/+freebsd/stdfd.ha \
 	$(STDLIB)/os/+freebsd/fs.ha \
+	$(STDLIB)/os/environ.ha \
 	$(STDLIB)/os/os.ha
 
-$(HARECACHE)/os/os-freebsd.ssa: $(stdlib_os_freebsd_srcs) $(stdlib_rt) $(stdlib_io_$(PLATFORM)) $(stdlib_strings_$(PLATFORM)) $(stdlib_fs_$(PLATFORM)) $(stdlib_encoding_utf8_$(PLATFORM)) $(stdlib_bytes_$(PLATFORM)) $(stdlib_bufio_$(PLATFORM)) $(stdlib_errors_$(PLATFORM)) $(stdlib_types_c_$(PLATFORM))
+$(HARECACHE)/os/os-freebsd.ssa: $(stdlib_os_freebsd_srcs) $(stdlib_rt) $(stdlib_io_$(PLATFORM)) $(stdlib_strings_$(PLATFORM)) $(stdlib_fs_$(PLATFORM)) $(stdlib_encoding_utf8_$(PLATFORM)) $(stdlib_bufio_$(PLATFORM)) $(stdlib_errors_$(PLATFORM)) $(stdlib_types_c_$(PLATFORM))
 	@printf 'HAREC \t$@\n'
 	@mkdir -p $(HARECACHE)/os
 	@$(stdlib_env) $(HAREC) $(HAREFLAGS) -o $@ -Nos \
@@ -4334,15 +4336,16 @@ $(TESTCACHE)/net/uri/net_uri-any.ssa: $(testlib_net_uri_any_srcs) $(testlib_rt)
 # os (+linux)
 testlib_os_linux_srcs = \
 	$(STDLIB)/os/+linux/dirfdfs.ha \
-	$(STDLIB)/os/+linux/environ.ha \
+	$(STDLIB)/os/+linux/platform_environ.ha \
 	$(STDLIB)/os/+linux/exit+test.ha \
 	$(STDLIB)/os/+linux/fs.ha \
 	$(STDLIB)/os/+linux/memory.ha \
 	$(STDLIB)/os/+linux/status.ha \
 	$(STDLIB)/os/+linux/stdfd.ha \
+	$(STDLIB)/os/environ.ha \
 	$(STDLIB)/os/os.ha
 
-$(TESTCACHE)/os/os-linux.ssa: $(testlib_os_linux_srcs) $(testlib_rt) $(testlib_io_$(PLATFORM)) $(testlib_strings_$(PLATFORM)) $(testlib_fs_$(PLATFORM)) $(testlib_encoding_utf8_$(PLATFORM)) $(testlib_bytes_$(PLATFORM)) $(testlib_bufio_$(PLATFORM)) $(testlib_errors_$(PLATFORM)) $(testlib_math_$(PLATFORM)) $(testlib_types_c_$(PLATFORM))
+$(TESTCACHE)/os/os-linux.ssa: $(testlib_os_linux_srcs) $(testlib_rt) $(testlib_io_$(PLATFORM)) $(testlib_strings_$(PLATFORM)) $(testlib_fs_$(PLATFORM)) $(testlib_encoding_utf8_$(PLATFORM)) $(testlib_bufio_$(PLATFORM)) $(testlib_errors_$(PLATFORM)) $(testlib_math_$(PLATFORM)) $(testlib_types_c_$(PLATFORM))
 	@printf 'HAREC \t$@\n'
 	@mkdir -p $(TESTCACHE)/os
 	@$(testlib_env) $(HAREC) $(TESTHAREFLAGS) -o $@ -Nos \
@@ -4350,15 +4353,16 @@ $(TESTCACHE)/os/os-linux.ssa: $(testlib_os_linux_srcs) $(testlib_rt) $(testlib_i
 
 # os (+freebsd)
 testlib_os_freebsd_srcs = \
-	$(STDLIB)/os/+freebsd/environ.ha \
+	$(STDLIB)/os/+freebsd/platform_environ.ha \
 	$(STDLIB)/os/+freebsd/exit+test.ha \
 	$(STDLIB)/os/+freebsd/dirfdfs.ha \
 	$(STDLIB)/os/+freebsd/status.ha \
 	$(STDLIB)/os/+freebsd/stdfd.ha \
 	$(STDLIB)/os/+freebsd/fs.ha \
+	$(STDLIB)/os/environ.ha \
 	$(STDLIB)/os/os.ha
 
-$(TESTCACHE)/os/os-freebsd.ssa: $(testlib_os_freebsd_srcs) $(testlib_rt) $(testlib_io_$(PLATFORM)) $(testlib_strings_$(PLATFORM)) $(testlib_fs_$(PLATFORM)) $(testlib_encoding_utf8_$(PLATFORM)) $(testlib_bytes_$(PLATFORM)) $(testlib_bufio_$(PLATFORM)) $(testlib_errors_$(PLATFORM)) $(testlib_types_c_$(PLATFORM))
+$(TESTCACHE)/os/os-freebsd.ssa: $(testlib_os_freebsd_srcs) $(testlib_rt) $(testlib_io_$(PLATFORM)) $(testlib_strings_$(PLATFORM)) $(testlib_fs_$(PLATFORM)) $(testlib_encoding_utf8_$(PLATFORM)) $(testlib_bufio_$(PLATFORM)) $(testlib_errors_$(PLATFORM)) $(testlib_types_c_$(PLATFORM))
 	@printf 'HAREC \t$@\n'
 	@mkdir -p $(TESTCACHE)/os
 	@$(testlib_env) $(HAREC) $(TESTHAREFLAGS) -o $@ -Nos \
-- 
2.41.0

[hare/patches] build failed

builds.sr.ht <builds@sr.ht>

Details

Message ID: <CT5P8JMGP6YZ.3GOU08SNQMQO3@cirno2>
In-Reply-To: <20230606161213.30583-1-ecs@d2evs.net> (view parent)
DKIM signature: missing

Download raw message

1 year, 4 months ago

hare/patches: FAILED in 1m37s

[os: add setenv and unsetenv][0] from [Ember Sawady][1]

[0]: https://lists.sr.ht/~sircmpwn/hare-dev/patches/41685
[1]: ecs@d2evs.net

✓ #1002657 SUCCESS hare/patches/freebsd.yml https://builds.sr.ht/~sircmpwn/job/1002657
✗ #1002656 FAILED  hare/patches/alpine.yml  https://builds.sr.ht/~sircmpwn/job/1002656

Alexey Yerin <yyp@disroot.org>

Details

Message ID: <CT5RAYRK6UP0.3GTDBFH3Z8XZA@desktop>
In-Reply-To: <20230606161213.30583-1-ecs@d2evs.net> (view parent)
DKIM signature: missing

Download raw message

1 year, 4 months ago

On Tue Jun 6, 2023 at 19:10 MSK, Ember Sawady wrote:
> +// Looks up an environment variable and returns its value, or void if unset.
> +export fn getenv(name: const str) (str | void) = {
> +	getenvs(); // populate envp
> +	const name_b = strings::toutf8(name);
> +	let full = strings::concat(name, "=");
> +	defer free(full);
> +	for (let i = 0z; i < len(envp); i += 1) {
> +		if (strings::hasprefix(envp[i], full)) {
> +			return strings::cut(envp[i], "=").1;
> +		};
> +	};
> +	return void;
> +};

This looks a bit weird to me. I think this would be a bit cleaner given
that you already use strings::cut:

	for (let i = 0z; i < len(envp); i += 1) {
		const (key, value) = strings::cut(envp[i], "=");
		if (key == name) {
			return value;
		};
	};

And nit: return void is not necessary.

> +// Unsets an environment variable. The name may not contain '=' or '\0'.
> +export fn unsetenv(name: const str) (void | errors::invalid) = {
> +	if (strings::contains(name, '=', '\0')) return errors::invalid;
> +	getenvs(); // populate envp
> +
> +	let full = strings::concat(name, "=");
> +	defer free(full);
> +	for (let i = 0z; i < len(envp); i += 1) {
> +		if (strings::hasprefix(envp[i], full)) {
> +			free(envp[i]);
> +			delete(envp[i]);
> +			break;
> +		};
> +	};

This can also use strings::cut.

Sorry.