From 395013e3cebf629aa730064b641d30752a3b8272 Mon Sep 17 00:00:00 2001 From: Andrey Saranchin Date: Fri, 28 Mar 2025 14:30:44 +0300 Subject: [PATCH 1/5] misc: remove trailing whitespaces in a bunch of files Trailing whitespaces are forbidden by any codestyle, so the commit removes them. Also, they can become annoying because some IDEs can automatically remove them turning diff of a patch into a mess. --- src/Client/Connection.hpp | 2 +- src/Client/ResponseReader.hpp | 2 +- src/Utils/Base64.hpp | 2 +- src/Utils/Mempool.hpp | 4 ++-- src/mpp/Dec.hpp | 2 +- test/ClientTest.cpp | 18 +++++++++--------- test/EncDecTest.cpp | 14 +++++++------- 7 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/Client/Connection.hpp b/src/Client/Connection.hpp index 7715a715c..d9d745f4f 100644 --- a/src/Client/Connection.hpp +++ b/src/Client/Connection.hpp @@ -177,7 +177,7 @@ class Connection template rid_t call(const std::string &func, const T &args); rid_t ping(); - + /** * Execute the SQL statement contained in the 'statement' parameter. * @param statement statement, which should conform to the rules for SQL grammar diff --git a/src/Client/ResponseReader.hpp b/src/Client/ResponseReader.hpp index aa8ac5b82..ef15624b5 100644 --- a/src/Client/ResponseReader.hpp +++ b/src/Client/ResponseReader.hpp @@ -99,7 +99,7 @@ template struct Data { using it_t = iterator_t; std::pair iters; - + /** Unpacks tuples to passed container. */ template bool decode(T& tuples) diff --git a/src/Utils/Base64.hpp b/src/Utils/Base64.hpp index 73686f636..bb6b4d0ca 100644 --- a/src/Utils/Base64.hpp +++ b/src/Utils/Base64.hpp @@ -135,7 +135,7 @@ std::pair decode(INP first, INP last, OUT dest) "\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377" "\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377" "\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377"; - + while (true) { if (BASE64_UNLIKELY(first == last)) return {first, dest}; diff --git a/src/Utils/Mempool.hpp b/src/Utils/Mempool.hpp index f7b3287bf..76bd61723 100644 --- a/src/Utils/Mempool.hpp +++ b/src/Utils/Mempool.hpp @@ -104,7 +104,7 @@ class MempoolInstance : public MempoolStats { static_assert(sizeof(Slab) == B * M, "Smth went wrong"); static constexpr size_t FIRST_OFFSET = B - sizeof(Slab::next); - using Stats_t = MempoolStats; + using Stats_t = MempoolStats; public: // Constants for stat. @@ -253,7 +253,7 @@ class MempoolStatic { public: static char *allocate() { return instance().allocate(); } static void deallocate(char *ptr) noexcept { instance().deallocate(ptr); } - int selfcheck() const { return instance().selfcheck(); } + int selfcheck() const { return instance().selfcheck(); } static constexpr size_t REAL_SIZE = Base_t::REAL_SIZE; static constexpr size_t BLOCK_SIZE = Base_t::BLOCK_SIZE; diff --git a/src/mpp/Dec.hpp b/src/mpp/Dec.hpp index 2b6311678..76f9c0f0b 100644 --- a/src/mpp/Dec.hpp +++ b/src/mpp/Dec.hpp @@ -60,7 +60,7 @@ constexpr bool is_any_putable_v = /** * If it is true, the object of type T will not be decoded - raw data will * be saved to it. - * + * * Now it supports only a pair of iterators (probably, wrapped with * mpp::as_raw). The check implicilty implies that BUF is an iterator, not * buffer - it would be strange to pass a pair of buffer to decoder. diff --git a/test/ClientTest.cpp b/test/ClientTest.cpp index 6ebccab0b..f98681ad9 100644 --- a/test/ClientTest.cpp +++ b/test/ClientTest.cpp @@ -114,7 +114,7 @@ printDatum(const Datum &datum) */ template > void -printResponse(Response &response, Data data = std::vector()) +printResponse(Response &response, Data data = std::vector()) { if (response.body.error_stack != std::nullopt) { Error err = (*response.body.error_stack)[0]; @@ -699,7 +699,7 @@ class StmtProcessorPrepare { std::string &stmt) { rid_t future = conn.prepare(stmt); - + client.wait(conn, future, WAIT_TIMEOUT); fail_unless(conn.futureIsReady(future)); std::optional> response = conn.getResponse(future); @@ -769,7 +769,7 @@ single_conn_sql(Connector &client) "COLUMN2 VARCHAR(50), COLUMN3 DOUBLE);"; auto stmt = StmtProcessor::process(client, conn, stmt_str); rid_t create_table = conn.execute(stmt, std::make_tuple()); - + client.wait(conn, create_table, WAIT_TIMEOUT); fail_unless(conn.futureIsReady(create_table)); std::optional> response = conn.getResponse(create_table); @@ -784,7 +784,7 @@ single_conn_sql(Connector &client) stmt_str = "INSERT INTO TSQL VALUES (20, 'first', 3.2), (21, 'second', 5.4)"; stmt = StmtProcessor::process(client, conn, stmt_str); rid_t insert = conn.execute(stmt, std::make_tuple()); - + client.wait(conn, insert, WAIT_TIMEOUT); fail_unless(conn.futureIsReady(insert)); response = conn.getResponse(insert); @@ -803,7 +803,7 @@ single_conn_sql(Connector &client) stmt_str = "INSERT INTO TSQL VALUES (?, ?, ?), (?, ?, ?), (?, ?, ?);"; stmt = StmtProcessor::process(client, conn, stmt_str); rid_t insert_args = conn.execute(stmt, args); - + client.wait(conn, insert_args, WAIT_TIMEOUT); fail_unless(conn.futureIsReady(insert_args)); response = conn.getResponse(insert_args); @@ -818,7 +818,7 @@ single_conn_sql(Connector &client) stmt_str = "SELECT * FROM SEQSCAN TSQL;"; stmt = StmtProcessor::process(client, conn, stmt_str); rid_t select = conn.execute(stmt, std::make_tuple()); - + client.wait(conn, select, WAIT_TIMEOUT); fail_unless(conn.futureIsReady(select)); response = conn.getResponse(select); @@ -841,7 +841,7 @@ single_conn_sql(Connector &client) stmt_str = "DROP TABLE IF EXISTS TSQL;"; stmt = StmtProcessor::process(client, conn, stmt_str); rid_t drop_table = conn.execute(stmt, std::make_tuple()); - + client.wait(conn, drop_table, WAIT_TIMEOUT); fail_unless(conn.futureIsReady(drop_table)); response = conn.getResponse(drop_table); @@ -902,7 +902,7 @@ single_conn_sql(Connector &client) stmt_str = "SELECT * FROM SEQSCAN TSQL;"; stmt = StmtProcessor::process(client, conn, stmt_str); select = conn.execute(stmt, std::make_tuple()); - + client.wait(conn, select, WAIT_TIMEOUT); fail_unless(conn.futureIsReady(select)); response = conn.getResponse(select); @@ -955,7 +955,7 @@ single_conn_sql(Connector &client) stmt_str = "UPDATE \"_session_settings\" SET \"value\" = false WHERE \"name\" = 'sql_full_metadata';"; stmt = StmtProcessor::process(client, conn, stmt_str); rid_t disable_metadata = conn.execute(stmt, std::make_tuple()); - + client.wait(conn, disable_metadata, WAIT_TIMEOUT); fail_unless(conn.futureIsReady(disable_metadata)); response = conn.getResponse(disable_metadata); diff --git a/test/EncDecTest.cpp b/test/EncDecTest.cpp index fb9694426..1eeb20687 100644 --- a/test/EncDecTest.cpp +++ b/test/EncDecTest.cpp @@ -1128,7 +1128,7 @@ test_raw() using it_t = Buf_t::iterator_common; it_t run = buf.begin(); - + std::pair to_wrap; auto raw_decoders = std::make_tuple( std::pair(), @@ -1148,7 +1148,7 @@ test_raw() fail_if(dec2.first != begin); fail_if(dec2.second != end); }; - + auto check_raw_decoders = [&](auto& begin, auto& end) { std::apply([&](auto&... decs){( ..., @@ -1185,7 +1185,7 @@ test_raw() const auto arr_end = run; TEST_CASE("decode the first element of array"); run = svp; - begin = run; + begin = run; mpp::decode(run, mpp::as_arr(std::forward_as_tuple(num))); fail_if(num != arr[0]); fail_if(run != arr_end); @@ -1203,7 +1203,7 @@ test_raw() auto elem_begin = begin + 1; auto elem_end = elem_begin; mpp::decode(elem_end, num); - check_each_raw_decoder(elem_begin, elem_end); + check_each_raw_decoder(elem_begin, elem_end); TEST_CASE("decode the array key by key"); run = svp; // Array is small - its header occupies one byte. @@ -1227,7 +1227,7 @@ test_raw() const auto map_end = run; TEST_CASE("decode one value from map"); run = svp; - begin = run; + begin = run; mpp::decode(run, mpp::as_map(std::forward_as_tuple(1, num))); fail_if(run != map_end); fail_if(num != 2); @@ -1248,7 +1248,7 @@ test_raw() elem_end = elem_begin; // Skip value. mpp::decode(elem_end, num); - check_each_raw_decoder(elem_begin, elem_end); + check_each_raw_decoder(elem_begin, elem_end); TEST_CASE("decode the map key by key"); run = svp; // Map is small - its header occupies one byte. @@ -1292,7 +1292,7 @@ test_variant() mpp::encode(buf, wr); mpp::decode(run, rd); fail_unless(wr == rd); - + wr.emplace<2>("string variant"); mpp::encode(buf, wr); mpp::decode(run, rd); From bb69791f1a2fecddb128436f7364c1e3681fc8d0 Mon Sep 17 00:00:00 2001 From: Andrey Saranchin Date: Fri, 28 Mar 2025 14:25:13 +0300 Subject: [PATCH 2/5] test: rename bench_func to remote_echo The function simply returns passed arguments, just like echo. Let's call it with more convenient name that doesn't have anything in common with benchmarks to use it in tests later. Part of #108 --- test/ClientPerfTest.cpp | 2 +- test/cfg.lua | 2 +- test/cfg_ssl.lua | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/ClientPerfTest.cpp b/test/ClientPerfTest.cpp index 6494ab841..f5cd9238a 100644 --- a/test/ClientPerfTest.cpp +++ b/test/ClientPerfTest.cpp @@ -100,7 +100,7 @@ executeRequest(Connection &conn, int request_type, int key) case Iproto::SELECT: return conn.space[space_id].select(std::make_tuple(key)); case Iproto::CALL: - return conn.call("bench_func", std::make_tuple(1, 2, 3, 4, 5)); + return conn.call("remote_echo", std::make_tuple(1, 2, 3, 4, 5)); default: abort(); } diff --git a/test/cfg.lua b/test/cfg.lua index c46b2fa6b..10a8a8800 100644 --- a/test/cfg.lua +++ b/test/cfg.lua @@ -30,7 +30,7 @@ function remote_map() return {key = 777} end -function bench_func(...) +function remote_echo(...) return {...} end diff --git a/test/cfg_ssl.lua b/test/cfg_ssl.lua index a201909dc..0718cee72 100644 --- a/test/cfg_ssl.lua +++ b/test/cfg_ssl.lua @@ -28,7 +28,7 @@ function remote_multi() return 'Hello', 1, 6.66 end -function bench_func(...) +function remote_echo(...) return {...} end From f24d1690d788a1182eb757c158a9c1d76c871e50 Mon Sep 17 00:00:00 2001 From: Andrey Saranchin Date: Fri, 28 Mar 2025 15:12:17 +0300 Subject: [PATCH 3/5] client: do not wrap call args into array All connection methods do not wrap keys and tuples into arrays - it's responsibility of user to follow IPROTO format. The only exception is method `call` (by mistake, I believe) - it wraps arguments into array and it doesn't allow to pass raw MsgPack to this method since `mpp::as_raw` tag does not work with any other tags. The commit makes method `call` work like other methods - it won't wrap `args` by itself anymore. Closes #108 --- src/Client/RequestEncoder.hpp | 2 +- test/ClientTest.cpp | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/Client/RequestEncoder.hpp b/src/Client/RequestEncoder.hpp index 40fda228c..2752d3b18 100644 --- a/src/Client/RequestEncoder.hpp +++ b/src/Client/RequestEncoder.hpp @@ -318,7 +318,7 @@ RequestEncoder::encodeCall(const std::string &func, const T &args) encodeHeader(Iproto::CALL); mpp::encode(m_Buf, mpp::as_map(std::forward_as_tuple( MPP_AS_CONST(Iproto::FUNCTION_NAME), func, - MPP_AS_CONST(Iproto::TUPLE), mpp::as_arr(args)))); + MPP_AS_CONST(Iproto::TUPLE), args))); uint32_t request_size = (m_Buf.end() - request_start) - PREHEADER_SIZE; ++request_start; request_start.set(__builtin_bswap32(request_size)); diff --git a/test/ClientTest.cpp b/test/ClientTest.cpp index f98681ad9..29547c0c3 100644 --- a/test/ClientTest.cpp +++ b/test/ClientTest.cpp @@ -588,6 +588,7 @@ single_conn_call(Connector &client) const static char *return_multi = "remote_multi"; const static char *return_nil = "remote_nil"; const static char *return_map = "remote_map"; + const static char *echo = "remote_echo"; Connection conn(client); int rc = test_connect(client, conn, localhost, port); @@ -671,6 +672,15 @@ single_conn_call(Connector &client) printResponse(*response, std::make_tuple(std::make_tuple(std::make_pair("key", int())))); + TEST_CASE("call remote_echo with raw arguments"); + /* [1, 2, 3] as a raw MsgPack. */ + const char raw_data[4] = {static_cast(0x93), 1, 2, 3}; + rid_t f11 = conn.call(echo, mpp::as_raw(raw_data)); + client.wait(conn, f11, WAIT_TIMEOUT); + fail_unless(conn.futureIsReady(f11)); + response = conn.getResponse(f11); + printResponse(*response, std::make_tuple(std::make_tuple(0, 0, 0))); + client.close(conn); } From 280f39ba4b89cfb602d6d0aec737f340e13fb8f3 Mon Sep 17 00:00:00 2001 From: Andrey Saranchin Date: Fri, 28 Mar 2025 17:20:33 +0300 Subject: [PATCH 4/5] client: fix assertion triggered on data decoding failure After decoding response data we check if the decoder advanced the iterator to the end of the raw data. However, that's not true when someone tries to decode data with non-matching format. The commit fixes the assertion - we should check iterator only if data was successfully decoded. Closes #99 --- src/Client/ResponseReader.hpp | 2 +- test/ClientTest.cpp | 37 +++++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/Client/ResponseReader.hpp b/src/Client/ResponseReader.hpp index ef15624b5..8b213c9af 100644 --- a/src/Client/ResponseReader.hpp +++ b/src/Client/ResponseReader.hpp @@ -106,7 +106,7 @@ struct Data { { it_t itr = iters.first; bool ok = mpp::decode(itr, tuples); - assert(itr == iters.second); + assert(!ok || itr == iters.second); return ok; } diff --git a/test/ClientTest.cpp b/test/ClientTest.cpp index 29547c0c3..0fdde8c29 100644 --- a/test/ClientTest.cpp +++ b/test/ClientTest.cpp @@ -1093,6 +1093,42 @@ test_dead_connection_wait(Connector &client) #endif } +/** + * Test for miscellaneous issues related to response decoding. + */ +template +void +response_decoding(Connector &client) +{ + TEST_INIT(0); + + Connection conn(client); + int rc = test_connect(client, conn, localhost, port); + fail_unless(rc == 0); + + TEST_CASE("decode data with non-matching format"); + rid_t f = conn.call("remote_uint", std::make_tuple()); + client.wait(conn, f, WAIT_TIMEOUT); + fail_unless(conn.futureIsReady(f)); + std::optional> response = conn.getResponse(f); + + fail_unless(response.has_value()); + fail_unless(response->body.data.has_value()); + + std::string str; + unsigned num; + std::tuple arr_of_str; + std::tuple arr_of_num; + /* Try to decode data with non-matching format. */ + fail_if(response->body.data->decode(str)); + fail_if(response->body.data->decode(num)); + fail_if(response->body.data->decode(arr_of_str)); + /* We should successfully decode data after all. */ + fail_unless(response->body.data->decode(arr_of_num)); + + client.close(conn); +} + int main() { #ifdef TNTCXX_ENABLE_SSL @@ -1141,5 +1177,6 @@ int main() ::test_sigpipe(client); #endif ::test_dead_connection_wait(client); + response_decoding(client); return 0; } From f116f3cf4df1f445f9f715b12b334b2596960948 Mon Sep 17 00:00:00 2001 From: Andrey Saranchin Date: Mon, 31 Mar 2025 11:56:52 +0300 Subject: [PATCH 5/5] ci: replace macos-12 workflows with newer ones GitHub Actions does not support `macos-12` anymore - let's replace it with the newer `macos-15` version. What's about sanitizers - let's run them on `macos-14`, neither the newest nor the oldest MacOS version. --- .github/workflows/testing.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/testing.yml b/.github/workflows/testing.yml index 1c902b974..d67566278 100644 --- a/.github/workflows/testing.yml +++ b/.github/workflows/testing.yml @@ -16,9 +16,9 @@ jobs: runs-on: - ubuntu-20.04 - ubuntu-22.04 - - macos-12 - macos-13 - macos-14 + - macos-15 build-type: - Debug - RelWithDebInfo @@ -37,9 +37,9 @@ jobs: runs-on: - ubuntu-20.04 - ubuntu-22.04 - - macos-12 - macos-13 - macos-14 + - macos-15 compiler: - {c: gcc, cxx: g++} - {c: clang, cxx: clang++} @@ -54,12 +54,12 @@ jobs: - cxx-standard: 17 compiler: {c: gcc, cxx: g++} # gcc on macos is just an alias for clang - - runs-on: macos-12 - compiler: {c: gcc, cxx: g++} - runs-on: macos-13 compiler: {c: gcc, cxx: g++} - runs-on: macos-14 compiler: {c: gcc, cxx: g++} + - runs-on: macos-15 + compiler: {c: gcc, cxx: g++} name: build (${{ matrix.runs-on }}, ${{ matrix.build-type }}, ${{ matrix.compiler.c }}, C++${{ matrix.cxx-standard }}) with: build-only: true @@ -89,7 +89,7 @@ jobs: matrix: runs-on: - ubuntu-22.04 - - macos-12 + - macos-14 build-type: - Debug - RelWithDebInfo @@ -98,7 +98,7 @@ jobs: - {c: clang, cxx: clang++} # gcc on macos is just an alias for clang exclude: - - runs-on: macos-12 + - runs-on: macos-14 compiler: {c: gcc, cxx: g++} name: sanitizers (${{ matrix.runs-on }}, ${{ matrix.build-type }}, ${{ matrix.compiler.c }}) with: