From 5dfca87a65884f8564a8c8e2444bb906073425f3 Mon Sep 17 00:00:00 2001 From: Suirad Date: Thu, 22 Nov 2018 04:15:22 -0600 Subject: [PATCH 1/6] Update windows imports --- std/os/index.zig | 81 ++++++++++++++++++++++++++++++++----- std/os/windows/kernel32.zig | 6 +-- 2 files changed, 74 insertions(+), 13 deletions(-) diff --git a/std/os/index.zig b/std/os/index.zig index 15be08c689..85e7eab01c 100644 --- a/std/os/index.zig +++ b/std/os/index.zig @@ -702,8 +702,8 @@ pub fn getEnvMap(allocator: *Allocator) !BufMap { errdefer result.deinit(); if (is_windows) { - const ptr = windows.GetEnvironmentStringsA() orelse return error.OutOfMemory; - defer assert(windows.FreeEnvironmentStringsA(ptr) != 0); + const ptr = windows.GetEnvironmentStringsW() orelse return error.OutOfMemory; + defer assert(windows.FreeEnvironmentStringsW(ptr) != 0); var i: usize = 0; while (true) { @@ -712,17 +712,50 @@ pub fn getEnvMap(allocator: *Allocator) !BufMap { const key_start = i; while (ptr[i] != 0 and ptr[i] != '=') : (i += 1) {} - const key = ptr[key_start..i]; + + const stack_var_len = 50; + const key_slice = ptr[key_start..i]; + var key: []u8 = undefined; + var heap_key = false; + + // parse the key on the stack if smaller than 'stack_var_len' + if (key_slice.len < stack_var_len-@sizeOf(usize)) { + var buf = []u8{0} ** stack_var_len; + var fallocator = &std.heap.FixedBufferAllocator.init(buf[0..]).allocator; + key = try std.unicode.utf16leToUtf8Alloc(fallocator, key_slice); + } else { + key = try std.unicode.utf16leToUtf8Alloc(allocator, key_slice); + heap_key = true; // key needs to outlive this scope, so we cannot defer + } if (ptr[i] == '=') i += 1; const value_start = i; while (ptr[i] != 0) : (i += 1) {} - const value = ptr[value_start..i]; + + const value_slice = ptr[value_start..i]; + var value: []u8 = undefined; + var heap_value = false; + + if (value_slice.len < stack_var_len-@sizeOf(usize)) { + var buf = []u8{0} ** stack_var_len; + var fallocator = &std.heap.FixedBufferAllocator.init(buf[0..]).allocator; + value = try std.unicode.utf16leToUtf8Alloc(fallocator, value_slice); + } else { + value = try std.unicode.utf16leToUtf8Alloc(allocator, value_slice); + heap_value = true; // value needs to outlive this scope, so we cannot defer + } i += 1; // skip over null byte try result.set(key, value); + + if (heap_key) { + allocator.free(key); + } + if (heap_value) { + allocator.free(value); + } } } else { for (posix_environ_raw) |ptr| { @@ -740,6 +773,19 @@ pub fn getEnvMap(allocator: *Allocator) !BufMap { } } +test "os.getEnvMap" { + var env = try getEnvMap(std.debug.global_allocator); + var seen_path = false; + var it = env.iterator(); + while (it.next()) |pair| { + if (mem.eql(u8, pair.key, "PATH")) { + seen_path = true; + } + } + + assert(seen_path == true); +} + /// TODO make this go through libc when we have it pub fn getEnvPosix(key: []const u8) ?[]const u8 { for (posix_environ_raw) |ptr| { @@ -760,21 +806,27 @@ pub fn getEnvPosix(key: []const u8) ?[]const u8 { pub const GetEnvVarOwnedError = error{ OutOfMemory, EnvironmentVariableNotFound, + DanglingSurrogateHalf, + ExpectedSecondSurrogateHalf, + UnexpectedSecondSurrogateHalf, + + /// See https://github.com/ziglang/zig/issues/1774 + InvalidUtf8, }; /// Caller must free returned memory. /// TODO make this go through libc when we have it pub fn getEnvVarOwned(allocator: *mem.Allocator, key: []const u8) GetEnvVarOwnedError![]u8 { if (is_windows) { - const key_with_null = try cstr.addNullByte(allocator, key); + const key_with_null = try std.unicode.utf8ToUtf16LeWithNull(allocator, key); defer allocator.free(key_with_null); - var buf = try allocator.alloc(u8, 256); - errdefer allocator.free(buf); + var buf = try allocator.alloc(u16, 256); + defer allocator.free(buf); while (true) { const windows_buf_len = math.cast(windows.DWORD, buf.len) catch return error.OutOfMemory; - const result = windows.GetEnvironmentVariableA(key_with_null.ptr, buf.ptr, windows_buf_len); + const result = windows.GetEnvironmentVariableW(key_with_null.ptr, buf.ptr, windows_buf_len); if (result == 0) { const err = windows.GetLastError(); @@ -788,11 +840,11 @@ pub fn getEnvVarOwned(allocator: *mem.Allocator, key: []const u8) GetEnvVarOwned } if (result > buf.len) { - buf = try allocator.realloc(u8, buf, result); + buf = try allocator.realloc(u16, buf, result); continue; } - return allocator.shrink(u8, buf, result); + return try std.unicode.utf16leToUtf8Alloc(allocator, buf); } } else { const result = getEnvPosix(key) orelse return error.EnvironmentVariableNotFound; @@ -800,6 +852,15 @@ pub fn getEnvVarOwned(allocator: *mem.Allocator, key: []const u8) GetEnvVarOwned } } +test "os.getEnvVarOwned" { + switch (builtin.os) { + builtin.Os.windows, builtin.Os.linux, builtin.Os.macosx, + builtin.Os.ios => _ = try getEnvVarOwned(debug.global_allocator, "PATH"), + else => @compileError("unimplemented"), + } +} + + /// Caller must free the returned memory. pub fn getCwdAlloc(allocator: *Allocator) ![]u8 { var buf: [MAX_PATH_BYTES]u8 = undefined; diff --git a/std/os/windows/kernel32.zig b/std/os/windows/kernel32.zig index 7eec5faba9..202b8bffeb 100644 --- a/std/os/windows/kernel32.zig +++ b/std/os/windows/kernel32.zig @@ -50,7 +50,7 @@ pub extern "kernel32" stdcallcc fn FindFirstFileW(lpFileName: [*]const u16, lpFi pub extern "kernel32" stdcallcc fn FindClose(hFindFile: HANDLE) BOOL; pub extern "kernel32" stdcallcc fn FindNextFileW(hFindFile: HANDLE, lpFindFileData: *WIN32_FIND_DATAW) BOOL; -pub extern "kernel32" stdcallcc fn FreeEnvironmentStringsA(penv: [*]u8) BOOL; +pub extern "kernel32" stdcallcc fn FreeEnvironmentStringsW(penv: [*]u16) BOOL; pub extern "kernel32" stdcallcc fn GetCommandLineA() LPSTR; @@ -63,9 +63,9 @@ pub extern "kernel32" stdcallcc fn GetCurrentDirectoryW(nBufferLength: DWORD, lp pub extern "kernel32" stdcallcc fn GetCurrentThread() HANDLE; pub extern "kernel32" stdcallcc fn GetCurrentThreadId() DWORD; -pub extern "kernel32" stdcallcc fn GetEnvironmentStringsA() ?[*]u8; +pub extern "kernel32" stdcallcc fn GetEnvironmentStringsW() ?[*]u16; -pub extern "kernel32" stdcallcc fn GetEnvironmentVariableA(lpName: LPCSTR, lpBuffer: LPSTR, nSize: DWORD) DWORD; +pub extern "kernel32" stdcallcc fn GetEnvironmentVariableW(lpName: LPWSTR, lpBuffer: LPWSTR, nSize: DWORD) DWORD; pub extern "kernel32" stdcallcc fn GetExitCodeProcess(hProcess: HANDLE, lpExitCode: *DWORD) BOOL; From 0abd5520bdc8118369895f67f581d10c847ac139 Mon Sep 17 00:00:00 2001 From: Suirad Date: Sat, 24 Nov 2018 17:29:06 -0600 Subject: [PATCH 2/6] Platform specific tests --- std/os/index.zig | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/std/os/index.zig b/std/os/index.zig index 85e7eab01c..62059a335b 100644 --- a/std/os/index.zig +++ b/std/os/index.zig @@ -775,15 +775,26 @@ pub fn getEnvMap(allocator: *Allocator) !BufMap { test "os.getEnvMap" { var env = try getEnvMap(std.debug.global_allocator); - var seen_path = false; + var seen_home = false; var it = env.iterator(); while (it.next()) |pair| { - if (mem.eql(u8, pair.key, "PATH")) { - seen_path = true; - } + switch (builtin.os){ + builtin.Os.windows => { + if (mem.eql(u8, pair.key, "HOMEPATH")) { + seen_home = true; + } + }, + builtin.Os.linux, builtin.Os.macosx, + builtin.Os.ios => { + if (mem.eql(u8, pair.key, "HOME")) { + seen_home = true; + } + }, + else => @compileError("unimplemented"), + } } - assert(seen_path == true); + assert(seen_home == true); } /// TODO make this go through libc when we have it @@ -854,8 +865,10 @@ pub fn getEnvVarOwned(allocator: *mem.Allocator, key: []const u8) GetEnvVarOwned test "os.getEnvVarOwned" { switch (builtin.os) { - builtin.Os.windows, builtin.Os.linux, builtin.Os.macosx, - builtin.Os.ios => _ = try getEnvVarOwned(debug.global_allocator, "PATH"), + builtin.Os.windows => _ = try getEnvVarOwned(debug.global_allocator, "HOMEPATH"), + + builtin.Os.linux, builtin.Os.macosx, + builtin.Os.ios => _ = try getEnvVarOwned(debug.global_allocator, "HOME"), else => @compileError("unimplemented"), } } From 24592d0216ce4830e9f0dd51a7d2542f1f8afa05 Mon Sep 17 00:00:00 2001 From: Suirad Date: Sat, 24 Nov 2018 19:21:12 -0600 Subject: [PATCH 3/6] Add more padding to parse buffer --- std/os/index.zig | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/std/os/index.zig b/std/os/index.zig index 62059a335b..4272374b3a 100644 --- a/std/os/index.zig +++ b/std/os/index.zig @@ -15,7 +15,7 @@ test "std.os" { _ = @import("get_user_id.zig"); _ = @import("linux/index.zig"); _ = @import("path.zig"); - _ = @import("test.zig"); + _ = @import("test.zig"); _ = @import("time.zig"); _ = @import("windows/index.zig"); _ = @import("get_app_data_dir.zig"); @@ -718,8 +718,9 @@ pub fn getEnvMap(allocator: *Allocator) !BufMap { var key: []u8 = undefined; var heap_key = false; - // parse the key on the stack if smaller than 'stack_var_len' - if (key_slice.len < stack_var_len-@sizeOf(usize)) { + /// revisit needing the "-@sizeof(usize)*2" + /// after https://github.com/ziglang/zig/issues/1774 + if (key_slice.len < stack_var_len-@sizeOf(usize)*2) { var buf = []u8{0} ** stack_var_len; var fallocator = &std.heap.FixedBufferAllocator.init(buf[0..]).allocator; key = try std.unicode.utf16leToUtf8Alloc(fallocator, key_slice); @@ -737,7 +738,9 @@ pub fn getEnvMap(allocator: *Allocator) !BufMap { var value: []u8 = undefined; var heap_value = false; - if (value_slice.len < stack_var_len-@sizeOf(usize)) { + /// revisit needing the "-@sizeof(usize)*2" + /// after https://github.com/ziglang/zig/issues/1774 + if (value_slice.len < stack_var_len-@sizeOf(usize)*2) { var buf = []u8{0} ** stack_var_len; var fallocator = &std.heap.FixedBufferAllocator.init(buf[0..]).allocator; value = try std.unicode.utf16leToUtf8Alloc(fallocator, value_slice); From 1fa2217c1008fefa7084ea34fedcf79ad214a02e Mon Sep 17 00:00:00 2001 From: Suirad Date: Thu, 29 Nov 2018 03:24:36 -0600 Subject: [PATCH 4/6] Simplify implementation --- std/os/index.zig | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/std/os/index.zig b/std/os/index.zig index 4272374b3a..cc7479b871 100644 --- a/std/os/index.zig +++ b/std/os/index.zig @@ -15,7 +15,7 @@ test "std.os" { _ = @import("get_user_id.zig"); _ = @import("linux/index.zig"); _ = @import("path.zig"); - _ = @import("test.zig"); + _ = @import("test.zig"); _ = @import("time.zig"); _ = @import("windows/index.zig"); _ = @import("get_app_data_dir.zig"); @@ -705,28 +705,26 @@ pub fn getEnvMap(allocator: *Allocator) !BufMap { const ptr = windows.GetEnvironmentStringsW() orelse return error.OutOfMemory; defer assert(windows.FreeEnvironmentStringsW(ptr) != 0); + var buf: [100]u8 = undefined; + var i: usize = 0; while (true) { if (ptr[i] == 0) return result; const key_start = i; + var fallocator = &std.heap.FixedBufferAllocator.init(buf[0..]).allocator; while (ptr[i] != 0 and ptr[i] != '=') : (i += 1) {} - const stack_var_len = 50; const key_slice = ptr[key_start..i]; var key: []u8 = undefined; var heap_key = false; - /// revisit needing the "-@sizeof(usize)*2" - /// after https://github.com/ziglang/zig/issues/1774 - if (key_slice.len < stack_var_len-@sizeOf(usize)*2) { - var buf = []u8{0} ** stack_var_len; - var fallocator = &std.heap.FixedBufferAllocator.init(buf[0..]).allocator; - key = try std.unicode.utf16leToUtf8Alloc(fallocator, key_slice); - } else { + key = std.unicode.utf16leToUtf8Alloc(fallocator, key_slice) catch undefined; + + if (key.len == 0) { key = try std.unicode.utf16leToUtf8Alloc(allocator, key_slice); - heap_key = true; // key needs to outlive this scope, so we cannot defer + heap_key = true; } if (ptr[i] == '=') i += 1; @@ -738,15 +736,11 @@ pub fn getEnvMap(allocator: *Allocator) !BufMap { var value: []u8 = undefined; var heap_value = false; - /// revisit needing the "-@sizeof(usize)*2" - /// after https://github.com/ziglang/zig/issues/1774 - if (value_slice.len < stack_var_len-@sizeOf(usize)*2) { - var buf = []u8{0} ** stack_var_len; - var fallocator = &std.heap.FixedBufferAllocator.init(buf[0..]).allocator; - value = try std.unicode.utf16leToUtf8Alloc(fallocator, value_slice); - } else { + value = std.unicode.utf16leToUtf8Alloc(fallocator, value_slice) catch undefined; + + if (value.len == 0) { value = try std.unicode.utf16leToUtf8Alloc(allocator, value_slice); - heap_value = true; // value needs to outlive this scope, so we cannot defer + heap_value = true; } i += 1; // skip over null byte From e8e6ae57d41fdcdb4027ae95c04ad7aeea4a7aac Mon Sep 17 00:00:00 2001 From: Suirad Date: Thu, 29 Nov 2018 16:03:12 -0600 Subject: [PATCH 5/6] Find CI env variables --- std/os/index.zig | 1 + 1 file changed, 1 insertion(+) diff --git a/std/os/index.zig b/std/os/index.zig index cc7479b871..d298ff4329 100644 --- a/std/os/index.zig +++ b/std/os/index.zig @@ -775,6 +775,7 @@ test "os.getEnvMap" { var seen_home = false; var it = env.iterator(); while (it.next()) |pair| { + debug.warn("{}: {}\n", pair.key, pair.value); switch (builtin.os){ builtin.Os.windows => { if (mem.eql(u8, pair.key, "HOMEPATH")) { From cf266ff80a69d19140465b949350421dc541d4c8 Mon Sep 17 00:00:00 2001 From: Suirad Date: Fri, 30 Nov 2018 02:15:33 -0600 Subject: [PATCH 6/6] Update tests --- std/os/index.zig | 31 +++---------------------------- 1 file changed, 3 insertions(+), 28 deletions(-) diff --git a/std/os/index.zig b/std/os/index.zig index d298ff4329..779fa8f8c8 100644 --- a/std/os/index.zig +++ b/std/os/index.zig @@ -772,27 +772,7 @@ pub fn getEnvMap(allocator: *Allocator) !BufMap { test "os.getEnvMap" { var env = try getEnvMap(std.debug.global_allocator); - var seen_home = false; - var it = env.iterator(); - while (it.next()) |pair| { - debug.warn("{}: {}\n", pair.key, pair.value); - switch (builtin.os){ - builtin.Os.windows => { - if (mem.eql(u8, pair.key, "HOMEPATH")) { - seen_home = true; - } - }, - builtin.Os.linux, builtin.Os.macosx, - builtin.Os.ios => { - if (mem.eql(u8, pair.key, "HOME")) { - seen_home = true; - } - }, - else => @compileError("unimplemented"), - } - } - - assert(seen_home == true); + defer env.deinit(); } /// TODO make this go through libc when we have it @@ -862,13 +842,8 @@ pub fn getEnvVarOwned(allocator: *mem.Allocator, key: []const u8) GetEnvVarOwned } test "os.getEnvVarOwned" { - switch (builtin.os) { - builtin.Os.windows => _ = try getEnvVarOwned(debug.global_allocator, "HOMEPATH"), - - builtin.Os.linux, builtin.Os.macosx, - builtin.Os.ios => _ = try getEnvVarOwned(debug.global_allocator, "HOME"), - else => @compileError("unimplemented"), - } + var ga = debug.global_allocator; + debug.assertError(getEnvVarOwned(ga, "BADENV"), error.EnvironmentVariableNotFound); }