Copilot commented on code in PR #13269:
URL: https://github.com/apache/trafficserver/pull/13269#discussion_r3424154354
##########
include/cripts/Urls.hpp:
##########
@@ -485,6 +504,25 @@ class Url
_state->standalone = true;
}
+ // Query owns a std::unique_ptr<State>, which deletes the implicit copy
operations.
+ // NOTE: not a true deep copy — `_owner` is shared, so Flush() on a copy
writes back
+ // to the source URL via TSUrlHttpQuerySet. Treat copies as read-only
snapshots.
+ Query(const self_type &o) : Component(o), _state(o._state ?
std::make_unique<State>(*o._state) : nullptr) {}
Review Comment:
Same as Path: the “read-only snapshots” wording is misleading because copies
still share `_owner` and can write through via Flush()/operator=, and the
copied State still references underlying TS buffers. Rewording will help avoid
consumers assuming an independent snapshot.
##########
include/cripts/Urls.hpp:
##########
@@ -335,6 +335,25 @@ class Url
using Component::Component;
+ // Path owns a std::unique_ptr<State>, which deletes the implicit copy
operations.
+ // NOTE: not a true deep copy — `_owner` is shared, so Flush() on a copy
writes back
+ // to the source URL via TSUrlPathSet. Treat copies as read-only snapshots.
+ Path(const self_type &o) : Component(o), _state(o._state ?
std::make_unique<State>(*o._state) : nullptr) {}
Review Comment:
The comment suggests copies should be treated as “read-only snapshots”, but
the type still exposes write-through operations (e.g. Flush()/operator=) that
will update the shared underlying URL via `_owner`. Consider rewording to
clarify that only the parsed `State` is deep-copied; the URL owner and
underlying TS buffers are still shared and can be invalidated by URL mutations.
--
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]
