strtoull is a feature added in C++ since C11. This parses the C-string str interpreting its content as an `unsigned long long int` which is more appropriate for the size_t (unsigned integer type) variables used by the `MemoryPressureMonitor.cpp` functions in counterpoint of atoll() what returns a `long long int`.
Created attachment 394354 [details] patch
Comment on attachment 394354 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=394354&action=review > Source/WebKit/UIProcess/linux/MemoryPressureMonitor.cpp:87 > + low += strtoull(token, nullptr, 10); This is not doing any error checking to ensure that the parsing was successful. There are also a generic string-to-integer conversion function in WTF which you could use here: bool ok = true; low += WTF::toIntegralType<size_t, UChar>(token, strlen(token), &ok, 10); if (!ok) return 0; // Return early on error. …and so on for the rest of the conversions.
Created attachment 394598 [details] patch
Created attachment 394599 [details] patch
Created attachment 394600 [details] patch
Created attachment 394601 [details] patch
(In reply to Adrian Perez from comment #2) > Comment on attachment 394354 [details] > patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=394354&action=review > > > Source/WebKit/UIProcess/linux/MemoryPressureMonitor.cpp:87 > > + low += strtoull(token, nullptr, 10); > > This is not doing any error checking to ensure that the parsing was > successful. There are also a generic string-to-integer conversion > function in WTF which you could use here: > > bool ok = true; > low += WTF::toIntegralType<size_t, UChar>(token, strlen(token), &ok, 10); > if (!ok) return 0; // Return early on error. > > …and so on for the rest of the conversions. Thanks for your hints. I've used those in my new patch. In addition, I decided to clean up some repetitive code and fix some issues that I found running WPE in a cgroup-v2 with unified hierarchy.
Comment on attachment 394601 [details] patch Thanks for the patch, having the checks for parsing numeric values in places and some of the duplicated code factored out makes me more confident in this being robust 👍️. Please check the comments before landing. It would be still nice to get rid of “strtok()” because often it is a source of footguns; but I do not have any good suggestion that would avoid writing a more formal parser… so let's just leave things as-is. View in context: https://bugs.webkit.org/attachment.cgi?id=394601&action=review > Source/WebKit/ChangeLog:34 > + getMemoryUsageWithCgroup() sighly. The name of the controller Typo: sighly → slightly > Source/WebKit/UIProcess/linux/MemoryPressureMonitor.cpp:232 > + CString memoryControllerPath = CString(); You can remove the “= CString()” assignment. The default constructor will initialize the value to an empty string anyway.
Created attachment 395030 [details] patch
Created attachment 395031 [details] patch
Committed r259287: <https://trac.webkit.org/changeset/259287> All reviewed patches have been landed. Closing bug and clearing flags on attachment 395031 [details].
<rdar://problem/61105994>