From ea74f647be847791b1824fc371e66cc9d79c9f48 Mon Sep 17 00:00:00 2001 From: Eddy Ashton Date: Thu, 11 Jul 2024 12:05:18 +0100 Subject: [PATCH 1/2] Make a private `nonstd.h` header, and remove some inline definitions (#6352) Signed-off-by: Markus Alexander Kuppe Co-authored-by: Markus Alexander Kuppe Co-authored-by: Amaury Chamayou --- CMakeLists.txt | 10 +- cmake/common.cmake | 7 +- include/ccf/ds/nonstd.h | 138 +-------------------- src/ds/nonstd.cpp | 54 ++++++++ src/ds/nonstd.h | 61 +++++++++ src/ds/test/nonstd.cpp | 59 +-------- src/endpoints/endpoint_registry.cpp | 1 + src/host/env.cpp | 60 +++++++++ src/host/env.h | 12 ++ src/host/main.cpp | 12 +- src/host/test/env.cpp | 63 ++++++++++ src/http/http2_utils.h | 2 +- tests/perf-system/submitter/CMakeLists.txt | 2 +- 13 files changed, 276 insertions(+), 205 deletions(-) create mode 100644 src/ds/nonstd.cpp create mode 100644 src/ds/nonstd.h create mode 100644 src/host/env.cpp create mode 100644 src/host/env.h create mode 100644 src/host/test/env.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index a96ccf98b15..cf1a11cd23e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -263,7 +263,9 @@ else() list(APPEND CCHOST_SOURCES src/host/snmalloc.cpp) endif() -list(APPEND CCHOST_SOURCES ${CCF_DIR}/src/host/main.cpp) +list(APPEND CCHOST_SOURCES ${CCF_DIR}/src/host/main.cpp + ${CCF_DIR}/src/ds/nonstd.cpp ${CCF_DIR}/src/host/env.cpp +) if(COMPILE_TARGET STREQUAL "sgx") list(APPEND CCHOST_SOURCES ${CCF_GENERATED_DIR}/ccf_u.cpp) @@ -625,6 +627,7 @@ add_custom_target(ccf ALL) set(CCF_IMPL_SOURCE ${CCF_DIR}/src/enclave/main.cpp ${CCF_DIR}/src/enclave/enclave_time.cpp ${CCF_DIR}/src/enclave/thread_local.cpp ${CCF_DIR}/src/node/quote.cpp + ${CCF_DIR}/src/ds/nonstd.cpp ) if(COMPILE_TARGET STREQUAL "sgx") @@ -1018,6 +1021,7 @@ if(BUILD_TESTS) add_test_bin( kp_cert_test ${CMAKE_CURRENT_SOURCE_DIR}/src/crypto/test/kp_cert.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/src/ds/nonstd.cpp ) target_link_libraries(kp_cert_test PRIVATE ${CMAKE_THREAD_LIBS_INIT}) @@ -1091,7 +1095,7 @@ if(BUILD_TESTS) ) # Merkle Tree memory test - add_executable(merkle_mem src/node/test/merkle_mem.cpp) + add_executable(merkle_mem src/node/test/merkle_mem.cpp src/ds/nonstd.cpp) target_compile_options(merkle_mem PRIVATE ${COMPILE_LIBCXX}) target_link_libraries( merkle_mem PRIVATE ${CMAKE_THREAD_LIBS_INIT} ${LINK_LIBCXX} @@ -1101,7 +1105,7 @@ if(BUILD_TESTS) # Raft driver and scenario test add_executable( raft_driver ${CMAKE_CURRENT_SOURCE_DIR}/src/consensus/aft/test/driver.cpp - src/enclave/thread_local.cpp + src/enclave/thread_local.cpp src/ds/nonstd.cpp ) target_link_libraries(raft_driver PRIVATE ccfcrypto.host) target_include_directories(raft_driver PRIVATE src/aft) diff --git a/cmake/common.cmake b/cmake/common.cmake index c2f98c0cb74..c0b5964d482 100644 --- a/cmake/common.cmake +++ b/cmake/common.cmake @@ -3,7 +3,10 @@ # Unit test wrapper function(add_unit_test name) - add_executable(${name} ${CCF_DIR}/src/enclave/thread_local.cpp ${ARGN}) + add_executable( + ${name} ${CCF_DIR}/src/enclave/thread_local.cpp + ${CCF_DIR}/src/ds/nonstd.cpp ${ARGN} + ) target_compile_options(${name} PRIVATE ${COMPILE_LIBCXX}) target_include_directories( ${name} PRIVATE src ${CCFCRYPTO_INC} ${CCF_DIR}/3rdparty/test @@ -349,7 +352,7 @@ function(add_picobench name) PARSE_ARGV 1 PARSED_ARGS "" "" "SRCS;INCLUDE_DIRS;LINK_LIBS" ) - add_executable(${name} ${PARSED_ARGS_SRCS}) + add_executable(${name} src/ds/nonstd.cpp ${PARSED_ARGS_SRCS}) target_include_directories(${name} PRIVATE src ${PARSED_ARGS_INCLUDE_DIRS}) diff --git a/include/ccf/ds/nonstd.h b/include/ccf/ds/nonstd.h index c7dc4780acf..6180fb0c61c 100644 --- a/include/ccf/ds/nonstd.h +++ b/include/ccf/ds/nonstd.h @@ -2,10 +2,8 @@ // Licensed under the Apache 2.0 License. #pragma once -#include #include #include -#include #include #include #include @@ -174,64 +172,9 @@ namespace ccf::nonstd /** These convert strings to upper or lower case, in-place */ - static inline void to_upper(std::string& s) - { - std::transform(s.begin(), s.end(), s.begin(), [](unsigned char c) { - return std::toupper(c); - }); - } - - static inline void to_lower(std::string& s) - { - std::transform(s.begin(), s.end(), s.begin(), [](unsigned char c) { - return std::tolower(c); - }); - } - - // Iterators for map-keys and map-values - template - class KeyIterator : public TMapIterator - { - public: - KeyIterator() : TMapIterator() {} - KeyIterator(TMapIterator it) : TMapIterator(it) {} - - using Key = - typename std::iterator_traits::value_type::first_type; - using value_type = Key; + void to_upper(std::string& s); - Key* operator->() - { - return TMapIterator::operator->()->first; - } - - Key operator*() - { - return TMapIterator::operator*().first; - } - }; - - template - class ValueIterator : public TMapIterator - { - public: - ValueIterator() : TMapIterator() {} - ValueIterator(TMapIterator it) : TMapIterator(it) {} - - using Value = - typename std::iterator_traits::value_type::second_type; - using value_type = Value; - - Value* operator->() - { - return TMapIterator::operator->()->second; - } - - Value operator*() - { - return TMapIterator::operator*().second; - } - }; + void to_lower(std::string& s); /// Iterate through tuple, calling functor on each element template @@ -243,81 +186,4 @@ namespace ccf::nonstd tuple_for_each(t, f); } } - - static inline std::string expand_envvar(const std::string& str) - { - if (str.empty() || str[0] != '$') - { - return str; - } - - char* e = std::getenv(str.c_str() + 1); - if (e == nullptr) - { - return str; - } - else - { - return std::string(e); - } - } - - static inline std::string expand_envvars_in_path(const std::string& str) - { - std::filesystem::path path(str); - - if (path.empty()) - { - return str; - } - - std::vector elements; - auto it = path.begin(); - if (path.has_root_directory()) - { - ++it; - elements.push_back(path.root_directory()); - } - - while (it != path.end()) - { - elements.push_back(expand_envvar(*it++)); - } - - std::filesystem::path resolved; - for (auto& element : elements) - { - resolved /= element; - } - - return resolved.lexically_normal().string(); - } - - static inline std::string camel_case( - std::string s, - // Should the first character be upper-cased? - bool camel_first = true, - // Regex fragment to identify which characters should be upper-cased, by - // matching a separator preceding them. Default is to match any - // non-alphanumeric character - const std::string& separator_regex = "[^[:alnum:]]") - { - // Replacement is always a 1-character string - std::string replacement(1, '\0'); - - std::string prefix_matcher = - camel_first ? fmt::format("(^|{})", separator_regex) : separator_regex; - std::regex re(prefix_matcher + "[a-z]"); - std::smatch match; - - while (std::regex_search(s, match, re)) - { - // Replacement is the upper-casing of the final character from the match - replacement[0] = std::toupper(match.str()[match.length() - 1]); - - s = s.replace(match.position(), match.length(), replacement); - } - - return s; - } } \ No newline at end of file diff --git a/src/ds/nonstd.cpp b/src/ds/nonstd.cpp new file mode 100644 index 00000000000..ef103c1e786 --- /dev/null +++ b/src/ds/nonstd.cpp @@ -0,0 +1,54 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the Apache 2.0 License. + +#include "ccf/ds/nonstd.h" + +#include "ds/nonstd.h" + +#include +#include +#include + +#define FMT_HEADER_ONLY +#include + +namespace ccf::nonstd +{ + // Implementations for ccf/ds/nonstd.h + void to_upper(std::string& s) + { + std::transform(s.begin(), s.end(), s.begin(), [](unsigned char c) { + return std::toupper(c); + }); + } + + void to_lower(std::string& s) + { + std::transform(s.begin(), s.end(), s.begin(), [](unsigned char c) { + return std::tolower(c); + }); + } + + // Implementations for ds/nonstd.h + std::string camel_case( + std::string s, bool camel_first, const std::string& separator_regex) + { + // Replacement is always a 1-character string + std::string replacement(1, '\0'); + + std::string prefix_matcher = + camel_first ? fmt::format("(^|{})", separator_regex) : separator_regex; + std::regex re(prefix_matcher + "[a-z]"); + std::smatch match; + + while (std::regex_search(s, match, re)) + { + // Replacement is the upper-casing of the final character from the match + replacement[0] = std::toupper(match.str()[match.length() - 1]); + + s = s.replace(match.position(), match.length(), replacement); + } + + return s; + } +} diff --git a/src/ds/nonstd.h b/src/ds/nonstd.h new file mode 100644 index 00000000000..f1b1674eaa8 --- /dev/null +++ b/src/ds/nonstd.h @@ -0,0 +1,61 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the Apache 2.0 License. +#pragma once +#include + +namespace ccf::nonstd +{ + // Iterators for map-keys and map-values + template + class KeyIterator : public TMapIterator + { + public: + KeyIterator() : TMapIterator() {} + KeyIterator(TMapIterator it) : TMapIterator(it) {} + + using Key = + typename std::iterator_traits::value_type::first_type; + using value_type = Key; + + Key* operator->() + { + return TMapIterator::operator->()->first; + } + + Key operator*() + { + return TMapIterator::operator*().first; + } + }; + + template + class ValueIterator : public TMapIterator + { + public: + ValueIterator() : TMapIterator() {} + ValueIterator(TMapIterator it) : TMapIterator(it) {} + + using Value = + typename std::iterator_traits::value_type::second_type; + using value_type = Value; + + Value* operator->() + { + return TMapIterator::operator->()->second; + } + + Value operator*() + { + return TMapIterator::operator*().second; + } + }; + + std::string camel_case( + std::string s, + // Should the first character be upper-cased? + bool camel_first = true, + // Regex fragment to identify which characters should be upper-cased, by + // matching a separator preceding them. Default is to match any + // non-alphanumeric character + const std::string& separator_regex = "[^[:alnum:]]"); +} diff --git a/src/ds/test/nonstd.cpp b/src/ds/test/nonstd.cpp index 7d2122bf0bb..7d902e025f6 100644 --- a/src/ds/test/nonstd.cpp +++ b/src/ds/test/nonstd.cpp @@ -3,6 +3,8 @@ #include "ccf/ds/nonstd.h" +#include "ds/nonstd.h" + #include #include #include @@ -264,63 +266,6 @@ TEST_CASE("rsplit" * doctest::test_suite("nonstd")) } } -TEST_CASE("envvars" * doctest::test_suite("nonstd")) -{ - { - INFO("Expand environment variable"); - - std::string test_value("test_value"); - ::setenv("TEST_ENV_VAR", test_value.c_str(), 1); - - REQUIRE("" == ccf::nonstd::expand_envvar("")); - REQUIRE("not an env var" == ccf::nonstd::expand_envvar("not an env var")); - REQUIRE( - "$ENV_VAR_NOT_SET" == ccf::nonstd::expand_envvar("$ENV_VAR_NOT_SET")); - REQUIRE(test_value == ccf::nonstd::expand_envvar("$TEST_ENV_VAR")); - - // ${} syntax is not supported - REQUIRE( - "${ENV_VAR_NOT_SET}" == ccf::nonstd::expand_envvar("${ENV_VAR_NOT_SET}")); - } - { - INFO("Expand path"); - - std::string test_value1("test_value1"); - ::setenv("TEST_ENV_VAR1", test_value1.c_str(), 1); - std::string test_value2("test_value2"); - ::setenv("TEST_ENV_VAR2", test_value2.c_str(), 1); - - REQUIRE("" == ccf::nonstd::expand_envvars_in_path("")); - REQUIRE("foo" == ccf::nonstd::expand_envvars_in_path("foo")); - REQUIRE("foo/" == ccf::nonstd::expand_envvars_in_path("foo/")); - REQUIRE("foo/bar" == ccf::nonstd::expand_envvars_in_path("foo/bar")); - REQUIRE("/" == ccf::nonstd::expand_envvars_in_path("/")); - REQUIRE("/foo" == ccf::nonstd::expand_envvars_in_path("/foo")); - REQUIRE("/foo/" == ccf::nonstd::expand_envvars_in_path("/foo/")); - REQUIRE("/foo/bar" == ccf::nonstd::expand_envvars_in_path("/foo/bar")); - - REQUIRE( - fmt::format("{}", test_value1) == - ccf::nonstd::expand_envvars_in_path("$TEST_ENV_VAR1")); - REQUIRE( - fmt::format("{}/", test_value1) == - ccf::nonstd::expand_envvars_in_path("$TEST_ENV_VAR1/")); - REQUIRE( - fmt::format("{}/{}", test_value1, test_value2) == - ccf::nonstd::expand_envvars_in_path("$TEST_ENV_VAR1/$TEST_ENV_VAR2")); - - REQUIRE( - fmt::format("/{}", test_value1) == - ccf::nonstd::expand_envvars_in_path("/$TEST_ENV_VAR1")); - REQUIRE( - fmt::format("/{}/", test_value1) == - ccf::nonstd::expand_envvars_in_path("/$TEST_ENV_VAR1/")); - REQUIRE( - fmt::format("/{}/{}", test_value1, test_value2) == - ccf::nonstd::expand_envvars_in_path("/$TEST_ENV_VAR1/$TEST_ENV_VAR2")); - } -} - TEST_CASE("camel_case" * doctest::test_suite("nonstd")) { { diff --git a/src/endpoints/endpoint_registry.cpp b/src/endpoints/endpoint_registry.cpp index e84a2bead4e..532b3290f6f 100644 --- a/src/endpoints/endpoint_registry.cpp +++ b/src/endpoints/endpoint_registry.cpp @@ -5,6 +5,7 @@ #include "ccf/common_auth_policies.h" #include "ccf/pal/locking.h" +#include "ds/nonstd.h" #include "http/http_parser.h" #include "node/rpc_context_impl.h" diff --git a/src/host/env.cpp b/src/host/env.cpp new file mode 100644 index 00000000000..f43607913ea --- /dev/null +++ b/src/host/env.cpp @@ -0,0 +1,60 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the Apache 2.0 License. + +#include "host/env.h" + +#include +#include +#include + +namespace ccf::env +{ + std::string expand_envvar(const std::string& str) + { + if (str.empty() || str[0] != '$') + { + return str; + } + + char* e = std::getenv(str.c_str() + 1); + if (e == nullptr) + { + return str; + } + else + { + return std::string(e); + } + } + + std::string expand_envvars_in_path(const std::string& str) + { + std::filesystem::path path(str); + + if (path.empty()) + { + return str; + } + + std::vector elements; + auto it = path.begin(); + if (path.has_root_directory()) + { + ++it; + elements.push_back(path.root_directory()); + } + + while (it != path.end()) + { + elements.push_back(expand_envvar(*it++)); + } + + std::filesystem::path resolved; + for (auto& element : elements) + { + resolved /= element; + } + + return resolved.lexically_normal().string(); + } +} diff --git a/src/host/env.h b/src/host/env.h new file mode 100644 index 00000000000..ac2f0da69a8 --- /dev/null +++ b/src/host/env.h @@ -0,0 +1,12 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the Apache 2.0 License. +#pragma once + +#include + +namespace ccf::env +{ + std::string expand_envvar(const std::string& str); + + std::string expand_envvars_in_path(const std::string& str); +} diff --git a/src/host/main.cpp b/src/host/main.cpp index 2a89bbc192a..847a5bd81a5 100644 --- a/src/host/main.cpp +++ b/src/host/main.cpp @@ -11,10 +11,12 @@ #include "ds/cli_helper.h" #include "ds/files.h" #include "ds/non_blocking.h" +#include "ds/nonstd.h" #include "ds/oversized.h" #include "ds/x509_time_fmt.h" #include "enclave.h" #include "handle_ring_buffer.h" +#include "host/env.h" #include "json_schema.h" #include "lfs_file_handler.h" #include "load_monitor.h" @@ -511,7 +513,7 @@ int main(int argc, char** argv) LOG_DEBUG_FMT( "Resolving snp_security_policy_file: {}", security_policy_file); security_policy_file = - ccf::nonstd::expand_envvars_in_path(security_policy_file); + ccf::env::expand_envvars_in_path(security_policy_file); LOG_DEBUG_FMT( "Resolved snp_security_policy_file: {}", security_policy_file); @@ -526,7 +528,7 @@ int main(int argc, char** argv) LOG_DEBUG_FMT( "Resolving snp_uvm_endorsements_file: {}", snp_uvm_endorsements_file); snp_uvm_endorsements_file = - ccf::nonstd::expand_envvars_in_path(snp_uvm_endorsements_file); + ccf::env::expand_envvars_in_path(snp_uvm_endorsements_file); LOG_DEBUG_FMT( "Resolved snp_uvm_endorsements_file: {}", snp_uvm_endorsements_file); @@ -549,14 +551,14 @@ int main(int argc, char** argv) auto pos = url.find(':'); if (pos == std::string::npos) { - endorsement_servers_it->url = ccf::nonstd::expand_envvar(url); + endorsement_servers_it->url = ccf::env::expand_envvar(url); } else { endorsement_servers_it->url = fmt::format( "{}:{}", - ccf::nonstd::expand_envvar(url.substr(0, pos)), - ccf::nonstd::expand_envvar(url.substr(pos + 1))); + ccf::env::expand_envvar(url.substr(0, pos)), + ccf::env::expand_envvar(url.substr(pos + 1))); } LOG_DEBUG_FMT( "Resolved snp_endorsements_server url: {}", diff --git a/src/host/test/env.cpp b/src/host/test/env.cpp new file mode 100644 index 00000000000..fd01437697a --- /dev/null +++ b/src/host/test/env.cpp @@ -0,0 +1,63 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the Apache 2.0 License. + +#include "host/env.h" + +#include + +TEST_CASE("envvars" * doctest::test_suite("env")) +{ + { + INFO("Expand environment variable"); + + std::string test_value("test_value"); + ::setenv("TEST_ENV_VAR", test_value.c_str(), 1); + + REQUIRE("" == ccf::nonstd::expand_envvar("")); + REQUIRE("not an env var" == ccf::nonstd::expand_envvar("not an env var")); + REQUIRE( + "$ENV_VAR_NOT_SET" == ccf::nonstd::expand_envvar("$ENV_VAR_NOT_SET")); + REQUIRE(test_value == ccf::nonstd::expand_envvar("$TEST_ENV_VAR")); + + // ${} syntax is not supported + REQUIRE( + "${ENV_VAR_NOT_SET}" == ccf::nonstd::expand_envvar("${ENV_VAR_NOT_SET}")); + } + { + INFO("Expand path"); + + std::string test_value1("test_value1"); + ::setenv("TEST_ENV_VAR1", test_value1.c_str(), 1); + std::string test_value2("test_value2"); + ::setenv("TEST_ENV_VAR2", test_value2.c_str(), 1); + + REQUIRE("" == ccf::nonstd::expand_envvars_in_path("")); + REQUIRE("foo" == ccf::nonstd::expand_envvars_in_path("foo")); + REQUIRE("foo/" == ccf::nonstd::expand_envvars_in_path("foo/")); + REQUIRE("foo/bar" == ccf::nonstd::expand_envvars_in_path("foo/bar")); + REQUIRE("/" == ccf::nonstd::expand_envvars_in_path("/")); + REQUIRE("/foo" == ccf::nonstd::expand_envvars_in_path("/foo")); + REQUIRE("/foo/" == ccf::nonstd::expand_envvars_in_path("/foo/")); + REQUIRE("/foo/bar" == ccf::nonstd::expand_envvars_in_path("/foo/bar")); + + REQUIRE( + fmt::format("{}", test_value1) == + ccf::nonstd::expand_envvars_in_path("$TEST_ENV_VAR1")); + REQUIRE( + fmt::format("{}/", test_value1) == + ccf::nonstd::expand_envvars_in_path("$TEST_ENV_VAR1/")); + REQUIRE( + fmt::format("{}/{}", test_value1, test_value2) == + ccf::nonstd::expand_envvars_in_path("$TEST_ENV_VAR1/$TEST_ENV_VAR2")); + + REQUIRE( + fmt::format("/{}", test_value1) == + ccf::nonstd::expand_envvars_in_path("/$TEST_ENV_VAR1")); + REQUIRE( + fmt::format("/{}/", test_value1) == + ccf::nonstd::expand_envvars_in_path("/$TEST_ENV_VAR1/")); + REQUIRE( + fmt::format("/{}/{}", test_value1, test_value2) == + ccf::nonstd::expand_envvars_in_path("/$TEST_ENV_VAR1/$TEST_ENV_VAR2")); + } +} diff --git a/src/http/http2_utils.h b/src/http/http2_utils.h index d8b94e88c8d..e1ad5f21b51 100644 --- a/src/http/http2_utils.h +++ b/src/http/http2_utils.h @@ -2,8 +2,8 @@ // Licensed under the Apache 2.0 License. #pragma once -#include "ccf/ds/nonstd.h" #include "ccf/http_header_map.h" +#include "ds/nonstd.h" #include #include diff --git a/tests/perf-system/submitter/CMakeLists.txt b/tests/perf-system/submitter/CMakeLists.txt index 45e9f9c45d4..8f20d3ad2c4 100644 --- a/tests/perf-system/submitter/CMakeLists.txt +++ b/tests/perf-system/submitter/CMakeLists.txt @@ -2,7 +2,7 @@ set(SUBMITTER_DIR ${CCF_DIR}/tests/perf-system/submitter) add_executable( submit ${SUBMITTER_DIR}/submit.cpp ${SUBMITTER_DIR}/handle_arguments.h - ${SUBMITTER_DIR}/parquet_data.h + ${SUBMITTER_DIR}/parquet_data.h ${CCF_DIR}/src/ds/nonstd.cpp ) add_library(stdcxxhttp_parser.host "${HTTP_PARSER_SOURCES}") From 9ccd6937854efa2442af82f1c7ddaae614a64eb4 Mon Sep 17 00:00:00 2001 From: Eddy Ashton Date: Thu, 11 Jul 2024 14:08:38 +0100 Subject: [PATCH 2/2] Add nonstd.cpp to tpcc_client (#6354) --- src/apps/tpcc/tpcc.cmake | 1 + 1 file changed, 1 insertion(+) diff --git a/src/apps/tpcc/tpcc.cmake b/src/apps/tpcc/tpcc.cmake index 3eed0b546fd..18918e3631d 100644 --- a/src/apps/tpcc/tpcc.cmake +++ b/src/apps/tpcc/tpcc.cmake @@ -4,6 +4,7 @@ add_client_exe( tpcc_client SRCS ${CMAKE_CURRENT_LIST_DIR}/clients/tpcc_client.cpp + ${CCF_DIR}/src/ds/nonstd.cpp ) if(CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 9) target_link_libraries(tpcc_client PRIVATE http_parser.host ccfcrypto.host)