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

Added BOOST_WINDOWS_API support to parse.cpp #8

Merged
merged 2 commits into from
Oct 21, 2015

Conversation

pkravchuk
Copy link
Collaborator

path.c_str() returns value_type*, which is a wchar_t* if BOOST_WINDOWS_API is defined. Tinyxml uses fopen to open files, which does not take wchar_t argument. This fix leaves tinyxml invariant, but there can be an error if a file with non-ascii characters is being opened. As far as ascii characters are concerned, everything should be ok. I don't know a good solution to this problem, here's why. Windows standard workaround is wfopen, but this function is not available on Cygwin, which is the environment that is most likely to be used on Windows to build sdpb. The reason for no wfopen is that Cygwin follows posix. However, boost does not believe that cygwin is posix and defines BOOST_WINDOWS_API. Perhaps I missed something, but that seems to be the situation.

path.c_str() returns value_type*, which is a wchar_t* if BOOST_WINDOWS_API is defined. Tinyxml uses fopen to open files, which does not take wchar_t argument. This fix leaves tinyxml invariant, but there can be an error if a file with non-ascii characters is being opened. Windows workaround is wfopen, but this function is not available on Cygwin, which is the environment that is most likely to be used on Windows to build sdpb. Cygwin follows posix, so that's why wfopen is not available. However, boost does not believe that cygwin is posix and defines BOOST_WINDOWS_API.
davidsd added a commit that referenced this pull request Oct 21, 2015
Added BOOST_WINDOWS_API support to parse.cpp
@davidsd davidsd merged commit e740fdb into davidsd:master Oct 21, 2015
@davidsd
Copy link
Owner

davidsd commented Oct 21, 2015

That's a pretty painless change. Thanks for figuring it out!

@pkravchuk
Copy link
Collaborator Author

The windows version of the call would perhaps work on all systems, but I can't test this. It's perhaps the safest to just leave it as it is.

@davidsd
Copy link
Owner

davidsd commented Oct 21, 2015

I just tested the windows call on Linux, and it seems to work fine. So I'll just switch to that and remove the ifndef.

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