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

Fix for end of line issue on Windows #2506

Closed
wants to merge 1 commit into from

Conversation

saeid-rostami
Copy link
Contributor

When reading from a text file on Windows line ending is CRLF, unlike on Linux whish is LF.
The code did not properly handle the line ending on Windows due to discrepancy in the interpretation of line endings between the tellg() function and the C++ runtime.
tellg() function considers CRLF (Carriage Return + Line Feed) as two characters, but when filling the buffer, C++ runtime interprets CRLF as a single character. As a result, the buffer contained incorrect data at the end of the buffer.
To address this issue, files are opened in binary mode to ensure that line ending is LF in both Windows and Linux.

@atamazov
Copy link
Contributor

atamazov commented Nov 4, 2023

@DrizztDoUrden Can you please look at this.

@atamazov
Copy link
Contributor

atamazov commented Nov 6, 2023

@saeid-rostami I am not sure that if using uif2-initial as a baseline is a good idea. You may with to rebase this change on top of develop. That would allow merging your fix into the main development branch of MIOpen sooner than uif2-initial is merged in. But this is up to you, of course.

@saeid-rostami
Copy link
Contributor Author

@atamazov , I'll send another PR for develop branch. Thanks.

@junliume
Copy link
Collaborator

junliume commented Nov 10, 2023

@saeid-rostami with #2515 up, should we close this one? if this is needed for uif2-initial branch which is also understandable, then let us know.

@atamazov
Copy link
Contributor

@junliume uif2-initial is only 7 commits behind develop and can be easily updated from there, I guess ;)

@saeid-rostami
Copy link
Contributor Author

saeid-rostami commented Nov 13, 2023

@junliume , Yes we can close this one. Already sent another one to develop branch.

@junliume junliume closed this Nov 13, 2023
@junliume junliume deleted the windows_unittest_end_of_file branch February 6, 2024 20:18
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.

4 participants