authorgravatar for git@vexu.euVeikka Tuominen <git@vexu.eu> 2020-12-28 01:08:12+02:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2020-12-28 01:08:12+02:00
log3fb0288d87b1142bd3d416b0a6df7aa32dd7a943
treec2c6161d39f88360a189c7c3ac8e419a0bbf4432
parentdd86e9d78cedf2c8505d18311e1978b0579e8e67
parent8000262e07d00b996dfcd07ab6c26c5db28f42d9
signaturebadge-question-mark Signed by PGP key 4AEE18F83AFDEB23

Merge pull request #7563 from ifreund/alloc-sent-cleanup

std: add test for createNullDelimitedEnvMap(), clean up sentinel handling for argv/environ

2 files changed, 46 insertions(+), 30 deletions(-)

lib/std/child_process.zig+43-20
......@@ -390,15 +390,8 @@ pub const ChildProcess = struct {
390390 // can fail between fork() and execve().
391391 // Therefore, we do all the allocation for the execve() before the fork().
392392 // This means we must do the null-termination of argv and env vars here.
393 const argv_buf = try arena.alloc(?[*:0]u8, self.argv.len + 1);
394 for (self.argv) |arg, i| {
395 const arg_buf = try arena.alloc(u8, arg.len + 1);
396 @memcpy(arg_buf.ptr, arg.ptr, arg.len);
397 arg_buf[arg.len] = 0;
398 argv_buf[i] = arg_buf[0..arg.len :0].ptr;
399 }
400 argv_buf[self.argv.len] = null;
401 const argv_ptr = argv_buf[0..self.argv.len :null].ptr;
393 const argv_buf = try arena.allocSentinel(?[*:0]u8, self.argv.len, null);
394 for (self.argv) |arg, i| argv_buf[i] = (try arena.dupeZ(u8, arg)).ptr;
402395
403396 const envp = m: {
404397 if (self.env_map) |env_map| {
......@@ -465,8 +458,8 @@ pub const ChildProcess = struct {
465458 }
466459
467460 const err = switch (self.expand_arg0) {
468 .expand => os.execvpeZ_expandArg0(.expand, argv_buf.ptr[0].?, argv_ptr, envp),
469 .no_expand => os.execvpeZ_expandArg0(.no_expand, argv_buf.ptr[0].?, argv_ptr, envp),
461 .expand => os.execvpeZ_expandArg0(.expand, argv_buf.ptr[0].?, argv_buf.ptr, envp),
462 .no_expand => os.execvpeZ_expandArg0(.no_expand, argv_buf.ptr[0].?, argv_buf.ptr, envp),
470463 };
471464 forkChildErrReport(err_pipe[1], err);
472465 }
......@@ -914,21 +907,51 @@ pub fn createWindowsEnvBlock(allocator: *mem.Allocator, env_map: *const BufMap)
914907
915908pub fn createNullDelimitedEnvMap(arena: *mem.Allocator, env_map: *const std.BufMap) ![:null]?[*:0]u8 {
916909 const envp_count = env_map.count();
917 const envp_buf = try arena.alloc(?[*:0]u8, envp_count + 1);
918 mem.set(?[*:0]u8, envp_buf, null);
910 const envp_buf = try arena.allocSentinel(?[*:0]u8, envp_count, null);
919911 {
920912 var it = env_map.iterator();
921913 var i: usize = 0;
922914 while (it.next()) |pair| : (i += 1) {
923 const env_buf = try arena.alloc(u8, pair.key.len + pair.value.len + 2);
924 @memcpy(env_buf.ptr, pair.key.ptr, pair.key.len);
915 const env_buf = try arena.allocSentinel(u8, pair.key.len + pair.value.len + 1, 0);
916 mem.copy(u8, env_buf, pair.key);
925917 env_buf[pair.key.len] = '=';
926 @memcpy(env_buf.ptr + pair.key.len + 1, pair.value.ptr, pair.value.len);
927 const len = env_buf.len - 1;
928 env_buf[len] = 0;
929 envp_buf[i] = env_buf[0..len :0].ptr;
918 mem.copy(u8, env_buf[pair.key.len + 1 ..], pair.value);
919 envp_buf[i] = env_buf.ptr;
930920 }
931921 assert(i == envp_count);
932922 }
933 return envp_buf[0..envp_count :null];
923 return envp_buf;
924}
925
926test "createNullDelimitedEnvMap" {
927 const testing = std.testing;
928 const allocator = testing.allocator;
929 var envmap = BufMap.init(allocator);
930 defer envmap.deinit();
931
932 try envmap.set("HOME", "/home/ifreund");
933 try envmap.set("WAYLAND_DISPLAY", "wayland-1");
934 try envmap.set("DISPLAY", ":1");
935 try envmap.set("DEBUGINFOD_URLS", " ");
936 try envmap.set("XCURSOR_SIZE", "24");
937
938 var arena = std.heap.ArenaAllocator.init(allocator);
939 defer arena.deinit();
940 const environ = try createNullDelimitedEnvMap(&arena.allocator, &envmap);
941
942 testing.expectEqual(@as(usize, 5), environ.len);
943
944 inline for (.{
945 "HOME=/home/ifreund",
946 "WAYLAND_DISPLAY=wayland-1",
947 "DISPLAY=:1",
948 "DEBUGINFOD_URLS= ",
949 "XCURSOR_SIZE=24",
950 }) |target| {
951 for (environ) |variable| {
952 if (mem.eql(u8, mem.span(variable orelse continue), target)) break;
953 } else {
954 testing.expect(false); // Environment variable not found
955 }
956 }
934957}
lib/std/process.zig+3-10
......@@ -816,15 +816,8 @@ pub fn execve(
816816 defer arena_allocator.deinit();
817817 const arena = &arena_allocator.allocator;
818818
819 const argv_buf = try arena.alloc(?[*:0]u8, argv.len + 1);
820 for (argv) |arg, i| {
821 const arg_buf = try arena.alloc(u8, arg.len + 1);
822 @memcpy(arg_buf.ptr, arg.ptr, arg.len);
823 arg_buf[arg.len] = 0;
824 argv_buf[i] = arg_buf[0..arg.len :0].ptr;
825 }
826 argv_buf[argv.len] = null;
827 const argv_ptr = argv_buf[0..argv.len :null].ptr;
819 const argv_buf = try arena.allocSentinel(?[*:0]u8, argv.len, null);
820 for (argv) |arg, i| argv_buf[i] = (try arena.dupeZ(u8, arg)).ptr;
828821
829822 const envp = m: {
830823 if (env_map) |m| {
......@@ -842,5 +835,5 @@ pub fn execve(
842835 }
843836 };
844837
845 return os.execvpeZ_expandArg0(.no_expand, argv_buf.ptr[0].?, argv_ptr, envp);
838 return os.execvpeZ_expandArg0(.no_expand, argv_buf.ptr[0].?, argv_buf.ptr, envp);
846839}