bneradt commented on code in PR #12688:
URL: https://github.com/apache/trafficserver/pull/12688#discussion_r2560575283
##########
src/iocore/cache/CacheDir.cc:
##########
@@ -724,9 +734,12 @@ Directory::remove(const CacheKey *key, StripeSM *stripe,
Dir *del)
return 0;
}
#endif
- if (dir_compare_tag(e, key) && dir_offset(e) == dir_offset(del)) {
+ int64_t offset = dir_offset(e);
+ if (dir_compare_tag(e, key) && offset == dir_offset(del)) {
ts::Metrics::Gauge::decrement(cache_rsb.direntries_used);
ts::Metrics::Gauge::decrement(stripe->cache_vol->vol_rsb.direntries_used);
+ ATS_PROBE7(cache_dir_remove, stripe ? stripe->fd : -1, s,
dir_to_offset(e, seg), offset, dir_approx_size(e),
+ key->slice64(0), key->slice64(1));
Review Comment:
Minor: probably just remove the null check on stripe. You don't do that
elsewhere in this patch, and it was just dereferenced on the previous line. So
if there's a problem with it being null, it won't be your fault. :)
Maybe add `ink_assert(stripe != nullptr);` to the top of the function
stating the invariant that it whould be non-null? Taking a `&` is a possibility
too, but then this patch is increasing beyond scope. So the assert should be
fine.
--
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]
