Copilot commented on code in PR #13616:
URL: https://github.com/apache/trafficserver/pull/13616#discussion_r3908873086
##########
src/tsutil/Metrics.cc:
##########
@@ -245,9 +253,52 @@ Metrics::Storage::rename(Metrics::IdType id,
std::string_view name)
return true;
}
+bool
+Metrics::Storage::tombstone(Metrics::IdType id, bool set)
+{
+ auto [blob_ix, offset] = _splitID(id);
+ Metrics::NamesAndAtomics *blob = _blobs[blob_ix].get();
+
+ // Only slots that have actually been allocated can be marked.
+ if (!blob || (blob_ix == _cur_blob && offset > _cur_off)) {
+ return false;
+ }
Review Comment:
`Storage::tombstone()` can accept IDs that don't correspond to an allocated
slot (e.g. offset == _cur_off, or any manufactured ID with offset >= MAX_SIZE).
That violates the documented contract ("false if id does not name an allocated
slot") and can also lead to out-of-bounds indexing on the flag array for
invalid offsets.
This issue also appears in the following locations of the same file:
- line 278
- line 296
--
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]
