[trafficserver] branch master updated: Update overridable config conversion logic. * Improve the constructors for the converter. * Better naming of the converter members. * Add some documentation for the converter implementation.

[am...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

amc pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/master by this push:
     new ba0e6d0  Update overridable config conversion logic. *  Improve the constructors for the converter. *  Better naming of the converter members. *  Add some documentation for the converter implementation.
ba0e6d0 is described below

commit ba0e6d0fa7cab89026b4c4e9182cdbfde0f0701e
Author: Alan M. Carroll <am...@apache.org>
AuthorDate: Thu Jan 31 10:26:57 2019 -0600

    Update overridable config conversion logic.
    *  Improve the constructors for the converter.
    *  Better naming of the converter members.
    *  Add some documentation for the converter implementation.
---
 CMakeLists.txt                         |  2 +-
 doc/developer-guide/config-vars.en.rst | 26 ++++++++++
 mgmt/MgmtDefs.h                        | 89 ++++++++++++++++++++++++++++------
 proxy/http/HttpConnectionCount.cc      |  8 +--
 src/traffic_server/InkAPI.cc           | 54 +++++++--------------
 5 files changed, 122 insertions(+), 57 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 9eb3dd0..efaa9f8 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -122,4 +122,4 @@ file(GLOB plugin_files
         )
 add_library(plugins SHARED ${plugin_files})
 
-add_custom_target(clang-format WORKING_DIRECTORY ${CMAKE_HOME_DIRECTORY} COMMAND make clang-format)
+add_custom_target(clang-format WORKING_DIRECTORY ${CMAKE_HOME_DIRECTORY} COMMAND make -j clang-format)
diff --git a/doc/developer-guide/config-vars.en.rst b/doc/developer-guide/config-vars.en.rst
index 5e85851..5fa8808 100644
--- a/doc/developer-guide/config-vars.en.rst
+++ b/doc/developer-guide/config-vars.en.rst
@@ -322,3 +322,29 @@ required for generic access:
 #. Update the Lua plugin enumeration ``TSLuaOverridableConfigKey`` in |ts_lua_http_config.c|_.
 
 #. Update the documentation of :ref:`ts-overridable-config` in |TSHttpOverridableConfig.en.rst|_.
+
+API conversions
+---------------
+
+A relatively new feature for overridable variables is the ability to keep them in more natural data
+types and convert as needed to the API types. This in turns enables defining the configuration
+locally in a module and then "exporting" it to the API interface. Modules then do not have to
+include headers for all types in all overridable configurations.
+
+The conversion is done through an instance of :code:`MgmtConverter`. This has 6 points to
+conversions, a load and store function for each of the types :code:`MgmtInt`, :code:`MgmtFloat`, and
+:code:`MgmtInt`. The :code:`MgmtByte` type is handled by the :code:`MgmtInt` conversions. In general
+each overridable variable will specify two of these, a load and store for a specific type, although
+it is possible to provide other pairs, e.g. if a value is an enumeration can should be settable
+as a string as well as an integer.
+
+The module is responsible for creating an instance of :code:`MgmtConverter` with the appropriate
+load / store function pairs set. The declaration must be visible in the :ts:git:`proxy/InkAPI.cc`
+file. The function :code:`_conf_to_memberp` sets up the conversion. For the value of the enumeration
+:c:type:`TSOverridableConfigKey` that specifies the overridable variable, code is added to specify
+the member and the conversion. There are default converters for the API types and if the overridable
+is one of those, it is only necessary to call :code:`_memberp_to_generic` passing in a pointer to
+the variable. For a variable with conversion, :arg:`ret` should be set to point to the variable and
+:arg:`conv` set to point to the converter for that variable. If multiple variables are of the same
+type they can use the same converter because a pointer to the specific member is passed to the
+converter.
diff --git a/mgmt/MgmtDefs.h b/mgmt/MgmtDefs.h
index 84424c4..ed8f681 100644
--- a/mgmt/MgmtDefs.h
+++ b/mgmt/MgmtDefs.h
@@ -50,7 +50,7 @@ typedef enum {
  * MgmtCallback
  *   Management Callback functions.
  */
-typedef void *(*MgmtCallback)(void *opaque_cb_data, char *data_raw, int data_len);
+using MgmtCallback = void *(*)(void *opaque_cb_data, char *data_raw, int data_len);
 
 //-------------------------------------------------------------------------
 // API conversion functions.
@@ -66,19 +66,80 @@ typedef void *(*MgmtCallback)(void *opaque_cb_data, char *data_raw, int data_len
  * in this header.
  */
 struct MgmtConverter {
-  // MgmtInt conversions.
-  std::function<MgmtInt(void *)> get_int{nullptr};
-  std::function<void(void *, MgmtInt)> set_int{nullptr};
-
-  // MgmtFloat conversions.
-  std::function<MgmtFloat(void *)> get_float{nullptr};
-  std::function<void(void *, MgmtFloat)> set_float{nullptr};
-
-  // MgmtString conversions.
-  // This is a bit different because it takes std::string_view instead of MgmtString but that's
-  // worth the difference.
-  std::function<std::string_view(void *)> get_string{nullptr};
-  std::function<void(void *, std::string_view)> set_string{nullptr};
+  /** Load a native type into a @c MgmtInt
+   *
+   * This is passed a @c void* which is a pointer to the member in the configuration instance.
+   * This function must return a @c MgmtInt converted from that value.
+   */
+  MgmtInt (*load_int)(void *) = nullptr;
+
+  /** Store a @c MgmtInt into a native type.
+   *
+   * This function is passed a @c void* which is a pointer to the member in the configuration
+   * instance and a @c MgmtInt. The member should be updated to correspond to the @c MgmtInt value.
+   */
+  void (*store_int)(void *, MgmtInt) = nullptr;
+
+  /** Load a @c MgmtFloat from a native type.
+   *
+   * This is passed a @c void* which is a pointer to the member in the configuration instance.
+   * This function must return a @c MgmtFloat converted from that value.
+   */
+  MgmtFloat (*load_float)(void *) = nullptr;
+
+  /** Store a @c MgmtFloat into a native type.
+   *
+   * This function is passed a @c void* which is a pointer to the member in the configuration
+   * instance and a @c MgmtFloat. The member should be updated to correspond to the @c MgmtFloat value.
+   */
+  void (*store_float)(void *, MgmtFloat) = nullptr;
+
+  /** Load a native type into view.
+   *
+   * This is passed a @c void* which is a pointer to the member in the configuration instance.
+   * This function must return a @c string_view which contains the text for the member.
+   */
+  std::string_view (*load_string)(void *) = nullptr;
+
+  /** Store a view in a native type.
+   *
+   * This is passed a @c void* which is a pointer to the member in the configuration instance.
+   * This function must return a @c string_view which contains the text for the member.
+   */
+  void (*store_string)(void *, std::string_view) = nullptr;
+
+  // Convenience constructors because generally only one pair is valid.
+  MgmtConverter(MgmtInt (*load)(void *), void (*store)(void *, MgmtInt));
+  MgmtConverter(MgmtFloat (*load)(void *), void (*store)(void *, MgmtFloat));
+  MgmtConverter(std::string_view (*load)(void *), void (*store)(void *, std::string_view));
+
+  MgmtConverter(MgmtInt (*_load_int)(void *), void (*_store_int)(void *, MgmtInt), MgmtFloat (*_load_float)(void *),
+                void (*_store_float)(void *, MgmtFloat), std::string_view (*_load_string)(void *),
+                void (*_store_string)(void *, std::string_view));
 };
 
+inline MgmtConverter::MgmtConverter(MgmtInt (*load)(void *), void (*store)(void *, MgmtInt)) : load_int(load), store_int(store) {}
+
+inline MgmtConverter::MgmtConverter(MgmtFloat (*load)(void *), void (*store)(void *, MgmtFloat))
+  : load_float(load), store_float(store)
+{
+}
+
+inline MgmtConverter::MgmtConverter(std::string_view (*load)(void *), void (*store)(void *, std::string_view))
+  : load_string(load), store_string(store)
+{
+}
+
+inline MgmtConverter::MgmtConverter(MgmtInt (*_load_int)(void *), void (*_store_int)(void *, MgmtInt),
+                                    MgmtFloat (*_load_float)(void *), void (*_store_float)(void *, MgmtFloat),
+                                    std::string_view (*_load_string)(void *), void (*_store_string)(void *, std::string_view))
+  : load_int(_load_int),
+    store_int(_store_int),
+    load_float(_load_float),
+    store_float(_store_float),
+    load_string(_load_string),
+    store_string(_store_string)
+{
+}
+
 #define LM_CONNECTION_SERVER "processerver.sock"
diff --git a/proxy/http/HttpConnectionCount.cc b/proxy/http/HttpConnectionCount.cc
index 3b59635..807f6c9 100644
--- a/proxy/http/HttpConnectionCount.cc
+++ b/proxy/http/HttpConnectionCount.cc
@@ -33,13 +33,9 @@ OutboundConnTrack::Imp OutboundConnTrack::_imp;
 
 OutboundConnTrack::GlobalConfig *OutboundConnTrack::_global_config{nullptr};
 
-const MgmtConverter OutboundConnTrack::MAX_CONV{
+const MgmtConverter OutboundConnTrack::MAX_CONV(
   [](void *data) -> MgmtInt { return static_cast<MgmtInt>(*static_cast<decltype(TxnConfig::max) *>(data)); },
-  [](void *data, MgmtInt i) -> void { *static_cast<decltype(TxnConfig::max) *>(data) = static_cast<decltype(TxnConfig::max)>(i); },
-  nullptr,
-  nullptr,
-  nullptr,
-  nullptr};
+  [](void *data, MgmtInt i) -> void { *static_cast<decltype(TxnConfig::max) *>(data) = static_cast<decltype(TxnConfig::max)>(i); });
 
 // Do integer and string conversions.
 const MgmtConverter OutboundConnTrack::MATCH_CONV{
diff --git a/src/traffic_server/InkAPI.cc b/src/traffic_server/InkAPI.cc
index b7c540f..dd8f96c 100644
--- a/src/traffic_server/InkAPI.cc
+++ b/src/traffic_server/InkAPI.cc
@@ -7971,32 +7971,14 @@ template <typename T>
 inline void *
 _memberp_to_generic(T *ptr, MgmtConverter const *&conv)
 {
-  static const MgmtConverter IntConverter{
-    [](void *data) -> MgmtInt { return *static_cast<MgmtInt *>(data); },
-    [](void *data, MgmtInt i) -> void { *static_cast<MgmtInt *>(data) = i; },
-    nullptr,
-    nullptr, // float
-    nullptr,
-    nullptr // string
-  };
+  static const MgmtConverter IntConverter([](void *data) -> MgmtInt { return *static_cast<MgmtInt *>(data); },
+                                          [](void *data, MgmtInt i) -> void { *static_cast<MgmtInt *>(data) = i; });
 
-  static const MgmtConverter ByteConverter{
-    [](void *data) -> MgmtInt { return *static_cast<MgmtByte *>(data); },
-    [](void *data, MgmtInt i) -> void { *static_cast<MgmtByte *>(data) = i; },
-    nullptr,
-    nullptr, // float
-    nullptr,
-    nullptr // string
-  };
+  static const MgmtConverter ByteConverter{[](void *data) -> MgmtInt { return *static_cast<MgmtByte *>(data); },
+                                           [](void *data, MgmtInt i) -> void { *static_cast<MgmtByte *>(data) = i; }};
 
-  static const MgmtConverter FloatConverter{
-    nullptr, // int
-    nullptr,
-    [](void *data) -> MgmtFloat { return *static_cast<MgmtFloat *>(data); },
-    [](void *data, MgmtFloat f) -> void { *static_cast<MgmtFloat *>(data) = f; },
-    nullptr,
-    nullptr // string
-  };
+  static const MgmtConverter FloatConverter{[](void *data) -> MgmtFloat { return *static_cast<MgmtFloat *>(data); },
+                                            [](void *data, MgmtFloat f) -> void { *static_cast<MgmtFloat *>(data) = f; }};
 
   // For now, strings are special.
 
@@ -8398,11 +8380,11 @@ TSHttpTxnConfigIntSet(TSHttpTxn txnp, TSOverridableConfigKey conf, TSMgmtInt val
 
   void *dest = _conf_to_memberp(conf, s->t_state.txn_conf, conv);
 
-  if (!dest || !conv->set_int) {
+  if (!dest || !conv->store_int) {
     return TS_ERROR;
   }
 
-  conv->set_int(dest, value);
+  conv->store_int(dest, value);
 
   return TS_SUCCESS;
 }
@@ -8417,11 +8399,11 @@ TSHttpTxnConfigIntGet(TSHttpTxn txnp, TSOverridableConfigKey conf, TSMgmtInt *va
   MgmtConverter const *conv;
   void *src = _conf_to_memberp(conf, s->t_state.txn_conf, conv);
 
-  if (!src || !conv->get_int) {
+  if (!src || !conv->load_int) {
     return TS_ERROR;
   }
 
-  *value = conv->get_int(src);
+  *value = conv->load_int(src);
 
   return TS_SUCCESS;
 }
@@ -8438,11 +8420,11 @@ TSHttpTxnConfigFloatSet(TSHttpTxn txnp, TSOverridableConfigKey conf, TSMgmtFloat
 
   void *dest = _conf_to_memberp(conf, s->t_state.txn_conf, conv);
 
-  if (!dest || !conv->set_float) {
+  if (!dest || !conv->store_float) {
     return TS_ERROR;
   }
 
-  conv->set_float(dest, value);
+  conv->store_float(dest, value);
 
   return TS_SUCCESS;
 }
@@ -8456,10 +8438,10 @@ TSHttpTxnConfigFloatGet(TSHttpTxn txnp, TSOverridableConfigKey conf, TSMgmtFloat
   MgmtConverter const *conv;
   void *src = _conf_to_memberp(conf, reinterpret_cast<HttpSM *>(txnp)->t_state.txn_conf, conv);
 
-  if (!src || !conv->get_float) {
+  if (!src || !conv->load_float) {
     return TS_ERROR;
   }
-  *value = conv->get_float(src);
+  *value = conv->load_float(src);
 
   return TS_SUCCESS;
 }
@@ -8552,8 +8534,8 @@ TSHttpTxnConfigStringSet(TSHttpTxn txnp, TSOverridableConfigKey conf, const char
   default: {
     MgmtConverter const *conv;
     void *dest = _conf_to_memberp(conf, s->t_state.txn_conf, conv);
-    if (dest != nullptr && conv != nullptr && conv->set_string) {
-      conv->set_string(dest, std::string_view(value, length));
+    if (dest != nullptr && conv != nullptr && conv->store_string) {
+      conv->store_string(dest, std::string_view(value, length));
     } else {
       return TS_ERROR;
     }
@@ -8589,8 +8571,8 @@ TSHttpTxnConfigStringGet(TSHttpTxn txnp, TSOverridableConfigKey conf, const char
   default: {
     MgmtConverter const *conv;
     void *src = _conf_to_memberp(conf, sm->t_state.txn_conf, conv);
-    if (src != nullptr && conv != nullptr && conv->get_string) {
-      auto sv = conv->get_string(src);
+    if (src != nullptr && conv != nullptr && conv->load_string) {
+      auto sv = conv->load_string(src);
       *value  = sv.data();
       *length = sv.size();
     } else {