From 330b393e1604b9a41fb8751b6446f73091c9f85c Mon Sep 17 00:00:00 2001 From: Rickard Andersson Date: Tue, 27 Jun 2023 21:37:10 +0300 Subject: [PATCH 1/4] fix(os): use `setenv` instead of `putenv` `setenv` doesn't copy the value that is put, which means that the previous code had a bug where we free'd the temporary memory and the environment was accidentally cleared right after the function finished. --- core/os/os_linux.odin | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/core/os/os_linux.odin b/core/os/os_linux.odin index e0b60fd36..aaff01165 100644 --- a/core/os/os_linux.odin +++ b/core/os/os_linux.odin @@ -458,6 +458,7 @@ foreign libc { @(link_name="execvp") _unix_execvp :: proc(path: cstring, argv: [^]cstring) -> int --- @(link_name="getenv") _unix_getenv :: proc(cstring) -> cstring --- @(link_name="putenv") _unix_putenv :: proc(cstring) -> c.int --- + @(link_name="setenv") _unix_setenv :: proc(key: cstring, value: cstring, overwrite: c.int) -> c.int --- @(link_name="realpath") _unix_realpath :: proc(path: cstring, resolved_path: rawptr) -> rawptr --- @(link_name="exit") _unix_exit :: proc(status: c.int) -> ! --- @@ -894,7 +895,10 @@ get_env :: proc(key: string, allocator := context.allocator) -> (value: string) set_env :: proc(key, value: string) -> Errno { runtime.DEFAULT_TEMP_ALLOCATOR_TEMP_GUARD() s := strings.concatenate({key, "=", value, "\x00"}, context.temp_allocator) - res := _unix_putenv(strings.unsafe_string_to_cstring(s)) + key_cstring := strings.unsafe_string_to_cstring(strings.concatenate({key, "\x00"}, context.temp_allocator)) + value_cstring := strings.unsafe_string_to_cstring(strings.concatenate({value, "\x00"}, context.temp_allocator)) + // NOTE(GoNZooo): `setenv` instead of `putenv` because it copies both key and value more commonly + res := _unix_setenv(key_cstring, value_cstring, 1) if res < 0 { return Errno(get_last_error()) } From 6ff0ce15e76019c28707a416f60852110b16d4fc Mon Sep 17 00:00:00 2001 From: Rickard Andersson Date: Tue, 27 Jun 2023 21:42:20 +0300 Subject: [PATCH 2/4] cleanup: remove leftover line --- core/os/os_linux.odin | 1 - 1 file changed, 1 deletion(-) diff --git a/core/os/os_linux.odin b/core/os/os_linux.odin index aaff01165..a2c07d98e 100644 --- a/core/os/os_linux.odin +++ b/core/os/os_linux.odin @@ -894,7 +894,6 @@ get_env :: proc(key: string, allocator := context.allocator) -> (value: string) set_env :: proc(key, value: string) -> Errno { runtime.DEFAULT_TEMP_ALLOCATOR_TEMP_GUARD() - s := strings.concatenate({key, "=", value, "\x00"}, context.temp_allocator) key_cstring := strings.unsafe_string_to_cstring(strings.concatenate({key, "\x00"}, context.temp_allocator)) value_cstring := strings.unsafe_string_to_cstring(strings.concatenate({value, "\x00"}, context.temp_allocator)) // NOTE(GoNZooo): `setenv` instead of `putenv` because it copies both key and value more commonly From d03d5d8f038ee5f964edd9e40441b9027c91f180 Mon Sep 17 00:00:00 2001 From: Rickard Andersson Date: Tue, 27 Jun 2023 21:46:00 +0300 Subject: [PATCH 3/4] style: use tabs :[ --- core/os/os_linux.odin | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/os/os_linux.odin b/core/os/os_linux.odin index a2c07d98e..0a3bdbb9e 100644 --- a/core/os/os_linux.odin +++ b/core/os/os_linux.odin @@ -894,9 +894,9 @@ get_env :: proc(key: string, allocator := context.allocator) -> (value: string) set_env :: proc(key, value: string) -> Errno { runtime.DEFAULT_TEMP_ALLOCATOR_TEMP_GUARD() - key_cstring := strings.unsafe_string_to_cstring(strings.concatenate({key, "\x00"}, context.temp_allocator)) - value_cstring := strings.unsafe_string_to_cstring(strings.concatenate({value, "\x00"}, context.temp_allocator)) - // NOTE(GoNZooo): `setenv` instead of `putenv` because it copies both key and value more commonly + key_cstring := strings.unsafe_string_to_cstring(strings.concatenate({key, "\x00"}, context.temp_allocator)) + value_cstring := strings.unsafe_string_to_cstring(strings.concatenate({value, "\x00"}, context.temp_allocator)) + // NOTE(GoNZooo): `setenv` instead of `putenv` because it copies both key and value more commonly res := _unix_setenv(key_cstring, value_cstring, 1) if res < 0 { return Errno(get_last_error()) From f048ad13b52d3c2be2fcd975f1039eb88604360f Mon Sep 17 00:00:00 2001 From: Rickard Andersson Date: Tue, 27 Jun 2023 21:48:53 +0300 Subject: [PATCH 4/4] fix(set_env): use `clone_to_cstring` instead of `unsafe_to_cstring` --- core/os/os_linux.odin | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/os/os_linux.odin b/core/os/os_linux.odin index 0a3bdbb9e..1a4c1fddb 100644 --- a/core/os/os_linux.odin +++ b/core/os/os_linux.odin @@ -894,8 +894,8 @@ get_env :: proc(key: string, allocator := context.allocator) -> (value: string) set_env :: proc(key, value: string) -> Errno { runtime.DEFAULT_TEMP_ALLOCATOR_TEMP_GUARD() - key_cstring := strings.unsafe_string_to_cstring(strings.concatenate({key, "\x00"}, context.temp_allocator)) - value_cstring := strings.unsafe_string_to_cstring(strings.concatenate({value, "\x00"}, context.temp_allocator)) + key_cstring := strings.clone_to_cstring(key, context.temp_allocator) + value_cstring := strings.clone_to_cstring(value, context.temp_allocator) // NOTE(GoNZooo): `setenv` instead of `putenv` because it copies both key and value more commonly res := _unix_setenv(key_cstring, value_cstring, 1) if res < 0 {