Fix paths on Windows #7537
Fix paths on Windows #7537
Conversation
| @@ -1235,15 +1235,13 @@ pub const Dir = struct { | |||
| } | |||
| } | |||
|
|
|||
| /// Same as `openDir` except the path parameter is WTF-16 encoded, NT-prefixed. | |||
| /// Same as `openDir` except the path parameter is WTF-16 encoded. | |||
| /// This function asserts the target OS is Windows. | |||
| pub fn openDirW(self: Dir, sub_path_w: [*:0]const u16, args: OpenDirOptions) OpenError!Dir { | |||
There was a problem hiding this comment.
Can we delete this function now? The idea of this function is that it's slightly more efficient if you already have wide strings you want to pass, but if we have to do the conversion anyway, there's no reason for this function to exist.
There was a problem hiding this comment.
If we delete it, then applications that have a u16 string will now need to do 2 conversions, one to u8 and then the second internal conversion.
| const status = ntdll.RtlDosPathNameToNtPathName_U_WithStatus(dos_path, &str, null, null); | ||
| if (status != .SUCCESS) return unexpectedStatus(status); |
There was a problem hiding this comment.
This introduces unnecessary heap allocation, causing OutOfMemory to be added to the OpenError set. We can do better. Check the wine implementation of RtlDosPathNameToNtPathName_U_WithStatus for inspiration. Opening a file should not do heap allocation for the path name.
|
The idea behind this PR was to remove the Windows logic from Zig's standard library and use the implementation that Windows provides instead for Nt path conversion. However, the interface that Windows provides requires heap allocation. @andrewrk has indicated that in order to avoid this heap allocation, we should duplicate the code inside With this new direction, I'll have to start over so I'm going to close this PR and create a new one for the new direction. |
fixes: #6044
This PR replaces the custom logic Zig has implemented to convert legacy DOS paths to NT paths with calls into
ntdllto do the conversion for us. These should be the same conversion functions that Win32 functions likeCreateFileuse to convert DOS paths to NT paths.Now that
ntdllis handling this conversion, missing features like support for"."and".."should be resolved.Follow Up: look for more NT calls
This PR has only replaced the implementation for the
openDir*calls because those functions boil down toNtCreateFilewhich requires an NT formatted path. We should see if there are any other filesystem functions that end up calling NT-specific functions and fix those as well (if there are any). Look for places wheresliceToPrefixedFileWis called and where those paths are passed toNTfunctions such asNtCreateFile.Follow Up: consider RtlDosPathNameToRelativeNtPathName_U_WithStatus
Another thing to follow up on is using
RtlDosPathNameToRelativeNtPathName_U_WithStatusfor relative paths. The current mechanism usesRtlDosPathNameToNtPathName_U_WithStatusfor the path and then removes the CWD prefix. I don't know whether one implementation has benefits over the other. There's also diverging implementations to consider, I noticed that Wine and ReactOS seemed to have differences in opinion on these functions, so we may want to keep our usage of them to a minimal subset.