Copilot commented on code in PR #13234:
URL: https://github.com/apache/trafficserver/pull/13234#discussion_r3376727812
##########
src/iocore/cache/RamCacheLRU.cc:
##########
@@ -190,9 +190,12 @@ RamCacheLRU::put(CryptoHash *key, IOBufferData *data,
[[maybe_unused]] uint32_t
}
uint32_t i = key->slice32(3) % nbuckets;
if ((cache_config_ram_cache_use_seen_filter == 1) ||
- // If proxy.config.cache.ram_cache.use_seen_filter is > 1, and the
cache is more than <n>% full, then use the seen filter.
- // <n>% is calculated based on this setting, with 2 == 50%, 3 == 67%, 4
== 75%, up to 9 == 90%.
- ((cache_config_ram_cache_use_seen_filter > 1) && (bytes >= max_bytes *
(1 - (1 / cache_config_ram_cache_use_seen_filter))))) {
+ // For use_seen_filter > 1, only apply the filter once the cache is more
than <n>% full:
+ // 2 == 50%, 3 == 67%, 4 == 75%, up to 9 == 90%. Written as bytes * N >=
max_bytes * (N - 1)
+ // rather than bytes >= max_bytes * (1 - 1 / N); the latter evaluates 1
/ N in integer
+ // arithmetic (0 for every N > 1), so the filter only ever engaged at
100% full.
+ ((cache_config_ram_cache_use_seen_filter > 1) &&
+ (bytes * cache_config_ram_cache_use_seen_filter >= max_bytes *
(cache_config_ram_cache_use_seen_filter - 1)))) {
Review Comment:
The new threshold check multiplies two signed `int64_t` values
(`bytes`/`max_bytes`) by the (unbounded) config integer. If
`proxy.config.cache.ram_cache.use_seen_filter` is accidentally set to a large
value, `bytes * N` / `max_bytes * (N - 1)` can overflow `int64_t`, which is
undefined behavior. Consider promoting the arithmetic to `__int128` (or
otherwise guarding/clamping `N`) so the comparison remains well-defined for all
config inputs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
