Copilot commented on code in PR #13927:
URL: https://github.com/apache/trafficserver/pull/13927#discussion_r4443234340


##########
src/iocore/cache/CacheVC.cc:
##########
@@ -853,8 +853,12 @@ CacheVC::scanObject(int /* event ATS_UNUSED */, Event * /* 
e ATS_UNUSED */)
         }
       }
       earliest_key = key;
-      int result1  = 
_action.continuation->handleEvent(CACHE_EVENT_SCAN_OBJECT, vector.get(i));
-      switch (result1) {
+      auto result1 = _action.call_back(CACHE_EVENT_SCAN_OBJECT, vector.get(i));
+      // The handler may have cancelled the scan.
+      if (_action.cancelled) {
+        goto Lcancel;
+      }

Review Comment:
   This dereferences `result1` without checking it holds a value. If 
`_action.call_back(...)` can return an empty/invalid result (e.g., due to no 
continuation or internal suppression), `switch (*result1)` can crash. Consider 
explicitly handling an empty result (e.g., treat it as cancellation or map to a 
default `CACHE_SCAN_RESULT_*`) before dereferencing.



##########
src/iocore/cache/CacheVC.cc:
##########
@@ -973,11 +977,18 @@ CacheVC::scanOpenWrite(int /* event ATS_UNUSED */, Event 
* /* e ATS_UNUSED */)
 {
   Dbg(dbg_ctl_cache_scan_truss, "inside %p:scanOpenWrite", this);
   cancel_trigger();
+  if (_action.cancelled) {
+    return free_CacheVC(this);
+  }
   // get volume lock
   if (writer_lock_retry > SCAN_WRITER_LOCK_MAX_RETRY) {
-    int r = 
_action.continuation->handleEvent(CACHE_EVENT_SCAN_OPERATION_BLOCKED, nullptr);
+    auto r = _action.call_back(CACHE_EVENT_SCAN_OPERATION_BLOCKED, nullptr);
     Dbg(dbg_ctl_cache_scan, "still haven't got the writer lock, asking 
user..");
-    switch (r) {
+    // The handler may have cancelled the scan.
+    if (_action.cancelled) {
+      return free_CacheVC(this);
+    }
+    switch (*r) {
     case CACHE_SCAN_RESULT_RETRY:

Review Comment:
   Similar to `scanObject`, this dereferences `r` without verifying it is 
valid/engaged. If `_action.call_back(...)` can yield an empty result, `switch 
(*r)` risks a crash. Add an explicit check for an empty result and decide on a 
safe fallback (e.g., cancel/free, or default to 
`CACHE_SCAN_RESULT_CONTINUE/STOP`).



-- 
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]

Reply via email to