brbzull0 opened a new pull request, #13904: URL: https://github.com/apache/trafficserver/pull/13904
`ConfigContext` is passed by value to every config reload handler and to the loaders they call. It held two inline `YAML::Node` members, the RPC-supplied content and the reload directives, which make it 144 bytes with libstdc++. Coverity reports each of those by-value parameters as a big parameter passed by value, against its 128-byte threshold. This moves both nodes into one payload held by a `std::shared_ptr`, so the class is a `weak_ptr` to the task plus a `shared_ptr` to the payload: 32 bytes with libstdc++. No signature or call site changes. Coverity CIDs: the 63 PASS_BY_VALUE reports on `ConfigContext` parameters (listed below). **What changes:** - `supplied_yaml()` and `reload_directives()` return the stored node, or a new Undefined node on each call when that field was not supplied. Each field records whether it was set, so a reload with content but no directives does not hand out a shared placeholder node for the directives. - The private setters build a new payload instead of assigning into the shared one: `YAML::Node::operator=` on a node that already exists changes it for every handle that shares it. `add_dependent_ctx()` shares the parent's payload, as it shared the parent's nodes before. - Copy-assignment rebinds the payload. Before, it went through `YAML::Node::operator=`, so assigning to one copy of a context could change another copy. The copy-assignments in the tree, of `LogConfig::reload_ctx` and in the plugin wrapper in `InkAPI.cc`, don't read the old copy afterwards. - `static_assert(sizeof(ConfigContext) <= 64)` in `ConfigContext.cc` keeps new state out of the class. **Tests:** new cases in `test_records` drive RPC content through `ConfigRegistry::execute_reload()` and check content with directives, content only, directives only, copies, copy-assignment, a dependent of a task-less context, and writes into a returned node. A case in `test_ConfigReloadProgress` checks that a dependent context shares its parent's content during a live reload. Four of the new cases fail with the old layout. **What this PR does not do:** - Change any handler or loader to take `ConfigContext` by reference. At 32 bytes the by-value parameters are below Coverity's threshold, so the signatures stay as they are. <details> <summary>CIDs</summary> 1658783, 1658784, 1658785, 1658786, 1658787, 1658788, 1658789, 1658790, 1658791, 1658792, 1658793, 1658794, 1658795, 1658796, 1658797, 1658798, 1658800, 1658801, 1658802, 1658803, 1658804, 1658805, 1658806, 1658807, 1658808, 1658809, 1658811, 1658813, 1658814, 1658815, 1658816, 1658817, 1658818, 1658819, 1658820, 1658821, 1658822, 1658823, 1658824, 1658825, 1658826, 1658827, 1658829, 1658830, 1658831, 1658832, 1658833, 1658834, 1658835, 1658838, 1658840, 1685940, 1687093, 1687094, 1687095, 1687096, 1687097, 1687099, 1687100, 1687101, 1687102, 1687103, 1687105 </details> -- 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]
