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.

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

Implemented windows versions of seekTo and getPos #710

Merged
merged 6 commits into from Jan 19, 2018

Conversation

Projects
None yet
3 participants
@Hejsil
Member

Hejsil commented Jan 19, 2018

No description provided.

std/io.zig Outdated
const err = system.GetLastError();
return switch (err) {
system.ERROR.INVALID_PARAMETER => error.BadFd,
else => os.unexpectedErrorPosix(err),

This comment has been minimized.

@andrewrk

andrewrk Jan 19, 2018

Member

I think this should be unexpectedErrorWindows

@@ -74,6 +74,9 @@ pub extern "kernel32" stdcallcc fn ReadFile(in_hFile: HANDLE, out_lpBuffer: LPVO
in_nNumberOfBytesToRead: DWORD, out_lpNumberOfBytesRead: &DWORD,
in_out_lpOverlapped: ?&OVERLAPPED) -> BOOL;
pub extern "kernel32" stdcallcc fn SetFilePointerEx(in_fFile: HANDLE, in_liDistanceToMove: LARGE_INTEGER,
out_opt_ldNewFilePointer: ?PLARGE_INTEGER, in_dwMoveMethod: DWORD) -> BOOL;

This comment has been minimized.

@andrewrk

andrewrk Jan 19, 2018

Member

The precedent here is to use ?&LARGE_INTEGER instead of PLARGE_INTEGER. The P naming by Microsoft is a bit of an anti-pattern.

std/io.zig Outdated
@@ -245,6 +263,18 @@ pub const File = struct {
}
return result;
},
Os.windows => {
var pos : usize = undefined;
if (system.SetFilePointerEx(self.handle, 0, @ptrCast(system.PLARGE_INTEGER, &pos), system.FILE_CURRENT) == 0) {

This comment has been minimized.

@andrewrk

andrewrk Jan 19, 2018

Member

LARGE_INTEGER is always an i64. usize is pointer-sized. These are not necessarily the same byte size. Also the signed-ness is different. So part of the job of this function is to get the i64 from Windows, and then convert it to a usize, making correct decisions about how to handle the following situations:

  • when usize is smaller than i64 (probably return error.FilePosLargerThanPointerRange, which is maybe a hint that we should actually return u64 instead of usize from getPos, but that's a separate issue, which could be fixed later)
  • when the i64 we get from microsoft is negative instead of positive (maybe we assert that it will not happen? maybe we return an error?)

Generally, @ptrCast should be avoided if possible, and this is a situation where I don't think we need one.

std/io.zig Outdated
@@ -224,6 +233,15 @@ pub const File = struct {
};
}
},
Os.windows => {
if (system.SetFilePointerEx(self.handle, @bitCast(isize, pos), null, system.FILE_BEGIN) == 0) {

This comment has been minimized.

@andrewrk

andrewrk Jan 19, 2018

Member

bitcasting pos to isize could produce an invalid value if pos has a 1 for the highest-order bit. We should handle this edge case correctly. perhaps std.math.cast could be of use.

@andrewrk andrewrk merged commit 2eede35 into ziglang:master Jan 19, 2018

0 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@MaxValue

This comment has been minimized.

MaxValue commented on d8469e3 Jan 21, 2018

Hey guys! Cool to be part of this!

@Hejsil Hejsil deleted the Hejsil:seekto-getpos-windows branch Jan 22, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment