From 8f4f88d12e4f71798c560d57d580b447a7a91d9e Mon Sep 17 00:00:00 2001 From: Ze Gan Date: Tue, 22 Dec 2020 22:31:41 +0800 Subject: [PATCH 01/17] Add utility for string and redis Signed-off-by: Ze Gan --- common/Makefile.am | 3 +- common/rediscommand.h | 1 + common/redisutility.h | 55 +++++++++++++++++++++ common/stringutility.cpp | 97 ++++++++++++++++++++++++++++++++++++++ common/stringutility.h | 75 +++++++++++++++++++++++++++++ tests/Makefile.am | 2 + tests/redisutility_ut.cpp | 26 ++++++++++ tests/stringutility_ut.cpp | 83 ++++++++++++++++++++++++++++++++ 8 files changed, 341 insertions(+), 1 deletion(-) create mode 100644 common/redisutility.h create mode 100644 common/stringutility.cpp create mode 100644 common/stringutility.h create mode 100644 tests/redisutility_ut.cpp create mode 100644 tests/stringutility_ut.cpp diff --git a/common/Makefile.am b/common/Makefile.am index a6231cb5..4a28269a 100644 --- a/common/Makefile.am +++ b/common/Makefile.am @@ -58,7 +58,8 @@ libswsscommon_la_SOURCES = \ exec.cpp \ subscriberstatetable.cpp \ timestamp.cpp \ - warm_restart.cpp + warm_restart.cpp \ + stringutility.cpp libswsscommon_la_CXXFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(LIBNL_CFLAGS) libswsscommon_la_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(LIBNL_CPPFLAGS) diff --git a/common/rediscommand.h b/common/rediscommand.h index 56821732..e5f2840e 100644 --- a/common/rediscommand.h +++ b/common/rediscommand.h @@ -2,6 +2,7 @@ #include #include #include +#include namespace swss { diff --git a/common/redisutility.h b/common/redisutility.h new file mode 100644 index 00000000..ed1ab87c --- /dev/null +++ b/common/redisutility.h @@ -0,0 +1,55 @@ +#ifndef __REDISUTILITY__ +#define __REDISUTILITY__ + +#include "rediscommand.h" +#include "logger.h" +#include "stringutility.h" + +#include +#include + +namespace swss +{ + +template +static bool get_value( + const std::vector &fvt, + const std::string &field, + T &value) +{ + SWSS_LOG_ENTER(); + + std::string target_field = field; + std::transform( + target_field.begin(), + target_field.end(), + target_field.begin(), + ::tolower); + auto itr = std::find_if( + fvt.begin(), + fvt.end(), + [&](const std::vector::value_type &entry) { + std::string available_field = fvField(entry); + std::transform( + available_field.begin(), + available_field.end(), + available_field.begin(), + ::tolower); + return available_field == target_field; + }); + if (itr != fvt.end()) + { + std::istringstream istream(fvValue(*itr)); + istream >> value; + SWSS_LOG_DEBUG( + "Set field '%s' as '%s'", + field.c_str(), + fvValue(*itr).c_str()); + return true; + } + SWSS_LOG_DEBUG("Cannot find field : %s", field.c_str()); + return false; +} +} + +#endif \ No newline at end of file diff --git a/common/stringutility.cpp b/common/stringutility.cpp new file mode 100644 index 00000000..955219c5 --- /dev/null +++ b/common/stringutility.cpp @@ -0,0 +1,97 @@ +#include "stringutility.h" + +#include +#include + +std::istringstream &swss::operator>>( + std::istringstream &istream, + bool &b) +{ + std::string buffer = istream.str(); + std::transform( + buffer.begin(), + buffer.end(), + buffer.begin(), + ::tolower); + if (buffer == "true" || buffer == "1") + { + b = true; + } + else if (buffer == "false" || buffer == "0") + { + b = false; + } + else + { + throw std::invalid_argument("Invalid bool string : " + buffer); + } + + return istream; +} + +bool swss::hex_to_binary( + const std::string &hex_str, + std::uint8_t *buffer, + size_t buffer_length) +{ + SWSS_LOG_ENTER(); + + if (hex_str.length() % 2 != 0) + { + SWSS_LOG_ERROR("Invalid hex string %s", hex_str.c_str()); + return false; + } + + if (hex_str.length() > (buffer_length * 2)) + { + SWSS_LOG_ERROR("Buffer length isn't sufficient"); + return false; + } + + size_t buffer_cur = 0; + size_t hex_cur = 0; + unsigned char *output = static_cast(buffer); + + while (hex_cur < hex_str.length()) + { + const char temp_buffer[] = { hex_str[hex_cur], hex_str[hex_cur + 1], 0 }; + unsigned int value = -1; + + if (sscanf(temp_buffer, "%X", &value) <= 0 || value > 0xff) + { + SWSS_LOG_ERROR("Invalid hex string %s", temp_buffer); + return false; + } + + output[buffer_cur] = static_cast(value); + hex_cur += 2; + buffer_cur += 1; + } + + return true; +} + +std::string swss::binary_to_hex(const void *buffer, size_t length) +{ + SWSS_LOG_ENTER(); + + std::string s; + + if (buffer == NULL || length == 0) + { + return s; + } + + s.resize(2 * length, '0'); + + const unsigned char *input = static_cast(buffer); + char *output = &s[0]; + + for (size_t i = 0; i < length; i++) + { + snprintf(&output[i * 2], 3, "%02X", input[i]); + } + + return s; +} + diff --git a/common/stringutility.h b/common/stringutility.h new file mode 100644 index 00000000..4d5f3603 --- /dev/null +++ b/common/stringutility.h @@ -0,0 +1,75 @@ +#ifndef __STRINGUTILITY__ +#define __STRINGUTILITY__ + +#include "logger.h" + +#include +#include +#include + +namespace swss { + +template +bool split(const std::string &input, char delimiter, T &output) +{ + SWSS_LOG_ENTER(); + if (input.find(delimiter) != std::string::npos) + { + return false; + } + std::istringstream istream(input); + istream >> output; + return true; +} + +template +bool split(const std::string &input, char delimiter, T &output, Args &... args) +{ + SWSS_LOG_ENTER(); + auto pos = input.find(delimiter); + if (pos == std::string::npos) + { + return false; + } + std::istringstream istream(input.substr(0, pos)); + istream >> output; + return split(input.substr(pos + 1, input.length() - pos - 1), delimiter, args...); +} + +template +std::string join(char delimiter, const T &input) +{ + SWSS_LOG_ENTER(); + std::ostringstream ostream; + ostream << input; + return ostream.str(); +} + +template +std::string join(char delimiter, const T &input, const Args &... args) +{ + SWSS_LOG_ENTER(); + std::ostringstream ostream; + ostream << input << delimiter << join(delimiter, args...); + return ostream.str(); +} + +std::istringstream &operator>>(std::istringstream &istream, bool &b); + +bool hex_to_binary(const std::string &hex_str, std::uint8_t *buffer, size_t buffer_length); + +template +void hex_to_binary( + const std::string &s, + T &value) +{ + SWSS_LOG_ENTER(); + + return hex_to_binary(s, &value, sizeof(T)); +} + +std::string binary_to_hex(const void *buffer, size_t length); + +} + +#endif \ No newline at end of file diff --git a/tests/Makefile.am b/tests/Makefile.am index a059ac79..4882b688 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -29,6 +29,8 @@ tests_SOURCES = redis_ut.cpp \ redis_multi_db_ut.cpp \ logger_ut.cpp \ fdb_flush.cpp \ + stringutility_ut.cpp \ + redisutility_ut.cpp \ main.cpp tests_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(LIBNL_CFLAGS) diff --git a/tests/redisutility_ut.cpp b/tests/redisutility_ut.cpp new file mode 100644 index 00000000..73e16c8d --- /dev/null +++ b/tests/redisutility_ut.cpp @@ -0,0 +1,26 @@ +#include "common/redisutility.h" + +#include "gtest/gtest.h" + +TEST(REDISUTILITY, get_value) +{ + std::vector fvt; + fvt.push_back(std::make_pair("int", "123")); + fvt.push_back(std::make_pair("bool", "true")); + fvt.push_back(std::make_pair("string", "name")); + + int i; + EXPECT_TRUE(swss::get_value(fvt, "int", i)); + EXPECT_EQ(i, 123); + + bool b; + EXPECT_TRUE(swss::get_value(fvt, "bool", b)); + EXPECT_EQ(b, true); + + std::string s; + EXPECT_TRUE(swss::get_value(fvt, "string", s)); + EXPECT_EQ(s, "name"); + + double d; + EXPECT_FALSE(swss::get_value(fvt, "double", d)); +} diff --git a/tests/stringutility_ut.cpp b/tests/stringutility_ut.cpp new file mode 100644 index 00000000..7b34ade5 --- /dev/null +++ b/tests/stringutility_ut.cpp @@ -0,0 +1,83 @@ +#include "common/stringutility.h" +#include "common/ipaddress.h" + +#include "gtest/gtest.h" + +#include +#include + +TEST(STRINGUTILITY, split_int) +{ + int i; + + EXPECT_TRUE(swss::split("123", ':', i)); + EXPECT_EQ(i, 123); + + EXPECT_TRUE(swss::split("0", ':', i)); + EXPECT_EQ(i, 0); + + EXPECT_TRUE(swss::split("-123", ':', i)); + EXPECT_EQ(i, -123); + + EXPECT_FALSE(swss::split("123:", ':', i)); +} + +TEST(STRINGUTILITY, split_bool) +{ + bool b; + + EXPECT_TRUE(swss::split("true", ':', b)); + EXPECT_EQ(b, true); + + EXPECT_TRUE(swss::split("TRUE", ':', b)); + EXPECT_EQ(b, true); + + EXPECT_TRUE(swss::split("1", ':', b)); + EXPECT_EQ(b, true); + + EXPECT_TRUE(swss::split("false", ':', b)); + EXPECT_EQ(b, false); + + EXPECT_TRUE(swss::split("0", ':', b)); + EXPECT_EQ(b, false); + + EXPECT_THROW(swss::split("123", ':', b), std::invalid_argument); +} + +TEST(STRINGUTILITY, split_mix) +{ + int i; + bool b; + std::string s; + + EXPECT_TRUE(swss::split("123:true:name", ':', i, b, s)); + EXPECT_EQ(i, 123); + EXPECT_EQ(b, true); + EXPECT_EQ("name", s); +} + +TEST(STRINGUTILITY, join) +{ + auto str = swss::join(':', 123, true, std::string("name")); + EXPECT_EQ(str, "123:1:name"); +} + +TEST(STRINGUTILITY, hex_to_binary) +{ + std::array a; + + EXPECT_TRUE(swss::hex_to_binary("01020aff05", a.data(), a.size())); + EXPECT_EQ(a, (std::array{0x1, 0x2, 0x0a, 0xff, 0x5})); + + EXPECT_TRUE(swss::hex_to_binary("01020AFF05", a.data(), a.size())); + EXPECT_EQ(a, (std::array{0x1, 0x2, 0x0A, 0xFF, 0x5})); + + EXPECT_FALSE(swss::hex_to_binary("01020", a.data(), a.size())); + EXPECT_FALSE(swss::hex_to_binary("xxxx", a.data(), a.size())); +} + +TEST(STRINGUTILITY, binary_to_hex) +{ + std::array a{0x1, 0x2, 0x0a, 0xff, 0x5}; + EXPECT_EQ(swss::binary_to_hex(a.data(), a.size()), "01020AFF05"); +} From cd10f060a91c9fb880e02fa278d7c397b5cf3f96 Mon Sep 17 00:00:00 2001 From: Ze Gan Date: Wed, 23 Dec 2020 11:11:07 +0800 Subject: [PATCH 02/17] Polish code Signed-off-by: Ze Gan --- common/redisutility.h | 5 +---- common/stringutility.cpp | 1 - common/stringutility.h | 9 +++------ 3 files changed, 4 insertions(+), 11 deletions(-) diff --git a/common/redisutility.h b/common/redisutility.h index ed1ab87c..ea5fb032 100644 --- a/common/redisutility.h +++ b/common/redisutility.h @@ -1,5 +1,4 @@ -#ifndef __REDISUTILITY__ -#define __REDISUTILITY__ +#pragma once #include "rediscommand.h" #include "logger.h" @@ -51,5 +50,3 @@ static bool get_value( return false; } } - -#endif \ No newline at end of file diff --git a/common/stringutility.cpp b/common/stringutility.cpp index 955219c5..46b3da37 100644 --- a/common/stringutility.cpp +++ b/common/stringutility.cpp @@ -94,4 +94,3 @@ std::string swss::binary_to_hex(const void *buffer, size_t length) return s; } - diff --git a/common/stringutility.h b/common/stringutility.h index 4d5f3603..1bb2533c 100644 --- a/common/stringutility.h +++ b/common/stringutility.h @@ -1,5 +1,4 @@ -#ifndef __STRINGUTILITY__ -#define __STRINGUTILITY__ +#pragma once #include "logger.h" @@ -23,7 +22,7 @@ bool split(const std::string &input, char delimiter, T &output) } template -bool split(const std::string &input, char delimiter, T &output, Args &... args) + bool split(const std::string &input, char delimiter, T &output, Args &... args) { SWSS_LOG_ENTER(); auto pos = input.find(delimiter); @@ -37,7 +36,7 @@ bool split(const std::string &input, char delimiter, T &output, Args &... args) } template -std::string join(char delimiter, const T &input) +std::string join(__attribute__((unused)) char delimiter, const T &input) { SWSS_LOG_ENTER(); std::ostringstream ostream; @@ -71,5 +70,3 @@ void hex_to_binary( std::string binary_to_hex(const void *buffer, size_t length); } - -#endif \ No newline at end of file From d1ed78ebb6b3a1b6b228b7fd7fdada937fbf3954 Mon Sep 17 00:00:00 2001 From: Ze Gan Date: Wed, 23 Dec 2020 11:13:22 +0800 Subject: [PATCH 03/17] Renmae get_value to fvGetValue Signed-off-by: Ze Gan --- common/redisutility.h | 2 +- tests/redisutility_ut.cpp | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/common/redisutility.h b/common/redisutility.h index ea5fb032..f929f1d4 100644 --- a/common/redisutility.h +++ b/common/redisutility.h @@ -11,7 +11,7 @@ namespace swss { template -static bool get_value( +static bool fvGetValue( const std::vector &fvt, const std::string &field, T &value) diff --git a/tests/redisutility_ut.cpp b/tests/redisutility_ut.cpp index 73e16c8d..8d301244 100644 --- a/tests/redisutility_ut.cpp +++ b/tests/redisutility_ut.cpp @@ -2,7 +2,7 @@ #include "gtest/gtest.h" -TEST(REDISUTILITY, get_value) +TEST(REDISUTILITY, fvGetValue) { std::vector fvt; fvt.push_back(std::make_pair("int", "123")); @@ -10,17 +10,17 @@ TEST(REDISUTILITY, get_value) fvt.push_back(std::make_pair("string", "name")); int i; - EXPECT_TRUE(swss::get_value(fvt, "int", i)); + EXPECT_TRUE(swss::fvGetValue(fvt, "int", i)); EXPECT_EQ(i, 123); bool b; - EXPECT_TRUE(swss::get_value(fvt, "bool", b)); + EXPECT_TRUE(swss::fvGetValue(fvt, "bool", b)); EXPECT_EQ(b, true); std::string s; - EXPECT_TRUE(swss::get_value(fvt, "string", s)); + EXPECT_TRUE(swss::fvGetValue(fvt, "string", s)); EXPECT_EQ(s, "name"); double d; - EXPECT_FALSE(swss::get_value(fvt, "double", d)); + EXPECT_FALSE(swss::fvGetValue(fvt, "double", d)); } From a55d746e7c4766304b7a467ea723fc17bc916497 Mon Sep 17 00:00:00 2001 From: Ze Gan Date: Wed, 23 Dec 2020 13:44:18 +0800 Subject: [PATCH 04/17] Fix operator>>(istream, bool) Signed-off-by: Ze Gan --- common/stringutility.cpp | 61 ++++++++++++++++++++++++++++++-------- tests/stringutility_ut.cpp | 18 ++++++++++- 2 files changed, 65 insertions(+), 14 deletions(-) diff --git a/common/stringutility.cpp b/common/stringutility.cpp index 46b3da37..1ce7bb7c 100644 --- a/common/stringutility.cpp +++ b/common/stringutility.cpp @@ -7,25 +7,60 @@ std::istringstream &swss::operator>>( std::istringstream &istream, bool &b) { - std::string buffer = istream.str(); - std::transform( - buffer.begin(), - buffer.end(), - buffer.begin(), - ::tolower); - if (buffer == "true" || buffer == "1") + SWSS_LOG_ENTER(); + + bool valid_bool_string = true; + char c; + istream >> c; + if (std::isdigit(c)) { - b = true; + if (c == '1') + { + b = true; + } + else if (c == '0') + { + b = false; + } + else + { + valid_bool_string = false; + } } - else if (buffer == "false" || buffer == "0") + else { - b = false; + std::string expect_string; + if (std::tolower(c) == 't') + { + expect_string = "true"; + b = true; + } + else if (std::tolower(c) == 'f') + { + expect_string = "false"; + b = false; + } + if (expect_string.empty()) + { + valid_bool_string = false; + } + else + { + std::string receive_string(expect_string.length(), 0); + receive_string[0] = static_cast(std::tolower(c)); + istream.read(&receive_string[1], expect_string.length() - 1); + std::transform( + receive_string.begin(), + receive_string.end(), + receive_string.begin(), + ::tolower); + valid_bool_string = (receive_string == expect_string); + } } - else + if (!valid_bool_string) { - throw std::invalid_argument("Invalid bool string : " + buffer); + throw std::invalid_argument("Invalid bool string : " + istream.str()); } - return istream; } diff --git a/tests/stringutility_ut.cpp b/tests/stringutility_ut.cpp index 7b34ade5..2aec9839 100644 --- a/tests/stringutility_ut.cpp +++ b/tests/stringutility_ut.cpp @@ -32,16 +32,32 @@ TEST(STRINGUTILITY, split_bool) EXPECT_TRUE(swss::split("TRUE", ':', b)); EXPECT_EQ(b, true); + EXPECT_TRUE(swss::split("True", ':', b)); + EXPECT_EQ(b, true); + + EXPECT_TRUE(swss::split("Truexxxx", ':', b)); + EXPECT_EQ(b, true); + EXPECT_TRUE(swss::split("1", ':', b)); EXPECT_EQ(b, true); + EXPECT_TRUE(swss::split("123", ':', b)); + EXPECT_EQ(b, true); + EXPECT_TRUE(swss::split("false", ':', b)); EXPECT_EQ(b, false); EXPECT_TRUE(swss::split("0", ':', b)); EXPECT_EQ(b, false); - EXPECT_THROW(swss::split("123", ':', b), std::invalid_argument); + EXPECT_TRUE(swss::split("023", ':', b)); + EXPECT_EQ(b, false); + + EXPECT_THROW(swss::split("213", ':', b), std::invalid_argument); + + EXPECT_THROW(swss::split("tr", ':', b), std::invalid_argument); + + EXPECT_THROW(swss::split("abcdefg", ':', b), std::invalid_argument); } TEST(STRINGUTILITY, split_mix) From 014c7946ce31e0297cf4bf8de5665c6553693ea4 Mon Sep 17 00:00:00 2001 From: Ze Gan Date: Wed, 23 Dec 2020 14:07:04 +0800 Subject: [PATCH 05/17] Polish split to call tokenize Signed-off-by: Ze Gan --- common/stringutility.h | 40 +++++++++++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/common/stringutility.h b/common/stringutility.h index 1bb2533c..7cea9e4d 100644 --- a/common/stringutility.h +++ b/common/stringutility.h @@ -1,7 +1,9 @@ #pragma once #include "logger.h" +#include "tokenize.h" +#include #include #include #include @@ -22,17 +24,45 @@ bool split(const std::string &input, char delimiter, T &output) } template - bool split(const std::string &input, char delimiter, T &output, Args &... args) +bool split( + std::vector::iterator begin_itr, + std::vector::iterator end_itr, + T &output) { SWSS_LOG_ENTER(); - auto pos = input.find(delimiter); - if (pos == std::string::npos) + auto cur_itr = begin_itr++; + if (begin_itr != end_itr) { return false; } - std::istringstream istream(input.substr(0, pos)); + std::istringstream istream(*cur_itr); istream >> output; - return split(input.substr(pos + 1, input.length() - pos - 1), delimiter, args...); + return true; +} + +template +bool split( + std::vector::iterator begin_itr, + std::vector::iterator end_itr, + T &output, + Args &... args) +{ + SWSS_LOG_ENTER(); + if (begin_itr == end_itr) + { + return false; + } + std::istringstream istream(*(begin_itr++)); + istream >> output; + return split(begin_itr, end_itr, args...); +} + +template +bool split(const std::string &input, char delimiter, T &output, Args &... args) +{ + SWSS_LOG_ENTER(); + auto tokens = tokenize(input, delimiter); + return split(tokens.begin(), tokens.end(), output, args...); } template From c96c62433b9be5da72dbdcc6b3d30f5615dd8d3c Mon Sep 17 00:00:00 2001 From: Ze Gan Date: Wed, 23 Dec 2020 14:42:51 +0800 Subject: [PATCH 06/17] Polish hex binary function by calling boost hex/unhex Signed-off-by: Ze Gan --- common/stringutility.cpp | 58 +++++++++++++------------------------- tests/stringutility_ut.cpp | 3 ++ 2 files changed, 23 insertions(+), 38 deletions(-) diff --git a/common/stringutility.cpp b/common/stringutility.cpp index 1ce7bb7c..8f6f89d0 100644 --- a/common/stringutility.cpp +++ b/common/stringutility.cpp @@ -1,7 +1,10 @@ #include "stringutility.h" +#include + #include #include +#include std::istringstream &swss::operator>>( std::istringstream &istream, @@ -71,39 +74,27 @@ bool swss::hex_to_binary( { SWSS_LOG_ENTER(); - if (hex_str.length() % 2 != 0) - { - SWSS_LOG_ERROR("Invalid hex string %s", hex_str.c_str()); - return false; - } - if (hex_str.length() > (buffer_length * 2)) { SWSS_LOG_ERROR("Buffer length isn't sufficient"); return false; } - size_t buffer_cur = 0; - size_t hex_cur = 0; - unsigned char *output = static_cast(buffer); - - while (hex_cur < hex_str.length()) + try { - const char temp_buffer[] = { hex_str[hex_cur], hex_str[hex_cur + 1], 0 }; - unsigned int value = -1; - - if (sscanf(temp_buffer, "%X", &value) <= 0 || value > 0xff) - { - SWSS_LOG_ERROR("Invalid hex string %s", temp_buffer); - return false; - } - - output[buffer_cur] = static_cast(value); - hex_cur += 2; - buffer_cur += 1; + boost::algorithm::unhex(hex_str, buffer); + return true; + } + catch(const boost::algorithm::not_enough_input &e) + { + SWSS_LOG_ERROR("Not enough input %s", hex_str.c_str()); + } + catch(const boost::algorithm::non_hex_input &e) + { + SWSS_LOG_ERROR("Invalid hex string %s", hex_str.c_str()); } - return true; + return false; } std::string swss::binary_to_hex(const void *buffer, size_t length) @@ -111,21 +102,12 @@ std::string swss::binary_to_hex(const void *buffer, size_t length) SWSS_LOG_ENTER(); std::string s; + auto _buffer = static_cast(buffer); - if (buffer == NULL || length == 0) - { - return s; - } - - s.resize(2 * length, '0'); - - const unsigned char *input = static_cast(buffer); - char *output = &s[0]; - - for (size_t i = 0; i < length; i++) - { - snprintf(&output[i * 2], 3, "%02X", input[i]); - } + boost::algorithm::hex( + _buffer, + _buffer + length, + std::insert_iterator(s, s.end())); return s; } diff --git a/tests/stringutility_ut.cpp b/tests/stringutility_ut.cpp index 2aec9839..46817475 100644 --- a/tests/stringutility_ut.cpp +++ b/tests/stringutility_ut.cpp @@ -89,6 +89,7 @@ TEST(STRINGUTILITY, hex_to_binary) EXPECT_EQ(a, (std::array{0x1, 0x2, 0x0A, 0xFF, 0x5})); EXPECT_FALSE(swss::hex_to_binary("01020", a.data(), a.size())); + EXPECT_FALSE(swss::hex_to_binary("0101010101010101", a.data(), a.size())); EXPECT_FALSE(swss::hex_to_binary("xxxx", a.data(), a.size())); } @@ -96,4 +97,6 @@ TEST(STRINGUTILITY, binary_to_hex) { std::array a{0x1, 0x2, 0x0a, 0xff, 0x5}; EXPECT_EQ(swss::binary_to_hex(a.data(), a.size()), "01020AFF05"); + + EXPECT_EQ(swss::binary_to_hex(nullptr, 0), ""); } From 6dc76dfb2b97fa0c0874d87bca1398f9526c76b0 Mon Sep 17 00:00:00 2001 From: Ze Gan Date: Wed, 23 Dec 2020 17:21:48 +0800 Subject: [PATCH 07/17] Polish name and optimize split performance Signed-off-by: Ze Gan --- common/stringutility.cpp | 20 +++++++++++++++----- common/stringutility.h | 40 +++++++++++++++++++++++++--------------- 2 files changed, 40 insertions(+), 20 deletions(-) diff --git a/common/stringutility.cpp b/common/stringutility.cpp index 8f6f89d0..196818d6 100644 --- a/common/stringutility.cpp +++ b/common/stringutility.cpp @@ -67,6 +67,15 @@ std::istringstream &swss::operator>>( return istream; } +template<> +bool swss::lexical_cast(const std::string &str) +{ + bool b; + std::istringstream istream(str); + istream >> b; + return b; +} + bool swss::hex_to_binary( const std::string &hex_str, std::uint8_t *buffer, @@ -74,6 +83,11 @@ bool swss::hex_to_binary( { SWSS_LOG_ENTER(); + if (hex_str.length() %2 != 0) + { + SWSS_LOG_ERROR("Not enough input %s", hex_str.c_str()); + return false; + } if (hex_str.length() > (buffer_length * 2)) { SWSS_LOG_ERROR("Buffer length isn't sufficient"); @@ -85,10 +99,6 @@ bool swss::hex_to_binary( boost::algorithm::unhex(hex_str, buffer); return true; } - catch(const boost::algorithm::not_enough_input &e) - { - SWSS_LOG_ERROR("Not enough input %s", hex_str.c_str()); - } catch(const boost::algorithm::non_hex_input &e) { SWSS_LOG_ERROR("Invalid hex string %s", hex_str.c_str()); @@ -107,7 +117,7 @@ std::string swss::binary_to_hex(const void *buffer, size_t length) boost::algorithm::hex( _buffer, _buffer + length, - std::insert_iterator(s, s.end())); + std::back_inserter(s)); return s; } diff --git a/common/stringutility.h b/common/stringutility.h index 7cea9e4d..a21c09ad 100644 --- a/common/stringutility.h +++ b/common/stringutility.h @@ -3,13 +3,26 @@ #include "logger.h" #include "tokenize.h" +#include + +#include #include #include #include #include +#include namespace swss { +template +T lexical_cast(const std::string &str) +{ + return boost::lexical_cast(str); +} + +template<> +bool lexical_cast(const std::string &str); + template bool split(const std::string &input, char delimiter, T &output) { @@ -18,43 +31,40 @@ bool split(const std::string &input, char delimiter, T &output) { return false; } - std::istringstream istream(input); - istream >> output; + output = lexical_cast(input); return true; } template bool split( - std::vector::iterator begin_itr, - std::vector::iterator end_itr, + std::vector::iterator begin, + std::vector::iterator end, T &output) { SWSS_LOG_ENTER(); - auto cur_itr = begin_itr++; - if (begin_itr != end_itr) + auto cur_itr = begin++; + if (begin != end) { return false; } - std::istringstream istream(*cur_itr); - istream >> output; + output = lexical_cast(*cur_itr); return true; } template bool split( - std::vector::iterator begin_itr, - std::vector::iterator end_itr, + std::vector::iterator begin, + std::vector::iterator end, T &output, Args &... args) { SWSS_LOG_ENTER(); - if (begin_itr == end_itr) + if (begin == end) { return false; } - std::istringstream istream(*(begin_itr++)); - istream >> output; - return split(begin_itr, end_itr, args...); + output = lexical_cast(*(begin++)); + return split(begin, end, args...); } template @@ -66,7 +76,7 @@ bool split(const std::string &input, char delimiter, T &output, Args &... args) } template -std::string join(__attribute__((unused)) char delimiter, const T &input) +std::string join(char , const T &input) { SWSS_LOG_ENTER(); std::ostringstream ostream; From 83e95e8e3f61a8ed8f3fa098548c3029d1b3b63b Mon Sep 17 00:00:00 2001 From: Ze Gan Date: Wed, 23 Dec 2020 17:29:02 +0800 Subject: [PATCH 08/17] strictly limit the buffer length with hex string length Signed-off-by: Ze Gan --- common/stringutility.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common/stringutility.cpp b/common/stringutility.cpp index 196818d6..dfd0aa75 100644 --- a/common/stringutility.cpp +++ b/common/stringutility.cpp @@ -83,12 +83,12 @@ bool swss::hex_to_binary( { SWSS_LOG_ENTER(); - if (hex_str.length() %2 != 0) + if (hex_str.length() % 2 != 0) { SWSS_LOG_ERROR("Not enough input %s", hex_str.c_str()); return false; } - if (hex_str.length() > (buffer_length * 2)) + if (hex_str.length() != (buffer_length * 2)) { SWSS_LOG_ERROR("Buffer length isn't sufficient"); return false; From ef4ea2c90782bd7bb43f314b085731cb64ba3c75 Mon Sep 17 00:00:00 2001 From: Ze Gan Date: Thu, 24 Dec 2020 10:49:54 +0800 Subject: [PATCH 09/17] Rename fvtGetValue Signed-off-by: Ze Gan --- common/redisutility.h | 2 +- tests/redisutility_ut.cpp | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/common/redisutility.h b/common/redisutility.h index f929f1d4..901fc12d 100644 --- a/common/redisutility.h +++ b/common/redisutility.h @@ -11,7 +11,7 @@ namespace swss { template -static bool fvGetValue( +static bool fvtGetValue( const std::vector &fvt, const std::string &field, T &value) diff --git a/tests/redisutility_ut.cpp b/tests/redisutility_ut.cpp index 8d301244..e96caaee 100644 --- a/tests/redisutility_ut.cpp +++ b/tests/redisutility_ut.cpp @@ -2,7 +2,7 @@ #include "gtest/gtest.h" -TEST(REDISUTILITY, fvGetValue) +TEST(REDISUTILITY, fvtGetValue) { std::vector fvt; fvt.push_back(std::make_pair("int", "123")); @@ -10,17 +10,17 @@ TEST(REDISUTILITY, fvGetValue) fvt.push_back(std::make_pair("string", "name")); int i; - EXPECT_TRUE(swss::fvGetValue(fvt, "int", i)); + EXPECT_TRUE(swss::fvtGetValue(fvt, "int", i)); EXPECT_EQ(i, 123); bool b; - EXPECT_TRUE(swss::fvGetValue(fvt, "bool", b)); + EXPECT_TRUE(swss::fvtGetValue(fvt, "bool", b)); EXPECT_EQ(b, true); std::string s; - EXPECT_TRUE(swss::fvGetValue(fvt, "string", s)); + EXPECT_TRUE(swss::fvtGetValue(fvt, "string", s)); EXPECT_EQ(s, "name"); double d; - EXPECT_FALSE(swss::fvGetValue(fvt, "double", d)); + EXPECT_FALSE(swss::fvtGetValue(fvt, "double", d)); } From 88d835694db013c2d2e3e9df9e145fe2f3accec6 Mon Sep 17 00:00:00 2001 From: Ze Gan Date: Thu, 24 Dec 2020 10:58:48 +0800 Subject: [PATCH 10/17] optimize fvtGetValue Signed-off-by: Ze Gan --- common/redisutility.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/common/redisutility.h b/common/redisutility.h index 901fc12d..61284362 100644 --- a/common/redisutility.h +++ b/common/redisutility.h @@ -38,8 +38,7 @@ static bool fvtGetValue( }); if (itr != fvt.end()) { - std::istringstream istream(fvValue(*itr)); - istream >> value; + value = swss::lexical_cast(fvValue(*itr)); SWSS_LOG_DEBUG( "Set field '%s' as '%s'", field.c_str(), From 3d79e04d47a157ab217be3221b0a920b76249071 Mon Sep 17 00:00:00 2001 From: Ze Gan Date: Thu, 24 Dec 2020 17:30:13 +0800 Subject: [PATCH 11/17] Fix code Signed-off-by: Ze Gan --- common/Makefile.am | 3 +- common/redisutility.h | 43 ++++-------- common/schema.h | 3 + common/stringutility.cpp | 123 ---------------------------------- common/stringutility.h | 132 ++++++++++++++++++++++++++----------- tests/redisutility_ut.cpp | 20 +++--- tests/stringutility_ut.cpp | 55 +++++++--------- 7 files changed, 144 insertions(+), 235 deletions(-) delete mode 100644 common/stringutility.cpp diff --git a/common/Makefile.am b/common/Makefile.am index 4a28269a..a6231cb5 100644 --- a/common/Makefile.am +++ b/common/Makefile.am @@ -58,8 +58,7 @@ libswsscommon_la_SOURCES = \ exec.cpp \ subscriberstatetable.cpp \ timestamp.cpp \ - warm_restart.cpp \ - stringutility.cpp + warm_restart.cpp libswsscommon_la_CXXFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(LIBNL_CFLAGS) libswsscommon_la_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(LIBNL_CPPFLAGS) diff --git a/common/redisutility.h b/common/redisutility.h index 61284362..37a9f242 100644 --- a/common/redisutility.h +++ b/common/redisutility.h @@ -4,48 +4,31 @@ #include "logger.h" #include "stringutility.h" +#include +#include + #include #include namespace swss { -template -static bool fvtGetValue( +static inline boost::optional fvtGetValue( const std::vector &fvt, - const std::string &field, - T &value) + const std::string &field) { SWSS_LOG_ENTER(); - std::string target_field = field; - std::transform( - target_field.begin(), - target_field.end(), - target_field.begin(), - ::tolower); - auto itr = std::find_if( - fvt.begin(), - fvt.end(), - [&](const std::vector::value_type &entry) { - std::string available_field = fvField(entry); - std::transform( - available_field.begin(), - available_field.end(), - available_field.begin(), - ::tolower); - return available_field == target_field; - }); - if (itr != fvt.end()) + for (auto itr = fvt.begin(); itr != fvt.end(); itr++) { - value = swss::lexical_cast(fvValue(*itr)); - SWSS_LOG_DEBUG( - "Set field '%s' as '%s'", - field.c_str(), - fvValue(*itr).c_str()); - return true; + if (boost::iequals(fvField(*itr), field)) + { + return boost::optional(fvValue(*itr)); + } } + SWSS_LOG_DEBUG("Cannot find field : %s", field.c_str()); - return false; + + return boost::optional(); } } diff --git a/common/schema.h b/common/schema.h index 0eaacb14..4c613261 100644 --- a/common/schema.h +++ b/common/schema.h @@ -312,6 +312,9 @@ namespace swss { #define CFG_DTEL_QUEUE_REPORT_TABLE_NAME "DTEL_QUEUE_REPORT" #define CFG_DTEL_EVENT_TABLE_NAME "DTEL_EVENT" +#define TRUE_STRING "true" +#define FALSE_STRING "false" + } #endif diff --git a/common/stringutility.cpp b/common/stringutility.cpp deleted file mode 100644 index dfd0aa75..00000000 --- a/common/stringutility.cpp +++ /dev/null @@ -1,123 +0,0 @@ -#include "stringutility.h" - -#include - -#include -#include -#include - -std::istringstream &swss::operator>>( - std::istringstream &istream, - bool &b) -{ - SWSS_LOG_ENTER(); - - bool valid_bool_string = true; - char c; - istream >> c; - if (std::isdigit(c)) - { - if (c == '1') - { - b = true; - } - else if (c == '0') - { - b = false; - } - else - { - valid_bool_string = false; - } - } - else - { - std::string expect_string; - if (std::tolower(c) == 't') - { - expect_string = "true"; - b = true; - } - else if (std::tolower(c) == 'f') - { - expect_string = "false"; - b = false; - } - if (expect_string.empty()) - { - valid_bool_string = false; - } - else - { - std::string receive_string(expect_string.length(), 0); - receive_string[0] = static_cast(std::tolower(c)); - istream.read(&receive_string[1], expect_string.length() - 1); - std::transform( - receive_string.begin(), - receive_string.end(), - receive_string.begin(), - ::tolower); - valid_bool_string = (receive_string == expect_string); - } - } - if (!valid_bool_string) - { - throw std::invalid_argument("Invalid bool string : " + istream.str()); - } - return istream; -} - -template<> -bool swss::lexical_cast(const std::string &str) -{ - bool b; - std::istringstream istream(str); - istream >> b; - return b; -} - -bool swss::hex_to_binary( - const std::string &hex_str, - std::uint8_t *buffer, - size_t buffer_length) -{ - SWSS_LOG_ENTER(); - - if (hex_str.length() % 2 != 0) - { - SWSS_LOG_ERROR("Not enough input %s", hex_str.c_str()); - return false; - } - if (hex_str.length() != (buffer_length * 2)) - { - SWSS_LOG_ERROR("Buffer length isn't sufficient"); - return false; - } - - try - { - boost::algorithm::unhex(hex_str, buffer); - return true; - } - catch(const boost::algorithm::non_hex_input &e) - { - SWSS_LOG_ERROR("Invalid hex string %s", hex_str.c_str()); - } - - return false; -} - -std::string swss::binary_to_hex(const void *buffer, size_t length) -{ - SWSS_LOG_ENTER(); - - std::string s; - auto _buffer = static_cast(buffer); - - boost::algorithm::hex( - _buffer, - _buffer + length, - std::back_inserter(s)); - - return s; -} diff --git a/common/stringutility.h b/common/stringutility.h index a21c09ad..17b4d058 100644 --- a/common/stringutility.h +++ b/common/stringutility.h @@ -2,100 +2,139 @@ #include "logger.h" #include "tokenize.h" +#include "schema.h" #include +#include -#include +#include +#include +#include #include #include #include #include -#include namespace swss { template -T lexical_cast(const std::string &str) +void cast(const std::string &str, T &t) { - return boost::lexical_cast(str); + SWSS_LOG_ENTER(); + t = boost::lexical_cast(str); } -template<> -bool lexical_cast(const std::string &str); - -template -bool split(const std::string &input, char delimiter, T &output) +static inline void cast(const std::string &str, bool &b) { SWSS_LOG_ENTER(); - if (input.find(delimiter) != std::string::npos) + try { - return false; + b = boost::lexical_cast(str); + } + catch(const boost::bad_lexical_cast& e) + { + if (str == TRUE_STRING) + { + b = true; + } + else if (str == FALSE_STRING) + { + b = false; + } + else + { + throw e; + } } - output = lexical_cast(input); - return true; } template -bool split( - std::vector::iterator begin, - std::vector::iterator end, - T &output) +void cast( + std::vector::const_iterator begin, + std::vector::const_iterator end, + T &t) { SWSS_LOG_ENTER(); + if (begin == end) + { + SWSS_LOG_THROW("Insufficient corpus"); + } auto cur_itr = begin++; if (begin != end) { - return false; + SWSS_LOG_THROW("Too much corpus"); } - output = lexical_cast(*cur_itr); - return true; + cast(*cur_itr, t); } template -bool split( - std::vector::iterator begin, - std::vector::iterator end, - T &output, +void cast( + std::vector::const_iterator begin, + std::vector::const_iterator end, + T &t, Args &... args) { SWSS_LOG_ENTER(); if (begin == end) { - return false; + SWSS_LOG_THROW("Insufficient corpus"); } - output = lexical_cast(*(begin++)); - return split(begin, end, args...); + cast(*(begin++), t); + return cast(begin, end, args...); } template -bool split(const std::string &input, char delimiter, T &output, Args &... args) +void cast(const std::vector &strs, T &t, Args &... args) { - SWSS_LOG_ENTER(); - auto tokens = tokenize(input, delimiter); - return split(tokens.begin(), tokens.end(), output, args...); + cast(strs.begin(), strs.end(), t, args...); } template -std::string join(char , const T &input) +void join(std::ostringstream &ostream, char, const T &t) { SWSS_LOG_ENTER(); - std::ostringstream ostream; - ostream << input; - return ostream.str(); + ostream << t; +} + +template +void join(std::ostringstream &ostream, char delimiter, const T &t, const Args &... args) +{ + SWSS_LOG_ENTER(); + ostream << t << delimiter; + join(ostream, delimiter, args...); } template -std::string join(char delimiter, const T &input, const Args &... args) +std::string join(char delimiter, const T &t, const Args &... args) { SWSS_LOG_ENTER(); std::ostringstream ostream; - ostream << input << delimiter << join(delimiter, args...); + join(ostream, delimiter, t, args...); return ostream.str(); } -std::istringstream &operator>>(std::istringstream &istream, bool &b); +static inline bool hex_to_binary(const std::string &hex_str, std::uint8_t *buffer, size_t buffer_length) +{ + SWSS_LOG_ENTER(); + + if (hex_str.length() != (buffer_length * 2)) + { + SWSS_LOG_DEBUG("Buffer length isn't sufficient"); + return false; + } + + try + { + boost::algorithm::unhex(hex_str, buffer); + } + catch(const boost::algorithm::non_hex_input &e) + { + SWSS_LOG_DEBUG("Invalid hex string %s", hex_str.c_str()); + return false; + } -bool hex_to_binary(const std::string &hex_str, std::uint8_t *buffer, size_t buffer_length); + return true; +} template void hex_to_binary( @@ -107,6 +146,19 @@ void hex_to_binary( return hex_to_binary(s, &value, sizeof(T)); } -std::string binary_to_hex(const void *buffer, size_t length); +static inline std::string binary_to_hex(const void *buffer, size_t length) +{ + SWSS_LOG_ENTER(); + + std::string s; + auto buf = static_cast(buffer); + + boost::algorithm::hex( + buf, + buf + length, + std::back_inserter(s)); + + return s; +} } diff --git a/tests/redisutility_ut.cpp b/tests/redisutility_ut.cpp index e96caaee..1b448412 100644 --- a/tests/redisutility_ut.cpp +++ b/tests/redisutility_ut.cpp @@ -9,18 +9,20 @@ TEST(REDISUTILITY, fvtGetValue) fvt.push_back(std::make_pair("bool", "true")); fvt.push_back(std::make_pair("string", "name")); - int i; - EXPECT_TRUE(swss::fvtGetValue(fvt, "int", i)); - EXPECT_EQ(i, 123); + auto si = swss::fvtGetValue(fvt, "int"); + EXPECT_TRUE(si); + auto sb = swss::fvtGetValue(fvt, "bool"); + EXPECT_TRUE(sb); + auto ss = swss::fvtGetValue(fvt, "string"); + EXPECT_TRUE(ss); + int i; bool b; - EXPECT_TRUE(swss::fvtGetValue(fvt, "bool", b)); - EXPECT_EQ(b, true); - std::string s; - EXPECT_TRUE(swss::fvtGetValue(fvt, "string", s)); + ASSERT_NO_THROW(swss::cast({si.get(), sb.get(), ss.get()}, i, b, s)); + EXPECT_EQ(i, 123); + EXPECT_EQ(b, true); EXPECT_EQ(s, "name"); - double d; - EXPECT_FALSE(swss::fvtGetValue(fvt, "double", d)); + EXPECT_FALSE(swss::fvtGetValue(fvt, "double")); } diff --git a/tests/stringutility_ut.cpp b/tests/stringutility_ut.cpp index 46817475..cb9bf8de 100644 --- a/tests/stringutility_ut.cpp +++ b/tests/stringutility_ut.cpp @@ -1,75 +1,68 @@ #include "common/stringutility.h" #include "common/ipaddress.h" +#include "common/schema.h" #include "gtest/gtest.h" #include #include -TEST(STRINGUTILITY, split_int) +TEST(STRINGUTILITY, cast_int) { int i; - EXPECT_TRUE(swss::split("123", ':', i)); + EXPECT_NO_THROW(swss::cast("123", i)); EXPECT_EQ(i, 123); - EXPECT_TRUE(swss::split("0", ':', i)); + EXPECT_NO_THROW(swss::cast("0", i)); EXPECT_EQ(i, 0); - EXPECT_TRUE(swss::split("-123", ':', i)); + EXPECT_NO_THROW(swss::cast("-123", i)); EXPECT_EQ(i, -123); - EXPECT_FALSE(swss::split("123:", ':', i)); + EXPECT_THROW(swss::cast("123:", i), boost::bad_lexical_cast); } -TEST(STRINGUTILITY, split_bool) +TEST(STRINGUTILITY, cast_bool) { bool b; - EXPECT_TRUE(swss::split("true", ':', b)); + EXPECT_NO_THROW(swss::cast(TRUE_STRING, b)); EXPECT_EQ(b, true); - EXPECT_TRUE(swss::split("TRUE", ':', b)); + EXPECT_NO_THROW(swss::cast("1", b)); EXPECT_EQ(b, true); - EXPECT_TRUE(swss::split("True", ':', b)); - EXPECT_EQ(b, true); - - EXPECT_TRUE(swss::split("Truexxxx", ':', b)); - EXPECT_EQ(b, true); - - EXPECT_TRUE(swss::split("1", ':', b)); - EXPECT_EQ(b, true); - - EXPECT_TRUE(swss::split("123", ':', b)); - EXPECT_EQ(b, true); - - EXPECT_TRUE(swss::split("false", ':', b)); - EXPECT_EQ(b, false); - - EXPECT_TRUE(swss::split("0", ':', b)); + EXPECT_NO_THROW(swss::cast(FALSE_STRING, b)); EXPECT_EQ(b, false); - EXPECT_TRUE(swss::split("023", ':', b)); + EXPECT_NO_THROW(swss::cast("0", b)); EXPECT_EQ(b, false); - EXPECT_THROW(swss::split("213", ':', b), std::invalid_argument); + ASSERT_NE("True", TRUE_STRING); + EXPECT_THROW(swss::cast("True", b), boost::bad_lexical_cast); - EXPECT_THROW(swss::split("tr", ':', b), std::invalid_argument); - - EXPECT_THROW(swss::split("abcdefg", ':', b), std::invalid_argument); + EXPECT_THROW(swss::cast("abcdefg", b), boost::bad_lexical_cast); } -TEST(STRINGUTILITY, split_mix) +TEST(STRINGUTILITY, cast_mix) { int i; bool b; std::string s; - EXPECT_TRUE(swss::split("123:true:name", ':', i, b, s)); + EXPECT_NO_THROW(swss::cast({"123", TRUE_STRING, "name"}, i, b, s)); EXPECT_EQ(i, 123); EXPECT_EQ(b, true); EXPECT_EQ("name", s); + + std::vector attrs{"123", TRUE_STRING, "name"}; + EXPECT_NO_THROW(swss::cast(attrs.begin(), attrs.end(), i, b, s)); + EXPECT_NO_THROW(swss::cast(attrs, i, b, s)); + + EXPECT_THROW(swss::cast(attrs.begin(), attrs.end(), i), std::runtime_error); + EXPECT_THROW(swss::cast({"123"}, i, b), std::runtime_error); + EXPECT_THROW(swss::cast(attrs, i, i, i), boost::bad_lexical_cast); } TEST(STRINGUTILITY, join) From 7a0a67206ff141f1c1355119900f29b039b21ea3 Mon Sep 17 00:00:00 2001 From: Ze Gan Date: Fri, 25 Dec 2020 10:25:46 +0800 Subject: [PATCH 12/17] Polish code Signed-off-by: Ze Gan --- common/Makefile.am | 3 +- common/redisutility.cpp | 33 ++++++++++++ common/redisutility.h | 41 +-------------- common/schema.h | 4 +- common/stringutility.h | 102 ++++++++++++++++++++----------------- tests/redisutility_ut.cpp | 17 ++++--- tests/stringutility_ut.cpp | 32 ++++++------ 7 files changed, 118 insertions(+), 114 deletions(-) create mode 100644 common/redisutility.cpp diff --git a/common/Makefile.am b/common/Makefile.am index 5ec4e232..d77ea310 100644 --- a/common/Makefile.am +++ b/common/Makefile.am @@ -62,7 +62,8 @@ libswsscommon_la_SOURCES = \ exec.cpp \ subscriberstatetable.cpp \ timestamp.cpp \ - warm_restart.cpp + warm_restart.cpp \ + redisutility.cpp libswsscommon_la_CXXFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(LIBNL_CFLAGS) libswsscommon_la_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(LIBNL_CPPFLAGS) diff --git a/common/redisutility.cpp b/common/redisutility.cpp new file mode 100644 index 00000000..1d1d0ece --- /dev/null +++ b/common/redisutility.cpp @@ -0,0 +1,33 @@ +#include "redisutility.h" +#include "stringutility.h" + +#include + + +boost::optional swss::fvsGetValue( + const std::vector &fvt, + const std::string &field, + bool case_insensitive) +{ + boost::optional ret; + + for (auto itr = fvt.begin(); itr != fvt.end(); itr++) + { + bool is_equal = false; + if (case_insensitive) + { + is_equal = boost::iequals(fvField(*itr), field); + } + else + { + is_equal = (fvField(*itr) == field); + } + if (is_equal) + { + ret = fvValue(*itr); + break; + } + } + + return ret; +} \ No newline at end of file diff --git a/common/redisutility.h b/common/redisutility.h index d9e17298..3da0ac66 100644 --- a/common/redisutility.h +++ b/common/redisutility.h @@ -1,10 +1,7 @@ #pragma once #include "rediscommand.h" -#include "logger.h" -#include "stringutility.h" -#include #include #include @@ -12,42 +9,8 @@ namespace swss { - -enum FieldNameStrategy -{ - CASE_SENSITIVE, - CASE_INSENSITIVE, -}; - -static inline boost::optional fvtGetValue( +boost::optional fvsGetValue( const std::vector &fvt, const std::string &field, - FieldNameStrategy strategy = FieldNameStrategy::CASE_SENSITIVE) -{ - SWSS_LOG_ENTER(); - - for (auto itr = fvt.begin(); itr != fvt.end(); itr++) - { - bool is_equal = false; - switch(strategy) - { - case FieldNameStrategy::CASE_SENSITIVE: - is_equal = boost::equals(fvField(*itr), field); - break; - case FieldNameStrategy::CASE_INSENSITIVE: - is_equal = boost::iequals(fvField(*itr), field); - break; - default: - is_equal = boost::equals(fvField(*itr), field); - } - if (is_equal) - { - return boost::optional(fvValue(*itr)); - } - } - - SWSS_LOG_DEBUG("Cannot find field : %s", field.c_str()); - - return boost::optional(); -} + bool case_insensitive = false); } diff --git a/common/schema.h b/common/schema.h index 952ae93a..53d87aac 100644 --- a/common/schema.h +++ b/common/schema.h @@ -376,8 +376,8 @@ namespace swss { #define CFG_DTEL_QUEUE_REPORT_TABLE_NAME "DTEL_QUEUE_REPORT" #define CFG_DTEL_EVENT_TABLE_NAME "DTEL_EVENT" -#define TRUE_STRING "true" -#define FALSE_STRING "false" +#define TRUE_STRING "true" +#define FALSE_STRING "false" #ifdef __cplusplus } diff --git a/common/stringutility.h b/common/stringutility.h index 17b4d058..1f418a75 100644 --- a/common/stringutility.h +++ b/common/stringutility.h @@ -18,13 +18,13 @@ namespace swss { template -void cast(const std::string &str, T &t) +static inline void lexical_convert(const std::string &str, T &t) { SWSS_LOG_ENTER(); t = boost::lexical_cast(str); } -static inline void cast(const std::string &str, bool &b) +static inline void lexical_convert(const std::string &str, bool &b) { SWSS_LOG_ENTER(); try @@ -48,68 +48,78 @@ static inline void cast(const std::string &str, bool &b) } } -template -void cast( - std::vector::const_iterator begin, - std::vector::const_iterator end, - T &t) +namespace lexical_convert_detail { - SWSS_LOG_ENTER(); - if (begin == end) - { - SWSS_LOG_THROW("Insufficient corpus"); - } - auto cur_itr = begin++; - if (begin != end) + + template + void lexical_convert( + std::vector::const_iterator begin, + std::vector::const_iterator end, + T &t) { - SWSS_LOG_THROW("Too much corpus"); + SWSS_LOG_ENTER(); + if (begin == end) + { + SWSS_LOG_THROW("Insufficient corpus"); + } + auto cur_itr = begin++; + if (begin != end) + { + SWSS_LOG_THROW("Too much corpus"); + } + swss::lexical_convert(*cur_itr, t); } - cast(*cur_itr, t); -} -template -void cast( - std::vector::const_iterator begin, - std::vector::const_iterator end, - T &t, - Args &... args) -{ - SWSS_LOG_ENTER(); - if (begin == end) + template + void lexical_convert( + std::vector::const_iterator begin, + std::vector::const_iterator end, + T &t, + Args &... args) { - SWSS_LOG_THROW("Insufficient corpus"); + SWSS_LOG_ENTER(); + if (begin == end) + { + SWSS_LOG_THROW("Insufficient corpus"); + } + swss::lexical_convert(*(begin++), t); + return lexical_convert(begin, end, args...); } - cast(*(begin++), t); - return cast(begin, end, args...); + } template -void cast(const std::vector &strs, T &t, Args &... args) +void lexical_convert(const std::vector &strs, T &t, Args &... args) { - cast(strs.begin(), strs.end(), t, args...); + lexical_convert_detail::lexical_convert(strs.begin(), strs.end(), t, args...); } -template -void join(std::ostringstream &ostream, char, const T &t) +namespace join_detail { - SWSS_LOG_ENTER(); - ostream << t; -} -template -void join(std::ostringstream &ostream, char delimiter, const T &t, const Args &... args) -{ - SWSS_LOG_ENTER(); - ostream << t << delimiter; - join(ostream, delimiter, args...); + template + void join(std::ostringstream &ostream, char, const T &t) + { + SWSS_LOG_ENTER(); + ostream << t; + } + + template + void join(std::ostringstream &ostream, char delimiter, const T &t, const Args &... args) + { + SWSS_LOG_ENTER(); + ostream << t << delimiter; + join(ostream, delimiter, args...); + } + } template -std::string join(char delimiter, const T &t, const Args &... args) +static inline std::string join(char delimiter, const T &t, const Args &... args) { SWSS_LOG_ENTER(); std::ostringstream ostream; - join(ostream, delimiter, t, args...); + join_detail::join(ostream, delimiter, t, args...); return ostream.str(); } @@ -137,9 +147,7 @@ static inline bool hex_to_binary(const std::string &hex_str, std::uint8_t *buffe } template -void hex_to_binary( - const std::string &s, - T &value) +static inline void hex_to_binary(const std::string &s, T &value) { SWSS_LOG_ENTER(); diff --git a/tests/redisutility_ut.cpp b/tests/redisutility_ut.cpp index 89ee23e8..8266b4ae 100644 --- a/tests/redisutility_ut.cpp +++ b/tests/redisutility_ut.cpp @@ -1,30 +1,31 @@ #include "common/redisutility.h" +#include "common/stringutility.h" #include "gtest/gtest.h" -TEST(REDISUTILITY, fvtGetValue) +TEST(REDISUTILITY, fvsGetValue) { std::vector fvt; fvt.push_back(std::make_pair("int", "123")); fvt.push_back(std::make_pair("bool", "true")); fvt.push_back(std::make_pair("string", "name")); - auto si = swss::fvtGetValue(fvt, "int"); + auto si = swss::fvsGetValue(fvt, "int"); EXPECT_TRUE(si); - auto sb = swss::fvtGetValue(fvt, "bool"); + auto sb = swss::fvsGetValue(fvt, "bool"); EXPECT_TRUE(sb); - auto ss = swss::fvtGetValue(fvt, "string"); + auto ss = swss::fvsGetValue(fvt, "string"); EXPECT_TRUE(ss); int i; bool b; std::string s; - ASSERT_NO_THROW(swss::cast({si.get(), sb.get(), ss.get()}, i, b, s)); + ASSERT_NO_THROW(swss::lexical_convert({*si, *sb, *ss}, i, b, s)); EXPECT_EQ(i, 123); EXPECT_EQ(b, true); EXPECT_EQ(s, "name"); - EXPECT_FALSE(swss::fvtGetValue(fvt, "Int")); - EXPECT_TRUE(swss::fvtGetValue(fvt, "Int", swss::FieldNameStrategy::CASE_INSENSITIVE)); - EXPECT_FALSE(swss::fvtGetValue(fvt, "double")); + EXPECT_FALSE(swss::fvsGetValue(fvt, "Int")); + EXPECT_TRUE(swss::fvsGetValue(fvt, "Int", true)); + EXPECT_FALSE(swss::fvsGetValue(fvt, "double")); } diff --git a/tests/stringutility_ut.cpp b/tests/stringutility_ut.cpp index d9806ea9..75d765e9 100644 --- a/tests/stringutility_ut.cpp +++ b/tests/stringutility_ut.cpp @@ -11,38 +11,38 @@ TEST(STRINGUTILITY, cast_int) { int i; - EXPECT_NO_THROW(swss::cast("123", i)); + EXPECT_NO_THROW(swss::lexical_convert("123", i)); EXPECT_EQ(i, 123); - EXPECT_NO_THROW(swss::cast("0", i)); + EXPECT_NO_THROW(swss::lexical_convert("0", i)); EXPECT_EQ(i, 0); - EXPECT_NO_THROW(swss::cast("-123", i)); + EXPECT_NO_THROW(swss::lexical_convert("-123", i)); EXPECT_EQ(i, -123); - EXPECT_THROW(swss::cast("123:", i), boost::bad_lexical_cast); + EXPECT_THROW(swss::lexical_convert("123:", i), boost::bad_lexical_cast); } TEST(STRINGUTILITY, cast_bool) { bool b; - EXPECT_NO_THROW(swss::cast(TRUE_STRING, b)); + EXPECT_NO_THROW(swss::lexical_convert(TRUE_STRING, b)); EXPECT_EQ(b, true); - EXPECT_NO_THROW(swss::cast("1", b)); + EXPECT_NO_THROW(swss::lexical_convert("1", b)); EXPECT_EQ(b, true); - EXPECT_NO_THROW(swss::cast(FALSE_STRING, b)); + EXPECT_NO_THROW(swss::lexical_convert(FALSE_STRING, b)); EXPECT_EQ(b, false); - EXPECT_NO_THROW(swss::cast("0", b)); + EXPECT_NO_THROW(swss::lexical_convert("0", b)); EXPECT_EQ(b, false); ASSERT_NE("True", TRUE_STRING); - EXPECT_THROW(swss::cast("True", b), boost::bad_lexical_cast); + EXPECT_THROW(swss::lexical_convert("True", b), boost::bad_lexical_cast); - EXPECT_THROW(swss::cast("abcdefg", b), boost::bad_lexical_cast); + EXPECT_THROW(swss::lexical_convert("abcdefg", b), boost::bad_lexical_cast); } TEST(STRINGUTILITY, cast_mix) @@ -51,23 +51,21 @@ TEST(STRINGUTILITY, cast_mix) bool b; std::string s; - EXPECT_NO_THROW(swss::cast(swss::tokenize("123:" TRUE_STRING ":name", ':'), i, b, s)); + EXPECT_NO_THROW(swss::lexical_convert(swss::tokenize("123:" TRUE_STRING ":name", ':'), i, b, s)); EXPECT_EQ(i, 123); EXPECT_EQ(b, true); EXPECT_EQ("name", s); - EXPECT_NO_THROW(swss::cast({"123", TRUE_STRING, "name"}, i, b, s)); + EXPECT_NO_THROW(swss::lexical_convert({"123", TRUE_STRING, "name"}, i, b, s)); EXPECT_EQ(i, 123); EXPECT_EQ(b, true); EXPECT_EQ("name", s); std::vector attrs{"123", TRUE_STRING, "name"}; - EXPECT_NO_THROW(swss::cast(attrs.begin(), attrs.end(), i, b, s)); - EXPECT_NO_THROW(swss::cast(attrs, i, b, s)); + EXPECT_NO_THROW(swss::lexical_convert(attrs, i, b, s)); - EXPECT_THROW(swss::cast(attrs.begin(), attrs.end(), i), std::runtime_error); - EXPECT_THROW(swss::cast({"123"}, i, b), std::runtime_error); - EXPECT_THROW(swss::cast(attrs, i, i, i), boost::bad_lexical_cast); + EXPECT_THROW(swss::lexical_convert({"123"}, i, b), std::runtime_error); + EXPECT_THROW(swss::lexical_convert(attrs, i, i, i), boost::bad_lexical_cast); } TEST(STRINGUTILITY, join) From 2adf276cae1ef68e12cdefcb90e32855e02d05b0 Mon Sep 17 00:00:00 2001 From: Ze Gan Date: Fri, 25 Dec 2020 10:45:49 +0800 Subject: [PATCH 13/17] Remove LOG_ENTER in short functions Signed-off-by: Ze Gan --- common/stringutility.h | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/common/stringutility.h b/common/stringutility.h index 1f418a75..72e4dda6 100644 --- a/common/stringutility.h +++ b/common/stringutility.h @@ -20,13 +20,11 @@ namespace swss { template static inline void lexical_convert(const std::string &str, T &t) { - SWSS_LOG_ENTER(); t = boost::lexical_cast(str); } static inline void lexical_convert(const std::string &str, bool &b) { - SWSS_LOG_ENTER(); try { b = boost::lexical_cast(str); @@ -57,7 +55,6 @@ namespace lexical_convert_detail std::vector::const_iterator end, T &t) { - SWSS_LOG_ENTER(); if (begin == end) { SWSS_LOG_THROW("Insufficient corpus"); @@ -77,7 +74,6 @@ namespace lexical_convert_detail T &t, Args &... args) { - SWSS_LOG_ENTER(); if (begin == end) { SWSS_LOG_THROW("Insufficient corpus"); @@ -100,14 +96,12 @@ namespace join_detail template void join(std::ostringstream &ostream, char, const T &t) { - SWSS_LOG_ENTER(); ostream << t; } template void join(std::ostringstream &ostream, char delimiter, const T &t, const Args &... args) { - SWSS_LOG_ENTER(); ostream << t << delimiter; join(ostream, delimiter, args...); } @@ -117,7 +111,6 @@ namespace join_detail template static inline std::string join(char delimiter, const T &t, const Args &... args) { - SWSS_LOG_ENTER(); std::ostringstream ostream; join_detail::join(ostream, delimiter, t, args...); return ostream.str(); @@ -125,8 +118,6 @@ static inline std::string join(char delimiter, const T &t, const Args &... args) static inline bool hex_to_binary(const std::string &hex_str, std::uint8_t *buffer, size_t buffer_length) { - SWSS_LOG_ENTER(); - if (hex_str.length() != (buffer_length * 2)) { SWSS_LOG_DEBUG("Buffer length isn't sufficient"); @@ -149,15 +140,11 @@ static inline bool hex_to_binary(const std::string &hex_str, std::uint8_t *buffe template static inline void hex_to_binary(const std::string &s, T &value) { - SWSS_LOG_ENTER(); - return hex_to_binary(s, &value, sizeof(T)); } static inline std::string binary_to_hex(const void *buffer, size_t length) { - SWSS_LOG_ENTER(); - std::string s; auto buf = static_cast(buffer); From 280594d2bbe7dd9b1851f5295720983847485eb9 Mon Sep 17 00:00:00 2001 From: Ze Gan Date: Fri, 25 Dec 2020 15:06:22 +0800 Subject: [PATCH 14/17] Add alpha boolean Signed-off-by: Ze Gan --- common/Makefile.am | 3 +- common/boolean.cpp | 68 ++++++++++++++++++++++++++++++++++++++ common/boolean.h | 30 +++++++++++++++++ common/schema.h | 3 -- common/stringutility.h | 44 ++++++++++++------------ tests/Makefile.am | 1 + tests/boolean_ut.cpp | 53 +++++++++++++++++++++++++++++ tests/redisutility_ut.cpp | 3 +- tests/stringutility_ut.cpp | 33 ++++++++---------- 9 files changed, 192 insertions(+), 46 deletions(-) create mode 100644 common/boolean.cpp create mode 100644 common/boolean.h create mode 100644 tests/boolean_ut.cpp diff --git a/common/Makefile.am b/common/Makefile.am index d77ea310..c43ee1db 100644 --- a/common/Makefile.am +++ b/common/Makefile.am @@ -63,7 +63,8 @@ libswsscommon_la_SOURCES = \ subscriberstatetable.cpp \ timestamp.cpp \ warm_restart.cpp \ - redisutility.cpp + redisutility.cpp \ + boolean.cpp libswsscommon_la_CXXFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(LIBNL_CFLAGS) libswsscommon_la_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(LIBNL_CPPFLAGS) diff --git a/common/boolean.cpp b/common/boolean.cpp new file mode 100644 index 00000000..f3b3e091 --- /dev/null +++ b/common/boolean.cpp @@ -0,0 +1,68 @@ +#include "boolean.h" + +#include + +swss::Boolean::operator bool() const +{ + return m_boolean; +} + +swss::Boolean::Boolean(bool boolean) : m_boolean(boolean) +{ +} + +swss::AlphaBoolean::AlphaBoolean(bool boolean) : Boolean(boolean) +{ +} + +const std::string swss::AlphaBoolean::ALPHA_TRUE = "true"; +const std::string swss::AlphaBoolean::ALPHA_FALSE = "false"; + +std::ostream &swss::operator<<(std::ostream &out, const swss::AlphaBoolean &b) +{ + out << (b ? swss::AlphaBoolean::ALPHA_TRUE : swss::AlphaBoolean::ALPHA_FALSE); + return out; +} + +std::istream &swss::operator>>(std::istream &in, swss::AlphaBoolean &b) +{ + bool target_bool = false; + std::string target_string; + std::string received_string; + if (in.peek() == 't') + { + target_string = swss::AlphaBoolean::ALPHA_TRUE; + target_bool = true; + } + else if (in.peek() == 'f') + { + target_string = swss::AlphaBoolean::ALPHA_FALSE; + target_bool = false; + } + else + { + in.setstate(std::istream::failbit); + return in; + } + + received_string.resize(target_string.length(), 0); + in.read(&received_string[0], received_string.length()); + + if (target_string != received_string) + { + for (auto itr = received_string.begin(); itr != received_string.end(); ++itr) + { + if (*itr != 0) + { + in.putback(*itr); + } + } + in.setstate(std::istream::failbit); + } + else + { + b = target_bool; + } + + return in; +} diff --git a/common/boolean.h b/common/boolean.h new file mode 100644 index 00000000..f27280e1 --- /dev/null +++ b/common/boolean.h @@ -0,0 +1,30 @@ +#pragma once + +#include + +namespace swss +{ +class Boolean +{ +public: + operator bool() const; +protected: + Boolean() = default; + Boolean(bool boolean); + bool m_boolean; +}; + +class AlphaBoolean : public Boolean +{ +public: + static const std::string ALPHA_TRUE; + static const std::string ALPHA_FALSE; + AlphaBoolean() = default; + AlphaBoolean(bool boolean); +}; + +std::ostream &operator<<(std::ostream &out, const AlphaBoolean &b); + +std::istream &operator>>(std::istream &in, AlphaBoolean &b); + +} diff --git a/common/schema.h b/common/schema.h index 53d87aac..f5084b80 100644 --- a/common/schema.h +++ b/common/schema.h @@ -376,9 +376,6 @@ namespace swss { #define CFG_DTEL_QUEUE_REPORT_TABLE_NAME "DTEL_QUEUE_REPORT" #define CFG_DTEL_EVENT_TABLE_NAME "DTEL_EVENT" -#define TRUE_STRING "true" -#define FALSE_STRING "false" - #ifdef __cplusplus } #endif diff --git a/common/stringutility.h b/common/stringutility.h index 72e4dda6..7add7a61 100644 --- a/common/stringutility.h +++ b/common/stringutility.h @@ -23,28 +23,28 @@ static inline void lexical_convert(const std::string &str, T &t) t = boost::lexical_cast(str); } -static inline void lexical_convert(const std::string &str, bool &b) -{ - try - { - b = boost::lexical_cast(str); - } - catch(const boost::bad_lexical_cast& e) - { - if (str == TRUE_STRING) - { - b = true; - } - else if (str == FALSE_STRING) - { - b = false; - } - else - { - throw e; - } - } -} +// static inline void lexical_convert(const std::string &str, bool &b) +// { +// try +// { +// b = boost::lexical_cast(str); +// } +// catch(const boost::bad_lexical_cast& e) +// { +// if (str == TRUE_STRING) +// { +// b = true; +// } +// else if (str == FALSE_STRING) +// { +// b = false; +// } +// else +// { +// throw e; +// } +// } +// } namespace lexical_convert_detail { diff --git a/tests/Makefile.am b/tests/Makefile.am index 1a99ebec..25827bab 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -32,6 +32,7 @@ tests_SOURCES = redis_ut.cpp \ fdb_flush.cpp \ stringutility_ut.cpp \ redisutility_ut.cpp \ + boolean_ut.cpp \ main.cpp tests_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(LIBNL_CFLAGS) diff --git a/tests/boolean_ut.cpp b/tests/boolean_ut.cpp new file mode 100644 index 00000000..fc0aa784 --- /dev/null +++ b/tests/boolean_ut.cpp @@ -0,0 +1,53 @@ +#include "common/boolean.h" + +#include "gtest/gtest.h" + +#include + +TEST(BOOLEAN, alpha_boolean) +{ + swss::AlphaBoolean bt(true); + swss::AlphaBoolean bf(false); + + EXPECT_TRUE(bt); + EXPECT_FALSE(bf); + + std::ostringstream ost; + ost << bt; + EXPECT_EQ(ost.str(), "true"); + + std::ostringstream osf; + osf << bf; + EXPECT_EQ(osf.str(), "false"); + + std::istringstream is; + + bt = false; + is.str("true"); + is >> bt; + EXPECT_TRUE(bt); + EXPECT_FALSE(is.fail()); + is.clear(); + + bt = false; + is.str("true123"); + int i; + is >> bt >> i; + EXPECT_TRUE(bt); + EXPECT_EQ(i, 123); + EXPECT_FALSE(is.fail()); + is.clear(); + + bf = true; + is.str("false"); + is >> bf; + EXPECT_FALSE(bf); + EXPECT_FALSE(is.fail()); + is.clear(); + + bf = true; + is.str("abdef"); + EXPECT_FALSE(is >> bt); + EXPECT_TRUE(is.fail()); + is.clear(); +} diff --git a/tests/redisutility_ut.cpp b/tests/redisutility_ut.cpp index 8266b4ae..6257580d 100644 --- a/tests/redisutility_ut.cpp +++ b/tests/redisutility_ut.cpp @@ -1,5 +1,6 @@ #include "common/redisutility.h" #include "common/stringutility.h" +#include "common/boolean.h" #include "gtest/gtest.h" @@ -18,7 +19,7 @@ TEST(REDISUTILITY, fvsGetValue) EXPECT_TRUE(ss); int i; - bool b; + swss::AlphaBoolean b; std::string s; ASSERT_NO_THROW(swss::lexical_convert({*si, *sb, *ss}, i, b, s)); EXPECT_EQ(i, 123); diff --git a/tests/stringutility_ut.cpp b/tests/stringutility_ut.cpp index 75d765e9..5f217c8e 100644 --- a/tests/stringutility_ut.cpp +++ b/tests/stringutility_ut.cpp @@ -1,6 +1,5 @@ #include "common/stringutility.h" -#include "common/ipaddress.h" -#include "common/schema.h" +#include "common/boolean.h" #include "gtest/gtest.h" @@ -23,23 +22,16 @@ TEST(STRINGUTILITY, cast_int) EXPECT_THROW(swss::lexical_convert("123:", i), boost::bad_lexical_cast); } -TEST(STRINGUTILITY, cast_bool) +TEST(STRINGUTILITY, cast_alpha_bool) { - bool b; + swss::AlphaBoolean b; - EXPECT_NO_THROW(swss::lexical_convert(TRUE_STRING, b)); + EXPECT_NO_THROW(swss::lexical_convert("true", b)); EXPECT_EQ(b, true); - EXPECT_NO_THROW(swss::lexical_convert("1", b)); - EXPECT_EQ(b, true); - - EXPECT_NO_THROW(swss::lexical_convert(FALSE_STRING, b)); - EXPECT_EQ(b, false); - - EXPECT_NO_THROW(swss::lexical_convert("0", b)); + EXPECT_NO_THROW(swss::lexical_convert("false", b)); EXPECT_EQ(b, false); - ASSERT_NE("True", TRUE_STRING); EXPECT_THROW(swss::lexical_convert("True", b), boost::bad_lexical_cast); EXPECT_THROW(swss::lexical_convert("abcdefg", b), boost::bad_lexical_cast); @@ -48,20 +40,20 @@ TEST(STRINGUTILITY, cast_bool) TEST(STRINGUTILITY, cast_mix) { int i; - bool b; + swss::AlphaBoolean b; std::string s; - EXPECT_NO_THROW(swss::lexical_convert(swss::tokenize("123:" TRUE_STRING ":name", ':'), i, b, s)); + EXPECT_NO_THROW(swss::lexical_convert(swss::tokenize("123:true:name", ':'), i, b, s)); EXPECT_EQ(i, 123); EXPECT_EQ(b, true); EXPECT_EQ("name", s); - EXPECT_NO_THROW(swss::lexical_convert({"123", TRUE_STRING, "name"}, i, b, s)); + EXPECT_NO_THROW(swss::lexical_convert({"123", "true", "name"}, i, b, s)); EXPECT_EQ(i, 123); EXPECT_EQ(b, true); EXPECT_EQ("name", s); - std::vector attrs{"123", TRUE_STRING, "name"}; + std::vector attrs{"123", "true", "name"}; EXPECT_NO_THROW(swss::lexical_convert(attrs, i, b, s)); EXPECT_THROW(swss::lexical_convert({"123"}, i, b), std::runtime_error); @@ -70,8 +62,11 @@ TEST(STRINGUTILITY, cast_mix) TEST(STRINGUTILITY, join) { - auto str = swss::join(':', 123, true, std::string("name")); - EXPECT_EQ(str, "123:1:name"); + EXPECT_EQ(swss::join(':', 123, true, std::string("name")), "123:1:name"); + + EXPECT_EQ(swss::join(':', 123, swss::AlphaBoolean(true), std::string("name")), "123:true:name"); + + EXPECT_EQ(swss::join('|', 123, swss::AlphaBoolean(false), std::string("name")), "123|false|name"); } TEST(STRINGUTILITY, hex_to_binary) From a7f5acd371c8de71d08e3d5d8c9416aa04962fbf Mon Sep 17 00:00:00 2001 From: Ze Gan Date: Mon, 28 Dec 2020 12:33:06 +0800 Subject: [PATCH 15/17] polish code Signed-off-by: Ze Gan --- common/boolean.cpp | 68 +++++++++++------------------------------- common/boolean.h | 21 +++++++++---- common/stringutility.h | 23 -------------- 3 files changed, 32 insertions(+), 80 deletions(-) diff --git a/common/boolean.cpp b/common/boolean.cpp index f3b3e091..bd3c6a5d 100644 --- a/common/boolean.cpp +++ b/common/boolean.cpp @@ -1,68 +1,34 @@ #include "boolean.h" #include +#include -swss::Boolean::operator bool() const +std::ostream &swss::operator<<(std::ostream &out, const swss::AlphaBoolean &b) { - return m_boolean; + bool value = b; + out << std::boolalpha << value; + return out; } -swss::Boolean::Boolean(bool boolean) : m_boolean(boolean) +std::istream &swss::operator>>(std::istream &in, swss::AlphaBoolean &b) { + bool value = false; + in >> std::boolalpha >> value; + b = value; + return in; } -swss::AlphaBoolean::AlphaBoolean(bool boolean) : Boolean(boolean) +std::ostream &swss::operator<<(std::ostream &out, const swss::Boolean &b) { -} - -const std::string swss::AlphaBoolean::ALPHA_TRUE = "true"; -const std::string swss::AlphaBoolean::ALPHA_FALSE = "false"; - -std::ostream &swss::operator<<(std::ostream &out, const swss::AlphaBoolean &b) -{ - out << (b ? swss::AlphaBoolean::ALPHA_TRUE : swss::AlphaBoolean::ALPHA_FALSE); + bool value = b; + out << value; return out; } -std::istream &swss::operator>>(std::istream &in, swss::AlphaBoolean &b) +std::istream &swss::operator>>(std::istream &in, swss::Boolean &b) { - bool target_bool = false; - std::string target_string; - std::string received_string; - if (in.peek() == 't') - { - target_string = swss::AlphaBoolean::ALPHA_TRUE; - target_bool = true; - } - else if (in.peek() == 'f') - { - target_string = swss::AlphaBoolean::ALPHA_FALSE; - target_bool = false; - } - else - { - in.setstate(std::istream::failbit); - return in; - } - - received_string.resize(target_string.length(), 0); - in.read(&received_string[0], received_string.length()); - - if (target_string != received_string) - { - for (auto itr = received_string.begin(); itr != received_string.end(); ++itr) - { - if (*itr != 0) - { - in.putback(*itr); - } - } - in.setstate(std::istream::failbit); - } - else - { - b = target_bool; - } - + bool value = false; + in >> value; + b = value; return in; } diff --git a/common/boolean.h b/common/boolean.h index f27280e1..bd736748 100644 --- a/common/boolean.h +++ b/common/boolean.h @@ -7,22 +7,31 @@ namespace swss class Boolean { public: - operator bool() const; -protected: Boolean() = default; - Boolean(bool boolean); + Boolean(bool boolean) : m_boolean(boolean) + { + } + operator bool() const + { + return m_boolean; + } +protected: bool m_boolean; }; class AlphaBoolean : public Boolean { public: - static const std::string ALPHA_TRUE; - static const std::string ALPHA_FALSE; AlphaBoolean() = default; - AlphaBoolean(bool boolean); + AlphaBoolean(bool boolean) : Boolean(boolean) + { + } }; +std::ostream &operator<<(std::ostream &out, const Boolean &b); + +std::istream &operator>>(std::istream &in, Boolean &b); + std::ostream &operator<<(std::ostream &out, const AlphaBoolean &b); std::istream &operator>>(std::istream &in, AlphaBoolean &b); diff --git a/common/stringutility.h b/common/stringutility.h index 7add7a61..c569fbe6 100644 --- a/common/stringutility.h +++ b/common/stringutility.h @@ -23,29 +23,6 @@ static inline void lexical_convert(const std::string &str, T &t) t = boost::lexical_cast(str); } -// static inline void lexical_convert(const std::string &str, bool &b) -// { -// try -// { -// b = boost::lexical_cast(str); -// } -// catch(const boost::bad_lexical_cast& e) -// { -// if (str == TRUE_STRING) -// { -// b = true; -// } -// else if (str == FALSE_STRING) -// { -// b = false; -// } -// else -// { -// throw e; -// } -// } -// } - namespace lexical_convert_detail { From 45af9c2b26c41daa773b984e786034804fabd392 Mon Sep 17 00:00:00 2001 From: Ze Gan Date: Mon, 28 Dec 2020 12:39:03 +0800 Subject: [PATCH 16/17] optimize stream operator Signed-off-by: Ze Gan --- common/Makefile.am | 3 +-- common/boolean.cpp | 34 ---------------------------------- common/boolean.h | 31 +++++++++++++++++++++++++++---- 3 files changed, 28 insertions(+), 40 deletions(-) delete mode 100644 common/boolean.cpp diff --git a/common/Makefile.am b/common/Makefile.am index c43ee1db..d77ea310 100644 --- a/common/Makefile.am +++ b/common/Makefile.am @@ -63,8 +63,7 @@ libswsscommon_la_SOURCES = \ subscriberstatetable.cpp \ timestamp.cpp \ warm_restart.cpp \ - redisutility.cpp \ - boolean.cpp + redisutility.cpp libswsscommon_la_CXXFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(LIBNL_CFLAGS) libswsscommon_la_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(LIBNL_CPPFLAGS) diff --git a/common/boolean.cpp b/common/boolean.cpp deleted file mode 100644 index bd3c6a5d..00000000 --- a/common/boolean.cpp +++ /dev/null @@ -1,34 +0,0 @@ -#include "boolean.h" - -#include -#include - -std::ostream &swss::operator<<(std::ostream &out, const swss::AlphaBoolean &b) -{ - bool value = b; - out << std::boolalpha << value; - return out; -} - -std::istream &swss::operator>>(std::istream &in, swss::AlphaBoolean &b) -{ - bool value = false; - in >> std::boolalpha >> value; - b = value; - return in; -} - -std::ostream &swss::operator<<(std::ostream &out, const swss::Boolean &b) -{ - bool value = b; - out << value; - return out; -} - -std::istream &swss::operator>>(std::istream &in, swss::Boolean &b) -{ - bool value = false; - in >> value; - b = value; - return in; -} diff --git a/common/boolean.h b/common/boolean.h index bd736748..ba46be4c 100644 --- a/common/boolean.h +++ b/common/boolean.h @@ -1,6 +1,7 @@ #pragma once #include +#include namespace swss { @@ -28,12 +29,34 @@ class AlphaBoolean : public Boolean } }; -std::ostream &operator<<(std::ostream &out, const Boolean &b); +static inline std::ostream &operator<<(std::ostream &out, const AlphaBoolean &b) +{ + bool value = b; + out << std::boolalpha << value; + return out; +} -std::istream &operator>>(std::istream &in, Boolean &b); +static inline std::istream &operator>>(std::istream &in, AlphaBoolean &b) +{ + bool value = false; + in >> std::boolalpha >> value; + b = value; + return in; +} -std::ostream &operator<<(std::ostream &out, const AlphaBoolean &b); +static inline std::ostream &operator<<(std::ostream &out, const Boolean &b) +{ + bool value = b; + out << value; + return out; +} -std::istream &operator>>(std::istream &in, AlphaBoolean &b); +static inline std::istream &operator>>(std::istream &in, Boolean &b) +{ + bool value = false; + in >> value; + b = value; + return in; +} } From c452b865715c9eb95205c113f0b1c4957446863e Mon Sep 17 00:00:00 2001 From: Ze Gan Date: Mon, 28 Dec 2020 14:01:19 +0800 Subject: [PATCH 17/17] polish code Signed-off-by: Ze Gan --- common/boolean.h | 22 ++++++++-------------- tests/boolean_ut.cpp | 27 +++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 14 deletions(-) diff --git a/common/boolean.h b/common/boolean.h index ba46be4c..12219f51 100644 --- a/common/boolean.h +++ b/common/boolean.h @@ -16,6 +16,10 @@ class Boolean { return m_boolean; } + operator bool&() + { + return m_boolean; + } protected: bool m_boolean; }; @@ -31,32 +35,22 @@ class AlphaBoolean : public Boolean static inline std::ostream &operator<<(std::ostream &out, const AlphaBoolean &b) { - bool value = b; - out << std::boolalpha << value; - return out; + return out << std::boolalpha << (bool)(b); } static inline std::istream &operator>>(std::istream &in, AlphaBoolean &b) { - bool value = false; - in >> std::boolalpha >> value; - b = value; - return in; + return in >> std::boolalpha >> (bool &)(b); } static inline std::ostream &operator<<(std::ostream &out, const Boolean &b) { - bool value = b; - out << value; - return out; + return out << (bool)(b); } static inline std::istream &operator>>(std::istream &in, Boolean &b) { - bool value = false; - in >> value; - b = value; - return in; + return in >> (bool &)(b); } } diff --git a/tests/boolean_ut.cpp b/tests/boolean_ut.cpp index fc0aa784..c8a214ad 100644 --- a/tests/boolean_ut.cpp +++ b/tests/boolean_ut.cpp @@ -4,6 +4,33 @@ #include +TEST(BOOLEAN, boolean) +{ + swss::Boolean b; + + b = true; + std::ostringstream ost; + ost << b; + EXPECT_EQ(ost.str(), "1"); + + b = false; + std::ostringstream osf; + osf << b; + EXPECT_EQ(osf.str(), "0"); + + b = false; + std::istringstream ist("1"); + ist >> b; + EXPECT_TRUE(b); + EXPECT_FALSE(ist.fail()); + + b = true; + std::istringstream isf("0"); + isf >> b; + EXPECT_FALSE(b); + EXPECT_FALSE(isf.fail()); +} + TEST(BOOLEAN, alpha_boolean) { swss::AlphaBoolean bt(true);