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


##########
src/tscore/Layout.cc:
##########
@@ -161,15 +161,15 @@ Layout::Layout(std::string_view const _prefix)
       if ((len + 1) > PATH_NAME_MAX) {
         ink_fatal("TS_ROOT environment variable is too big: %d, max %d\n", 
len, PATH_NAME_MAX - 1);
       }
-      path = env_path;
+      path = std::move(env_path);
       while (path.back() == '/') {
         path.pop_back();
       }

Review Comment:
   `path.back()` is called unconditionally after assigning `path` from 
`TS_ROOT`. If `TS_ROOT` is set but empty, `path` becomes empty and 
`path.back()` is undefined behavior. Guard the loop with an emptiness check to 
avoid a potential crash.



##########
src/api/InkAPI.cc:
##########
@@ -8935,7 +8935,7 @@ TSRPCHandlerDone(TSYaml resp)
 {
   Dbg(dbg_ctl_rpc_api, ">> Handler seems to be done");
   std::lock_guard<std::mutex> lock(::rpc::g_rpcHandlingMutex);
-  auto                        data       = *reinterpret_cast<YAML::Node 
*>(resp);
+  auto                       &data       = *reinterpret_cast<YAML::Node 
*>(resp);

Review Comment:
   `TSRPCHandlerDone` only reads the YAML node; using a non-const reference 
from a non-const pointer cast makes it easier to accidentally mutate the node 
(and hides that `resp` is treated as read-only). Cast to `YAML::Node const*` 
and bind a `const` reference instead.



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