Skip to content
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

open ifstream in zstr as binary always #793

Merged
merged 1 commit into from
Apr 7, 2022
Merged

open ifstream in zstr as binary always #793

merged 1 commit into from
Apr 7, 2022

Conversation

svigerske
Copy link
Collaborator

@svigerske svigerske commented Apr 6, 2022

Sorry that things come in small pieces.
I had problems reading compressed input on Windows with zstr. (There was an exception from std::string thrown.)

Turned out that this is due to mateidavid/zstr#15, i.e., not opening the input stream as binary. On Windows, there are some conversions of carriage-return (\r) happening in that case that cause trouble.

This PR changes the zstr library to always open the input stream in binary form.
As far as I understand zstr/issues#15, this means that \r in the input are not converted away.
Fortunately, your LP and MPS readers seem to handle this case:

@svigerske
Copy link
Collaborator Author

svigerske commented Apr 6, 2022

Of course it had to fail on mac now.

PS: Changed to open in binary mode only if windows.

- fixes reading compressed input on windows by avoiding trouble with
  end-of-line conversions, see zstr/issues#15
- the lp+mps readers can handle \r in lines, fortunately
Copy link
Member

@jajhall jajhall left a comment

Choose a reason for hiding this comment

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

Thanks

@jajhall jajhall merged commit 586cacc into master Apr 7, 2022
@svigerske svigerske deleted the fix-zstr-windows branch October 8, 2023 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants