Copilot commented on code in PR #13904:
URL: https://github.com/apache/trafficserver/pull/13904#discussion_r4375930782
##########
src/mgmt/config/ConfigContext.cc:
##########
@@ -27,9 +27,20 @@
#include <yaml-cpp/yaml.h>
-// Defined here (not = default in header) so that YAML::Node ctor/dtor/copy
-// symbols are only emitted in this TU (part of librecords, which links
yaml-cpp).
-// Otherwise every consumer of RecCore.h would need yaml-cpp at link time.
+#include <optional>
+
+// Shared by every copy of a context, so a built Payload is never changed: the
+// setters build a new one instead, because assigning into a YAML::Node writes
+// through to every other handle on the same node. The YAML it holds is still
+// shared, so a write through a returned node reaches every copy.
+struct ConfigContext::Payload {
+ std::optional<YAML::Node> supplied_yaml;
+ std::optional<YAML::Node> reload_directives;
+};
+
+// ConfigContext is passed by value to every reload handler.
+static_assert(sizeof(ConfigContext) <= 64, "ConfigContext is passed by value;
add new state to ConfigContext::Payload");
+
Review Comment:
The comment explaining why these special member functions are defined out of
line was removed, but they remain out of line with no stated reason. `Payload`
is incomplete in the header, and `shared_ptr` does not need its type to be
complete for these members, so the defaults could move to the header. If they
stay in this file, restore a comment that explains why.
##########
src/mgmt/config/ConfigContext.cc:
##########
@@ -27,9 +27,20 @@
#include <yaml-cpp/yaml.h>
-// Defined here (not = default in header) so that YAML::Node ctor/dtor/copy
-// symbols are only emitted in this TU (part of librecords, which links
yaml-cpp).
-// Otherwise every consumer of RecCore.h would need yaml-cpp at link time.
+#include <optional>
+
+// Shared by every copy of a context, so a built Payload is never changed: the
+// setters build a new one instead, because assigning into a YAML::Node writes
+// through to every other handle on the same node. The YAML it holds is still
+// shared, so a write through a returned node reaches every copy.
+struct ConfigContext::Payload {
+ std::optional<YAML::Node> supplied_yaml;
+ std::optional<YAML::Node> reload_directives;
+};
+
+// ConfigContext is passed by value to every reload handler.
+static_assert(sizeof(ConfigContext) <= 64, "ConfigContext is passed by value;
add new state to ConfigContext::Payload");
Review Comment:
"The bound of
--
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]
