Commit cfffb9c5e9

Andrew Kelley <andrew@ziglang.org>
2020-02-22 23:35:36
improve handling of environment variables on Windows
std.os.getenv and std.os.getenvZ have nice compile errors when not linking libc and using Windows. std.os.getenvW is provided as a Windows-only API that does not require an allocator. It uses the Process Environment Block. std.process.getEnvVarOwned is improved to be a simple wrapper on top of std.os.getenvW. std.process.getEnvMap is improved to use the Process Environment Block rather than calling GetEnvironmentVariableW. std.zig.system.NativePaths uses process.getEnvVarOwned instead of std.os.getenvZ, which works on Windows as well as POSIX.
1 parent 936d0b1
Changed files (5)
lib/std/os/windows/bits.zig
@@ -1187,7 +1187,7 @@ pub const RTL_USER_PROCESS_PARAMETERS = extern struct {
     DllPath: UNICODE_STRING,
     ImagePathName: UNICODE_STRING,
     CommandLine: UNICODE_STRING,
-    Environment: [*]WCHAR,
+    Environment: [*:0]WCHAR,
     dwX: ULONG,
     dwY: ULONG,
     dwXSize: ULONG,
lib/std/os/test.zig
@@ -351,3 +351,11 @@ test "mmap" {
 
     try fs.cwd().deleteFile(test_out_file);
 }
+
+test "getenv" {
+    if (builtin.os == .windows) {
+        expect(os.getenvW(&[_:0]u16{ 'B', 'O', 'G', 'U', 'S', 0x11, 0x22, 0x33, 0x44, 0x55 }) == null);
+    } else {
+        expect(os.getenvZ("BOGUSDOESNOTEXISTENVVAR") == null);
+    }
+}
lib/std/zig/system.zig
@@ -5,6 +5,8 @@ const ArrayList = std.ArrayList;
 const assert = std.debug.assert;
 const process = std.process;
 
+const is_windows = std.Target.current.isWindows();
+
 pub const NativePaths = struct {
     include_dirs: ArrayList([:0]u8),
     lib_dirs: ArrayList([:0]u8),
@@ -21,7 +23,9 @@ pub const NativePaths = struct {
         errdefer self.deinit();
 
         var is_nix = false;
-        if (std.os.getenvZ("NIX_CFLAGS_COMPILE")) |nix_cflags_compile| {
+        if (process.getEnvVarOwned(allocator, "NIX_CFLAGS_COMPILE")) |nix_cflags_compile| {
+            defer allocator.free(nix_cflags_compile);
+
             is_nix = true;
             var it = mem.tokenize(nix_cflags_compile, " ");
             while (true) {
@@ -37,8 +41,14 @@ pub const NativePaths = struct {
                     break;
                 }
             }
+        } else |err| switch (err) {
+            error.InvalidUtf8 => {},
+            error.EnvironmentVariableNotFound => {},
+            error.OutOfMemory => |e| return e,
         }
-        if (std.os.getenvZ("NIX_LDFLAGS")) |nix_ldflags| {
+        if (process.getEnvVarOwned(allocator, "NIX_LDFLAGS")) |nix_ldflags| {
+            defer allocator.free(nix_ldflags);
+
             is_nix = true;
             var it = mem.tokenize(nix_ldflags, " ");
             while (true) {
@@ -57,39 +67,40 @@ pub const NativePaths = struct {
                     break;
                 }
             }
+        } else |err| switch (err) {
+            error.InvalidUtf8 => {},
+            error.EnvironmentVariableNotFound => {},
+            error.OutOfMemory => |e| return e,
         }
         if (is_nix) {
             return self;
         }
 
-        switch (std.builtin.os) {
-            .windows => {},
-            else => {
-                const triple = try std.Target.current.linuxTriple(allocator);
-
-                // TODO: $ ld --verbose | grep SEARCH_DIR
-                // the output contains some paths that end with lib64, maybe include them too?
-                // TODO: what is the best possible order of things?
-                // TODO: some of these are suspect and should only be added on some systems. audit needed.
-
-                try self.addIncludeDir("/usr/local/include");
-                try self.addLibDir("/usr/local/lib");
-                try self.addLibDir("/usr/local/lib64");
-
-                try self.addIncludeDirFmt("/usr/include/{}", .{triple});
-                try self.addLibDirFmt("/usr/lib/{}", .{triple});
-
-                try self.addIncludeDir("/usr/include");
-                try self.addLibDir("/lib");
-                try self.addLibDir("/lib64");
-                try self.addLibDir("/usr/lib");
-                try self.addLibDir("/usr/lib64");
-
-                // example: on a 64-bit debian-based linux distro, with zlib installed from apt:
-                // zlib.h is in /usr/include (added above)
-                // libz.so.1 is in /lib/x86_64-linux-gnu (added here)
-                try self.addLibDirFmt("/lib/{}", .{triple});
-            },
+        if (!is_windows) {
+            const triple = try std.Target.current.linuxTriple(allocator);
+
+            // TODO: $ ld --verbose | grep SEARCH_DIR
+            // the output contains some paths that end with lib64, maybe include them too?
+            // TODO: what is the best possible order of things?
+            // TODO: some of these are suspect and should only be added on some systems. audit needed.
+
+            try self.addIncludeDir("/usr/local/include");
+            try self.addLibDir("/usr/local/lib");
+            try self.addLibDir("/usr/local/lib64");
+
+            try self.addIncludeDirFmt("/usr/include/{}", .{triple});
+            try self.addLibDirFmt("/usr/lib/{}", .{triple});
+
+            try self.addIncludeDir("/usr/include");
+            try self.addLibDir("/lib");
+            try self.addLibDir("/lib64");
+            try self.addLibDir("/usr/lib");
+            try self.addLibDir("/usr/lib64");
+
+            // example: on a 64-bit debian-based linux distro, with zlib installed from apt:
+            // zlib.h is in /usr/include (added above)
+            // libz.so.1 is in /lib/x86_64-linux-gnu (added here)
+            try self.addLibDirFmt("/lib/{}", .{triple});
         }
 
         return self;
lib/std/os.zig
@@ -1104,10 +1104,6 @@ pub fn freeNullDelimitedEnvMap(allocator: *mem.Allocator, envp_buf: []?[*:0]u8)
 /// Get an environment variable.
 /// See also `getenvZ`.
 pub fn getenv(key: []const u8) ?[]const u8 {
-    if (builtin.os == .windows) {
-        // TODO update this to use the ProcessEnvironmentBlock
-        @compileError("TODO implement std.os.getenv for Windows");
-    }
     if (builtin.link_libc) {
         var small_key_buf: [64]u8 = undefined;
         if (key.len < small_key_buf.len) {
@@ -1133,6 +1129,9 @@ pub fn getenv(key: []const u8) ?[]const u8 {
         }
         return null;
     }
+    if (builtin.os == .windows) {
+        @compileError("std.os.getenv is unavailable for Windows because environment string is in WTF-16 format. See std.process.getEnvVarOwned for cross-platform API or std.os.getenvW for Windows-specific API.");
+    }
     // TODO see https://github.com/ziglang/zig/issues/4524
     for (environ) |ptr| {
         var line_i: usize = 0;
@@ -1155,17 +1154,44 @@ pub const getenvC = getenvZ;
 /// Get an environment variable with a null-terminated name.
 /// See also `getenv`.
 pub fn getenvZ(key: [*:0]const u8) ?[]const u8 {
-    if (builtin.os == .windows) {
-        // TODO update this to use the ProcessEnvironmentBlock
-        @compileError("TODO implement std.os.getenv for Windows");
-    }
     if (builtin.link_libc) {
         const value = system.getenv(key) orelse return null;
         return mem.toSliceConst(u8, value);
     }
+    if (builtin.os == .windows) {
+        @compileError("std.os.getenvZ is unavailable for Windows because environment string is in WTF-16 format. See std.process.getEnvVarOwned for cross-platform API or std.os.getenvW for Windows-specific API.");
+    }
     return getenv(mem.toSliceConst(u8, key));
 }
 
+/// Windows-only. Get an environment variable with a null-terminated, WTF-16 encoded name.
+/// See also `getenv`.
+pub fn getenvW(key: [*:0]const u16) ?[:0]const u16 {
+    if (builtin.os != .windows) {
+        @compileError("std.os.getenvW is a Windows-only API");
+    }
+    const key_slice = mem.toSliceConst(u16, key);
+    const ptr = windows.peb().ProcessParameters.Environment;
+    var i: usize = 0;
+    while (ptr[i] != 0) {
+        const key_start = i;
+
+        while (ptr[i] != 0 and ptr[i] != '=') : (i += 1) {}
+        const this_key = ptr[key_start..i];
+
+        if (ptr[i] == '=') i += 1;
+
+        const value_start = i;
+        while (ptr[i] != 0) : (i += 1) {}
+        const this_value = ptr[value_start..i :0];
+
+        if (mem.eql(u16, key_slice, this_key)) return this_value;
+
+        i += 1; // skip over null byte
+    }
+    return null;
+}
+
 pub const GetCwdError = error{
     NameTooLong,
     CurrentWorkingDirectoryUnlinked,
lib/std/process.zig
@@ -37,14 +37,11 @@ pub fn getEnvMap(allocator: *Allocator) !BufMap {
     errdefer result.deinit();
 
     if (builtin.os == .windows) {
-        // TODO update this to use the ProcessEnvironmentBlock
-        const ptr = try os.windows.GetEnvironmentStringsW();
+        const ptr = windows.peb().ProcessParameters.Environment;
         defer os.windows.FreeEnvironmentStringsW(ptr);
 
         var i: usize = 0;
-        while (true) {
-            if (ptr[i] == 0) return result;
-
+        while (ptr[i] != 0) {
             const key_start = i;
 
             while (ptr[i] != 0 and ptr[i] != '=') : (i += 1) {}
@@ -64,6 +61,7 @@ pub fn getEnvMap(allocator: *Allocator) !BufMap {
 
             try result.setMove(key, value);
         }
+        return result;
     } else if (builtin.os == .wasi) {
         var environ_count: usize = undefined;
         var environ_buf_size: usize = undefined;
@@ -141,34 +139,18 @@ pub const GetEnvVarOwnedError = error{
 /// Caller must free returned memory.
 pub fn getEnvVarOwned(allocator: *mem.Allocator, key: []const u8) GetEnvVarOwnedError![]u8 {
     if (builtin.os == .windows) {
-        const key_with_null = try std.unicode.utf8ToUtf16LeWithNull(allocator, key);
-        defer allocator.free(key_with_null);
-
-        var buf = try allocator.alloc(u16, 256);
-        defer allocator.free(buf);
-
-        while (true) {
-            const windows_buf_len = math.cast(os.windows.DWORD, buf.len) catch return error.OutOfMemory;
-            const result = os.windows.GetEnvironmentVariableW(
-                key_with_null.ptr,
-                buf.ptr,
-                windows_buf_len,
-            ) catch |err| switch (err) {
-                error.Unexpected => return error.EnvironmentVariableNotFound,
-                else => |e| return e,
-            };
-            if (result > buf.len) {
-                buf = try allocator.realloc(buf, result);
-                continue;
-            }
+        const result_w = blk: {
+            const key_w = try std.unicode.utf8ToUtf16LeWithNull(allocator, key);
+            defer allocator.free(key_w);
 
-            return std.unicode.utf16leToUtf8Alloc(allocator, buf[0..result]) catch |err| switch (err) {
-                error.DanglingSurrogateHalf => return error.InvalidUtf8,
-                error.ExpectedSecondSurrogateHalf => return error.InvalidUtf8,
-                error.UnexpectedSecondSurrogateHalf => return error.InvalidUtf8,
-                else => |e| return e,
-            };
-        }
+            break :blk std.os.getenvW(key_w) orelse return error.EnvironmentVariableNotFound;
+        };
+        return std.unicode.utf16leToUtf8Alloc(allocator, result_w) catch |err| switch (err) {
+            error.DanglingSurrogateHalf => return error.InvalidUtf8,
+            error.ExpectedSecondSurrogateHalf => return error.InvalidUtf8,
+            error.UnexpectedSecondSurrogateHalf => return error.InvalidUtf8,
+            else => |e| return e,
+        };
     } else {
         const result = os.getenv(key) orelse return error.EnvironmentVariableNotFound;
         return mem.dupe(allocator, u8, result);