PrivateAggregationHost: Simplify logic that drops excess contributions
I was concerned about potential UB in ContributeToHistogram() around `int num_to_copy`. The expression subtracts a size_t from an int before assigning the resulting size_t into an int. I think this was actually safe as written because of we never keep more contributions than the maximum, but I think it deserves a rewrite. This CL makes the following changes: (1) It changes the type of kMaxNumberOfContributions from int to size_t. This complies with the Chromium C++ style guide's recommendation to use size_t for object counts, which I believe supersedes the Google C++ style guide's decision to use int for nearly everything. See <https://chromium.googlesource.com/chromium/src/+/main/styleguide/c++/c++.md#types>. (2) It adds some local type aliases and rephrases the arithmetic in a way that I think is easier to understand. (3) It CHECKs the invariant that we never accept more than the maximum number of contributions. (4) It replaces std::vector::resize() with base::span::first(). Apparently, resize() takes O(n) time relative to the size difference, whereas the span operation should be O(1). (5) It fixes the call to `base::ranges::transform()`, which violated the precondition that `op` must not modify elements in the input range. I think this was technically fine because the contribution type boils down to a POD struct, but let's not tempt fate. Change-Id: I8b4f65b90c6a25d0d8c646446211879ef251a545 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5402426 Reviewed-by:Alex Turner <alexmt@chromium.org> Commit-Queue: Dan McArdle <dmcardle@chromium.org> Cr-Commit-Position: refs/heads/main@{#1280331}
Loading
Please register or sign in to comment