microsoft / StorScore

A test framework to evaluate SSDs and HDDs
http://aka.ms/storscore
MIT License
79 stars 40 forks source link

Fixing integer overflow when target >2TB #31

Closed bdenison99 closed 8 years ago

msftclas commented 8 years ago

Hi @bdenison99, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution! This seems like a small (but important) contribution, so no Contribution License Agreement is required at this point. Real humans will now evaluate your PR.

TTYL, MSBOT;

msftclas commented 8 years ago

@bdenison99, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.
Thanks, MSBOT;

bdenison99 commented 8 years ago

This is some additional changes that I hope will fix the GLE:87 errors which map to invalid parameters. These all seem to stem from >2TB disk sizes. Most of the fixes were to change int to int64_t, and stoi to _atoi64. I've tested the latest failing test iterations with this new code, and it worked instead of crashing. Hoping not to see any more.

marksantaniello commented 8 years ago

I'd prefer stoll over _atoi64/c_str. http://www.cplusplus.com/reference/string/stoll/

bdenison99 commented 8 years ago

Any particular reason/benefit for using stoll over _atoi64?

marksantaniello commented 8 years ago

It's just the modern C++ standards-compliant way to accomplish the same thing. Avoids the pointless c_str() business too.

bdenison99 commented 8 years ago

OK - created a new pull request with the _atoi64 converted to std::stoll. Here's hoping that's the end of the >2TB issues.

marksantaniello commented 8 years ago

Thanks very much for the fix!