Skip to content

Fix windows create process retry/path search #2705

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 18, 2019
Merged

Fix windows create process retry/path search #2705

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 22 additions & 15 deletions std/child_process.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -543,25 +543,32 @@ pub const ChildProcess = struct {

const PATH = try process.getEnvVarOwned(self.allocator, "PATH");
defer self.allocator.free(PATH);
const PATHEXT = try process.getEnvVarOwned(self.allocator, "PATHEXT");
defer self.allocator.free(PATHEXT);

var it = mem.tokenize(PATH, ";");
while (it.next()) |search_path| {
const joined_path = try fs.path.join(self.allocator, [_][]const u8{ search_path, app_name });
defer self.allocator.free(joined_path);

const joined_path_w = try unicode.utf8ToUtf16LeWithNull(self.allocator, joined_path);
defer self.allocator.free(joined_path_w);

if (windowsCreateProcess(joined_path_w.ptr, cmd_line_w.ptr, envp_ptr, cwd_w_ptr, &siStartInfo, &piProcInfo)) |_| {
break;
} else |err| if (err == error.FileNotFound) {
continue;
} else {
return err;
retry: while (it.next()) |search_path| {
var ext_it = mem.tokenize(PATHEXT, ";");
while (ext_it.next()) |app_ext| {
const app_basename = try mem.concat(self.allocator, u8, [_][]const u8{app_name[0..app_name.len - 1], app_ext});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing needed to do here, but note that #265 will make this minus one unnecessary and more type-safe.

defer self.allocator.free(app_basename);

const joined_path = try fs.path.join(self.allocator, [_][]const u8{ search_path, app_basename });
defer self.allocator.free(joined_path);

const joined_path_w = try unicode.utf8ToUtf16LeWithNull(self.allocator, joined_path);
defer self.allocator.free(joined_path_w);

if (windowsCreateProcess(joined_path_w.ptr, cmd_line_w.ptr, envp_ptr, cwd_w_ptr, &siStartInfo, &piProcInfo)) |_| {
break :retry;
} else |err| switch (err) {
error.FileNotFound => { continue; },
error.AccessDenied => { continue; },
else => { return err; },
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
else => { return err; },
else => |e| return err,

This will return an error but with the type missing the 2 handled errors above. This might not matter in this case but in general it helps with error set inference.

}
}
} else {
// Every other error would have been returned earlier.
return error.FileNotFound;
return no_path_err; // return the original error
}
};

Expand Down
2 changes: 2 additions & 0 deletions std/os/windows.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -632,6 +632,7 @@ pub fn GetEnvironmentVariableW(lpName: LPWSTR, lpBuffer: LPWSTR, nSize: DWORD) G

pub const CreateProcessError = error{
FileNotFound,
AccessDenied,
InvalidName,
Unexpected,
};
Expand Down Expand Up @@ -663,6 +664,7 @@ pub fn CreateProcessW(
switch (kernel32.GetLastError()) {
ERROR.FILE_NOT_FOUND => return error.FileNotFound,
ERROR.PATH_NOT_FOUND => return error.FileNotFound,
ERROR.ACCESS_DENIED => return error.AccessDenied,
ERROR.INVALID_PARAMETER => unreachable,
ERROR.INVALID_NAME => return error.InvalidName,
else => |err| return unexpectedError(err),
Expand Down