Copilot commented on code in PR #13745:
URL: https://github.com/apache/trafficserver/pull/13745#discussion_r4130171095
##########
src/iocore/cache/CacheHosting.cc:
##########
@@ -175,14 +175,14 @@ CacheHostMatcher::NewEntry(matcher_line *line_info)
if (errNo) {
// There was a problem so undo the effects this function
memset(static_cast<void *>(cur_d), 0, sizeof(CacheHostRecord));
- return;
+ return false;
Review Comment:
When `Init` rejects a later volume after accepting an earlier one, `cur_d`
already owns its allocated `cp` array. The preceding `memset` erases that
pointer, so every such rejected host entry leaks memory; repeated bad reloads
can grow the server indefinitely. Destroy and reconstruct the unused array slot
before returning so partial allocations are released and the slot remains
reusable.
##########
src/iocore/cache/CacheHosting.cc:
##########
@@ -508,7 +514,8 @@ CacheHostRecord::Init(matcher_line *line_info, CacheType
typ)
*s = '\0';
volume_number = atoi(vol_no);
- cachep = cp_list.head;
+ is_vol_present = 0;
+ cachep = cp_list.head;
Review Comment:
This stricter loop is also used by `createCacheHostRecord`, so it changes
invalid-list handling for remap `@volume=` and
`proxy.config.cache.default_volumes`, but the new test exercises only
`hosting.config`. Please add cases such as `1,99` for both shared consumers to
verify that each rejects the whole list rather than retaining volume 1 or
silently falling back through an unintended path.
--
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]
