hnakamur commented on PR #12060: URL: https://github.com/apache/trafficserver/pull/12060#issuecomment-2697245093
## `RecEstablishStaticConfig*` family needs a pointer to the config value `RecEstablishStaticConfig*` family have to take a pointer since `RecLinkConfig*` family take a pointer. https://github.com/apache/trafficserver/blob/91748d47712482fe39dbdfc5c29d46fc65f7d3ae/include/records/RecCore.h#L116-L122 For example, `RecEstablishStaticConfigStringAlloc` calls `RecLinkConfigString`. https://github.com/hnakamur/trafficserver/blob/4d8bd3638cc0683b12773bedf0a6a27df6e109af/src/records/RecCore.cc#L915-L922 ``` RecErrT RecEstablishStaticConfigStringAlloc(const char *name, RecString *rec_string, bool lock) { if (RecLinkConfigString(name, rec_string) == REC_ERR_OKAY) { ats_free(*rec_string); } return RecGetRecordStringOrNullptr_Xmalloc(name, rec_string, lock); } ``` ## other functions can be changed to return the config value and take a pointer to `RecErrT err` or `bool found` As for other functions, there are few use cases which needs to know wether the config is found or not, for example: https://github.com/hnakamur/trafficserver/blob/4d8bd3638cc0683b12773bedf0a6a27df6e109af/src/mgmt/config/FileManager.cc#L280-L281 ``` int enabled; bool found = RecGetRecordIntOrZero("proxy.config.body_factory.enable_customizations", &enabled) == REC_ERR_OKAY; ``` We can change RecGetRecordIntOrZero to return the config value and take a pointer to RecErrT instead. ``` RecInt RecGetRecordIntOrZero3(const char *name, RecErrT *err = nullptr, bool lock = true); ``` ``` RecInt RecGetRecordIntOrZero2(const char *name, RecErrT *err, bool lock) { RecData data; RecErrT rec_err = RecGetRecord_Xmalloc(name, RECD_INT, &data, lock); if (err) { *err = rec_err; } return rec_err == REC_ERR_OKAY ? data.rec_int : 0; } ``` However we need one more line after rewriting to use this version. ``` RecErrT rec_err; int enabled = RecGetRecordIntOrZero2("proxy.config.body_factory.enable_customizations", &rec_err); bool found = rec_err == REC_ERR_OKAY; ``` Maybe we can take a pointer to bool instead of RecErrT. ``` RecInt RecGetRecordIntOrZero3(const char *name, bool *found = nullptr, bool lock = true); ``` ``` RecInt RecGetRecordIntOrZero3(const char *name, bool *found, bool lock) { RecData data; RecErrT rec_err = RecGetRecord_Xmalloc(name, RECD_INT, &data, lock); if (found) { *found = rec_err == REC_ERR_OKAY; } return rec_err == REC_ERR_OKAY ? data.rec_int : 0; } ``` ``` bool found; int enabled = RecGetRecordIntOrZero2("proxy.config.body_factory.enable_customizations", &found); ``` I welcome other suggestions if any. -- 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]
