-
Notifications
You must be signed in to change notification settings - Fork 87
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
Fix deserialization on Windows #32
base: master
Are you sure you want to change the base?
Conversation
@rostyboost sorry for another ping, but could you pleast take a look at this one too? This should make vectorflow usable on Windows. |
@rostyboost ping |
1 similar comment
@rostyboost ping |
@rostyboost ping. How is your day? :) |
@rostyboost ping. Could you please look at it? |
@rostyboost just anpother ping. Have a great day! |
@rostyboost Hi, it's been a while. Maybe you got some free time to merge this? ^^ |
auto str = new ubyte[str_sz.to!size_t]; | ||
str = _f.rawRead(str); | ||
return cast(string)str; | ||
// On Windows EOF is not set before you try to read past the end, so it requires some special handling |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this actually Windows specific? On Linux I get an assertion failure too, with this program:
import std.file;
import std.stdio;
void main()
{
std.file.write("test.txt", "Test\n");
auto f = File("test.txt");
f.readln();
assert(f.eof);
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be fair, I didn't do much checking - the tests failed on Windows and they didn't fail on Linux. Therefore I debugged a little and found out this. I can't remember the details anymore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This really looks like it needs to be fixed somewhere else.
Merge time? 🤔 |
On Windows, the
EOF
flag is not set as long as you don't read past the end of the file. That causes current deserialization algorithm to crash on Windows with obscure errors. I added specialversion(Windows)
code into deserialization logic to handle Windows file reading. I left the original code untouched to make it distinguishable.This PR makes
vectorflow
pass all the tests on windows, btw :^)