ROCm / MIOpen

AMD's Machine Intelligence Library
https://rocm.docs.amd.com/projects/MIOpen/en/latest/
Other
1.08k stars 226 forks source link

User database file corrupted under Windows #122

Closed stmuxa closed 2 weeks ago

stmuxa commented 5 years ago

If you guys will eventually support Windows platform, it is better to use ios::binary openmode here: https://github.com/ROCmSoftwarePlatform/MIOpen/blob/7a1833b9e645848b470fdb3e279aa2423f60282c/src/db.cpp#L267 https://github.com/ROCmSoftwarePlatform/MIOpen/blob/7a1833b9e645848b470fdb3e279aa2423f60282c/src/db.cpp#L276 Here we have a problem with new line characters, it's 2 bytes under Windows (vs 1 byte under Unix), each database record update introduces 1 byte shift, hence database becomes corrupted without ios::binary.

atamazov commented 5 years ago

It is better to open text files in text mode. You have to configure git properly I guess. https://help.github.com/en/articles/configuring-git-to-handle-line-endings

stmuxa commented 5 years ago

I was talking about user db. It has nothing to do with git.

stmuxa commented 5 years ago

Here is what I saw: Consider text file:

AAA<CRLF>
BBB<CRLF>

(where <CRLF> is a newline under windows platform) and inside this call https://github.com/ROCmSoftwarePlatform/MIOpen/blob/7a1833b9e645848b470fdb3e279aa2423f60282c/src/db.cpp#L287 you want to copy first line from stream from to stream to, and pos->begin = 5 points to the beginning of the second line (in bytes). You use ifstream::read function to do so: from.read(buffer, 5);. This extracts 5 characters from stream from, but <CRLF> is replaced by \n. So, the buffer contents after read is AAA\nB (5 chars). This is what I was talking about,

atamazov commented 5 years ago

Now I see, thanks for the explanation.

ppanchad-amd commented 5 months ago

@stmuxa Please check if this is still an issue with latest ROCm 6.1.1? If not, please close ticket. Thanks!

stmuxa commented 5 months ago

@ppanchad-amd I am not a part of a team anymore. Please, contact repo owner.

taylding-amd commented 2 weeks ago

The issue has been fixed with this PR: https://github.com/ROCm/MIOpen/pull/2515