You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by wk...@apache.org on 2022/06/28 14:12:38 UTC
[trafficserver] branch master updated: Use std::variant to clean up handling of unmarshal functions. (#8922)
This is an automated email from the ASF dual-hosted git repository.
wkaras 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 7978815a6 Use std::variant to clean up handling of unmarshal functions. (#8922)
7978815a6 is described below
commit 7978815a69ced6a826250cd9cee7f0bec4edbb83
Author: Walt Karas <wk...@verizonmedia.com>
AuthorDate: Tue Jun 28 09:12:31 2022 -0500
Use std::variant to clean up handling of unmarshal functions. (#8922)
Co-authored-by: Walt Karas <wk...@yahooinc.com>
---
proxy/logging/LogAccess.cc | 137 ++++++++++++++++++---------------------------
proxy/logging/LogAccess.h | 6 +-
proxy/logging/LogField.cc | 51 +++++++----------
proxy/logging/LogField.h | 16 +++---
proxy/logging/Makefile.am | 1 +
5 files changed, 86 insertions(+), 125 deletions(-)
diff --git a/proxy/logging/LogAccess.cc b/proxy/logging/LogAccess.cc
index eee6eb919..6116688b3 100644
--- a/proxy/logging/LogAccess.cc
+++ b/proxy/logging/LogAccess.cc
@@ -599,50 +599,6 @@ LogAccess::marshal_proxy_req_all_header_fields(char *buf)
/*-------------------------------------------------------------------------
-------------------------------------------------------------------------*/
-/*-------------------------------------------------------------------------
- LogAccess::unmarshal_str
-
- Retrieve the string from the location pointed at by the buffer and
- advance the pointer past the string. The local strlen function is used
- to advance the pointer, thus matching the corresponding strlen that was
- used to lay the string into the buffer.
- -------------------------------------------------------------------------*/
-
-int
-LogAccess::unmarshal_str(char **buf, char *dest, int len, LogSlice *slice)
-{
- ink_assert(buf != nullptr);
- ink_assert(*buf != nullptr);
- ink_assert(dest != nullptr);
-
- char *val_buf = *buf;
- int val_len = static_cast<int>(::strlen(val_buf));
-
- *buf += LogAccess::strlen(val_buf); // this is how it was stored
-
- if (slice && slice->m_enable) {
- int offset, n;
-
- n = slice->toStrOffset(val_len, &offset);
- if (n <= 0) {
- return 0;
- }
-
- if (n >= len) {
- return -1;
- }
-
- memcpy(dest, (val_buf + offset), n);
- return n;
- }
-
- if (val_len < len) {
- memcpy(dest, val_buf, val_len);
- return val_len;
- }
- return -1;
-}
-
namespace
{
class EscLookup
@@ -694,9 +650,7 @@ nibble(int nib)
return nib >= 0xa ? 'a' + (nib - 0xa) : '0' + nib;
}
-} // end anonymous namespace
-
-static int
+int
escape_json(char *dest, const char *buf, int len)
{
int escaped_len = 0;
@@ -742,12 +696,9 @@ escape_json(char *dest, const char *buf, int len)
}
int
-LogAccess::unmarshal_str_json(char **buf, char *dest, int len, LogSlice *slice)
+unmarshal_str_json(char **buf, char *dest, int len, LogSlice *slice)
{
Debug("log-escape", "unmarshal_str_json start, len=%d, slice=%p", len, slice);
- ink_assert(buf != nullptr);
- ink_assert(*buf != nullptr);
- ink_assert(dest != nullptr);
char *val_buf = *buf;
int val_len = static_cast<int>(::strlen(val_buf));
@@ -778,6 +729,56 @@ LogAccess::unmarshal_str_json(char **buf, char *dest, int len, LogSlice *slice)
return -1;
}
+} // end anonymous namespace
+
+/*-------------------------------------------------------------------------
+ LogAccess::unmarshal_str
+
+ Retrieve the string from the location pointed at by the buffer and
+ advance the pointer past the string. The local strlen function is used
+ to advance the pointer, thus matching the corresponding strlen that was
+ used to lay the string into the buffer.
+ -------------------------------------------------------------------------*/
+
+int
+LogAccess::unmarshal_str(char **buf, char *dest, int len, LogSlice *slice, LogEscapeType escape_type)
+{
+ ink_assert(buf != nullptr);
+ ink_assert(*buf != nullptr);
+ ink_assert(dest != nullptr);
+
+ if (LOG_ESCAPE_JSON == escape_type) {
+ return unmarshal_str_json(buf, dest, len, slice);
+ }
+
+ char *val_buf = *buf;
+ int val_len = static_cast<int>(::strlen(val_buf));
+
+ *buf += LogAccess::strlen(val_buf); // this is how it was stored
+
+ if (slice && slice->m_enable) {
+ int offset, n;
+
+ n = slice->toStrOffset(val_len, &offset);
+ if (n <= 0) {
+ return 0;
+ }
+
+ if (n >= len) {
+ return -1;
+ }
+
+ memcpy(dest, (val_buf + offset), n);
+ return n;
+ }
+
+ if (val_len < len) {
+ memcpy(dest, val_buf, val_len);
+ return val_len;
+ }
+ return -1;
+}
+
int
LogAccess::unmarshal_ttmsf(char **buf, char *dest, int len)
{
@@ -920,7 +921,7 @@ LogAccess::unmarshal_http_version(char **buf, char *dest, int len)
-------------------------------------------------------------------------*/
int
-LogAccess::unmarshal_http_text(char **buf, char *dest, int len, LogSlice *slice)
+LogAccess::unmarshal_http_text(char **buf, char *dest, int len, LogSlice *slice, LogEscapeType escape_type)
{
ink_assert(buf != nullptr);
ink_assert(*buf != nullptr);
@@ -929,41 +930,13 @@ LogAccess::unmarshal_http_text(char **buf, char *dest, int len, LogSlice *slice)
char *p = dest;
// int res1 = unmarshal_http_method (buf, p, len);
- int res1 = unmarshal_str(buf, p, len);
- if (res1 < 0) {
- return -1;
- }
- p += res1;
- *p++ = ' ';
- int res2 = unmarshal_str(buf, p, len - res1 - 1, slice);
- if (res2 < 0) {
- return -1;
- }
- p += res2;
- *p++ = ' ';
- int res3 = unmarshal_http_version(buf, p, len - res1 - res2 - 2);
- if (res3 < 0) {
- return -1;
- }
- return res1 + res2 + res3 + 2;
-}
-
-int
-LogAccess::unmarshal_http_text_json(char **buf, char *dest, int len, LogSlice *slice)
-{
- ink_assert(buf != nullptr);
- ink_assert(*buf != nullptr);
- ink_assert(dest != nullptr);
-
- char *p = dest;
-
- int res1 = unmarshal_str_json(buf, p, len);
+ int res1 = unmarshal_str(buf, p, len, nullptr, escape_type);
if (res1 < 0) {
return -1;
}
p += res1;
*p++ = ' ';
- int res2 = unmarshal_str_json(buf, p, len - res1 - 1, slice);
+ int res2 = unmarshal_str(buf, p, len - res1 - 1, slice, escape_type);
if (res2 < 0) {
return -1;
}
diff --git a/proxy/logging/LogAccess.h b/proxy/logging/LogAccess.h
index db799131c..178e3e247 100644
--- a/proxy/logging/LogAccess.h
+++ b/proxy/logging/LogAccess.h
@@ -304,15 +304,13 @@ public:
static int unmarshal_itox(int64_t val, char *dest, int field_width = 0, char leading_char = ' ');
static int unmarshal_int_to_str(char **buf, char *dest, int len);
static int unmarshal_int_to_str_hex(char **buf, char *dest, int len);
- static int unmarshal_str(char **buf, char *dest, int len, LogSlice *slice = nullptr);
- static int unmarshal_str_json(char **buf, char *dest, int len, LogSlice *slice = nullptr);
+ static int unmarshal_str(char **buf, char *dest, int len, LogSlice *slice, LogEscapeType escape_type);
static int unmarshal_ttmsf(char **buf, char *dest, int len);
static int unmarshal_int_to_date_str(char **buf, char *dest, int len);
static int unmarshal_int_to_time_str(char **buf, char *dest, int len);
static int unmarshal_int_to_netscape_str(char **buf, char *dest, int len);
static int unmarshal_http_version(char **buf, char *dest, int len);
- static int unmarshal_http_text(char **buf, char *dest, int len, LogSlice *slice = nullptr);
- static int unmarshal_http_text_json(char **buf, char *dest, int len, LogSlice *slice = nullptr);
+ static int unmarshal_http_text(char **buf, char *dest, int len, LogSlice *slice, LogEscapeType escape_type);
static int unmarshal_http_status(char **buf, char *dest, int len);
static int unmarshal_ip(char **buf, IpEndpoint *dest);
static int unmarshal_ip_to_str(char **buf, char *dest, int len);
diff --git a/proxy/logging/LogField.cc b/proxy/logging/LogField.cc
index d787ce08e..66495d7fc 100644
--- a/proxy/logging/LogField.cc
+++ b/proxy/logging/LogField.cc
@@ -28,6 +28,7 @@
representation of a logging field.
***************************************************************************/
#include "tscore/ink_platform.h"
+#include "swoc/swoc_meta.h"
#include "MIME.h"
#include "LogUtils.h"
@@ -218,14 +219,19 @@ LogField::init_milestone_container()
}
// Generic field ctor
-LogField::LogField(const char *name, const char *symbol, Type type, MarshalFunc marshal, UnmarshalFunc unmarshal, SetFunc _setfunc)
+LogField::LogField(const char *name, const char *symbol, Type type, MarshalFunc marshal, VarUnmarshalFuncSliceOnly unmarshal,
+ SetFunc _setfunc)
: m_name(ats_strdup(name)),
m_symbol(ats_strdup(symbol)),
m_type(type),
m_container(NO_CONTAINER),
m_marshal_func(marshal),
- m_unmarshal_func(unmarshal),
- m_unmarshal_func_map(nullptr),
+ m_unmarshal_func([](VarUnmarshalFuncSliceOnly const &f) -> VarUnmarshalFunc {
+ if (auto *ff = std::get_if<UnmarshalFunc>(&f); ff) {
+ return *ff;
+ }
+ return std::get<UnmarshalFuncWithSlice>(f);
+ }(unmarshal)),
m_agg_op(NO_AGGREGATE),
m_agg_cnt(0),
m_agg_val(0),
@@ -244,12 +250,6 @@ LogField::LogField(const char *name, const char *symbol, Type type, MarshalFunc
strcmp(m_symbol, "cqtn") == 0 || strcmp(m_symbol, "cqtd") == 0 || strcmp(m_symbol, "cqtt") == 0);
}
-LogField::LogField(const char *name, const char *symbol, Type type, MarshalFunc marshal, UnmarshalFuncWithSlice unmarshal,
- SetFunc _setfunc)
- : LogField(name, symbol, type, marshal, reinterpret_cast<UnmarshalFunc>(unmarshal), _setfunc)
-{
-}
-
LogField::LogField(const char *name, const char *symbol, Type type, MarshalFunc marshal, UnmarshalFuncWithMap unmarshal,
const Ptr<LogFieldAliasMap> &map, SetFunc _setfunc)
: m_name(ats_strdup(name)),
@@ -257,8 +257,7 @@ LogField::LogField(const char *name, const char *symbol, Type type, MarshalFunc
m_type(type),
m_container(NO_CONTAINER),
m_marshal_func(marshal),
- m_unmarshal_func(nullptr),
- m_unmarshal_func_map(unmarshal),
+ m_unmarshal_func(unmarshal),
m_agg_op(NO_AGGREGATE),
m_agg_cnt(0),
m_agg_val(0),
@@ -325,7 +324,6 @@ LogField::LogField(const char *field, Container container, SetFunc _setfunc)
m_container(container),
m_marshal_func(nullptr),
m_unmarshal_func(nullptr),
- m_unmarshal_func_map(nullptr),
m_agg_op(NO_AGGREGATE),
m_agg_cnt(0),
m_agg_val(0),
@@ -354,7 +352,7 @@ LogField::LogField(const char *field, Container container, SetFunc _setfunc)
case ESSH:
case ECSSH:
case SCFG:
- m_unmarshal_func = reinterpret_cast<UnmarshalFunc>(&(LogAccess::unmarshal_str));
+ m_unmarshal_func = &(LogAccess::unmarshal_str);
break;
case ICFG:
@@ -395,7 +393,6 @@ LogField::LogField(const LogField &rhs)
m_container(rhs.m_container),
m_marshal_func(rhs.m_marshal_func),
m_unmarshal_func(rhs.m_unmarshal_func),
- m_unmarshal_func_map(rhs.m_unmarshal_func_map),
m_agg_op(rhs.m_agg_op),
m_agg_cnt(0),
m_agg_val(0),
@@ -600,23 +597,15 @@ LogField::marshal_agg(char *buf)
unsigned
LogField::unmarshal(char **buf, char *dest, int len, LogEscapeType escape_type)
{
- if (!m_alias_map) {
- if (m_unmarshal_func == reinterpret_cast<UnmarshalFunc>(LogAccess::unmarshal_str) ||
- m_unmarshal_func == reinterpret_cast<UnmarshalFunc>(LogAccess::unmarshal_http_text)) {
- UnmarshalFuncWithSlice func = reinterpret_cast<UnmarshalFuncWithSlice>(m_unmarshal_func);
- if (escape_type == LOG_ESCAPE_JSON) {
- if (m_unmarshal_func == reinterpret_cast<UnmarshalFunc>(LogAccess::unmarshal_str)) {
- func = reinterpret_cast<UnmarshalFuncWithSlice>(LogAccess::unmarshal_str_json);
- } else if (m_unmarshal_func == reinterpret_cast<UnmarshalFunc>(LogAccess::unmarshal_http_text)) {
- func = reinterpret_cast<UnmarshalFuncWithSlice>(LogAccess::unmarshal_http_text_json);
- }
- }
- return (*func)(buf, dest, len, &m_slice);
- }
- return (*m_unmarshal_func)(buf, dest, len);
- } else {
- return (*m_unmarshal_func_map)(buf, dest, len, m_alias_map);
- }
+ return std::visit(
+ swoc::meta::vary{[&](UnmarshalFuncWithSlice f) -> unsigned { return (*f)(buf, dest, len, &m_slice, escape_type); },
+ [&](UnmarshalFuncWithMap f) -> unsigned { return (*f)(buf, dest, len, m_alias_map); },
+ [&](UnmarshalFunc f) -> unsigned { return (*f)(buf, dest, len); },
+ [&](decltype(nullptr)) -> unsigned {
+ ink_assert(false);
+ return 0;
+ }},
+ m_unmarshal_func);
}
/*-------------------------------------------------------------------------
diff --git a/proxy/logging/LogField.h b/proxy/logging/LogField.h
index 102175f8c..173a14064 100644
--- a/proxy/logging/LogField.h
+++ b/proxy/logging/LogField.h
@@ -25,6 +25,7 @@
#include <string_view>
#include <string>
+#include <variant>
#include "tscore/ink_platform.h"
#include "tscore/List.h"
@@ -80,10 +81,13 @@ class LogField
public:
typedef int (LogAccess::*MarshalFunc)(char *buf);
typedef int (*UnmarshalFunc)(char **buf, char *dest, int len);
- typedef int (*UnmarshalFuncWithSlice)(char **buf, char *dest, int len, LogSlice *slice);
+ typedef int (*UnmarshalFuncWithSlice)(char **buf, char *dest, int len, LogSlice *slice, LogEscapeType escape_type);
typedef int (*UnmarshalFuncWithMap)(char **buf, char *dest, int len, const Ptr<LogFieldAliasMap> &map);
typedef void (LogAccess::*SetFunc)(char *buf, int len);
+ using VarUnmarshalFuncSliceOnly = std::variant<UnmarshalFunc, UnmarshalFuncWithSlice>;
+ using VarUnmarshalFunc = std::variant<decltype(nullptr), UnmarshalFunc, UnmarshalFuncWithSlice, UnmarshalFuncWithMap>;
+
enum Type {
sINT = 0,
dINT,
@@ -122,10 +126,7 @@ public:
N_AGGREGATES,
};
- LogField(const char *name, const char *symbol, Type type, MarshalFunc marshal, UnmarshalFunc unmarshal,
- SetFunc _setFunc = nullptr);
-
- LogField(const char *name, const char *symbol, Type type, MarshalFunc marshal, UnmarshalFuncWithSlice unmarshal,
+ LogField(const char *name, const char *symbol, Type type, MarshalFunc marshal, VarUnmarshalFuncSliceOnly unmarshal,
SetFunc _setFunc = nullptr);
LogField(const char *name, const char *symbol, Type type, MarshalFunc marshal, UnmarshalFuncWithMap unmarshal,
@@ -194,9 +195,8 @@ private:
char *m_symbol;
Type m_type;
Container m_container;
- MarshalFunc m_marshal_func; // place data into buffer
- UnmarshalFunc m_unmarshal_func; // create a string of the data
- UnmarshalFuncWithMap m_unmarshal_func_map;
+ MarshalFunc m_marshal_func; // place data into buffer
+ VarUnmarshalFunc m_unmarshal_func; // create a string of the data
Aggregate m_agg_op;
int64_t m_agg_cnt;
int64_t m_agg_val;
diff --git a/proxy/logging/Makefile.am b/proxy/logging/Makefile.am
index 061834cca..9dd6ab1d0 100644
--- a/proxy/logging/Makefile.am
+++ b/proxy/logging/Makefile.am
@@ -22,6 +22,7 @@ AM_CPPFLAGS += \
$(iocore_include_dirs) \
-I$(abs_top_srcdir)/include \
-I$(abs_top_srcdir)/lib \
+ -I$(abs_top_srcdir)/lib/swoc/include \
-I$(abs_top_srcdir)/proxy \
-I$(abs_top_srcdir)/proxy/http \
-I$(abs_top_srcdir)/proxy/http/remap \