Mem value sanitizing - take 3 #654
Conversation
|
Commit to address last review issue in #633 coming shortly... |
|
@gerboland I just fixed the last issue you detected |
|
@townsend2010 only the |
|
@ricab, Yeah, I saw that, but is it necessary? I'd really like to just get this merged and not be blocked on the move to the C++17 standard. |
|
@townsend2010 without further changes, compilation breaks if I remove it (because a warning is issued which is treated as an error) |
|
@ricab, Ok, I was afraid of that:/ Oh well... |
|
Perhaps the previous "solution": a fallthrough comment, will avoid a warning and mean we can land this? |
|
@gerboland is that enough for clang and msvc too? |
|
I've seen people say clang supports same thing, but I'll test it on Mac & Windows to be sure. |
Use a corrected regular expression to validate memory size values. Accept lowercase suffixes (k, m, g). Improve and extend corresponding tests. Fixes #470.
Test a new memory normalization interface, to simultaneously validate and convert memory values to the smallest unit (bytes).
Implement verification and normalization of memory values in a single step. Use that in the daemon, replacing mere validation. Update hostname validation for consistency. Update tests accordingly. Fixes #616.
Provide a first step to obtain the number of bytes (to compare with other numbers) and a second step to express the same thing as a string that is suitable for backend memory/disk arguments.
Make tests expect error messages in error stream instead.
Makes failing tests pass. Fixes #588.
This attempts to address a review concern.
Use Qt's API rather than the standard library to convert a QString to long long, thus avoiding the intermediate std::string step. This addresses a review concern.
Drop structured-bindings and access struct attributes by name. This addresses a review request.
Replace QString::front with QString::at. The former was only introduced in Qt 5.10 but we are using Qt 5.9.
C++17 is not supported quite yet and this needs to be merged.
ricab
force-pushed
the
mem-value-sanitizing
branch
from
6376d2a to
5732fb9
Compare
|
OK, just finished rebasing on master and resolving all conflicts. I hope everything is sane... I had to make a few changes to integrate the new stream encapsulation. The most significant were in this commit, but there were others. Dropped |
Codecov Report
@@ Coverage Diff @@
## master #654 +/- ##
==========================================
+ Coverage 65.86% 66.39% +0.52%
==========================================
Files 165 165
Lines 5888 5912 +24
==========================================
+ Hits 3878 3925 +47
+ Misses 2010 1987 -23
Continue to review full report at Codecov.
|
1 similar comment
Codecov Report
@@ Coverage Diff @@
## master #654 +/- ##
==========================================
+ Coverage 65.86% 66.39% +0.52%
==========================================
Files 165 165
Lines 5888 5912 +24
==========================================
+ Hits 3878 3925 +47
+ Misses 2010 1987 -23
Continue to review full report at Codecov.
|
654: Mem value sanitizing - take 3 r=gerboland a=ricab Moved from #633 (after rebase and force-push). Co-authored-by: Ricardo Abreu <[email protected]>
Build succeeded |
Moved from #633 (after rebase and force-push).