diff --git a/indra/llcorehttp/tests/bufferarray_test.cpp b/indra/llcorehttp/tests/bufferarray_test.cpp index 65691b3779..f4b14aaead 100644 --- a/indra/llcorehttp/tests/bufferarray_test.cpp +++ b/indra/llcorehttp/tests/bufferarray_test.cpp @@ -207,13 +207,13 @@ TEST_CASE("BufferArray Tests: BufferArray multiple writes - sequential reads", " UNSCOPED_INFO("nothing written past what was read"); CHECK(buffer[4] == 'X'); - // Read some more without repositioning + // Read on from where the last read stopped memset(buffer, 'X', sizeof(buffer)); len = ba->read(12, buffer, sizeof(buffer)); CHECK((str1_len - 2) == len); CHECK(strncmp(buffer, str1+2, str1_len-2) == 0); UNSCOPED_INFO("nothing written past what was read"); - CHECK(buffer[str1_len-1] == 'X'); + CHECK(buffer[str1_len-2] == 'X'); // release the implicit reference, causing the object to be released ba->release(); @@ -244,7 +244,7 @@ TEST_CASE("BufferArray Tests: BufferArray overwrite spanning blocks and appendin len = ba->write(8, str2, str2_len); CHECK(str2_len == len); - // Leave position and read verifying content + // A zero-length read at the end of the data reads nothing memset(buffer, 'X', sizeof(buffer)); len = ba->read(8 + str2_len, buffer, 0); CHECK(len == 0); diff --git a/indra/llcorehttp/tests/bufferstream_test.cpp b/indra/llcorehttp/tests/bufferstream_test.cpp index ca2b81afc0..2536fe790c 100644 --- a/indra/llcorehttp/tests/bufferstream_test.cpp +++ b/indra/llcorehttp/tests/bufferstream_test.cpp @@ -46,7 +46,7 @@ typedef BufferArrayStreamBuf::traits_type tst_traits_t; TEST_CASE("BufferStream Tests: BufferArrayStreamBuf over a NULL BufferArray fails every call", "[BufferStream_Tests]") { - // create a new ref counted object with an implicit reference + // a stream buffer over no BufferArray BufferArrayStreamBuf * bsb = new BufferArrayStreamBuf(NULL); // Not much will work with a NULL @@ -58,7 +58,7 @@ TEST_CASE("BufferStream Tests: BufferArrayStreamBuf over a NULL BufferArray fail CHECK(bsb->xsputn("blah", 4) == 0); CHECK(bsb->seekoff(0, std::ios_base::beg, std::ios_base::in) == std::streampos(-1)); - // release the implicit reference, causing the object to be released + // the stream buffer is not ref counted delete bsb; bsb = NULL; } @@ -66,7 +66,7 @@ TEST_CASE("BufferStream Tests: BufferArrayStreamBuf over a NULL BufferArray fail TEST_CASE("BufferStream Tests: BufferArrayStream over a NULL BufferArray starts out good", "[BufferStream_Tests]") { - // create a new ref counted object with an implicit reference + // a stream over no BufferArray BufferArrayStream * bas = new BufferArrayStream(NULL); // Not much will work with a NULL here @@ -74,37 +74,39 @@ TEST_CASE("BufferStream Tests: BufferArrayStream over a NULL BufferArray starts CHECK_FALSE(bas->fail()); CHECK(bas->good()); - // release the implicit reference, causing the object to be released + // the stream is not ref counted delete bas; bas = NULL; } -TEST_CASE("BufferStream Tests: BufferArrayStreamBuf construction with empty BufferArray", "[BufferStream_Tests]") +TEST_CASE("BufferStream Tests: BufferArrayStreamBuf takes a reference on an empty BufferArray", "[BufferStream_Tests]") { // create a new ref counted BufferArray with implicit reference BufferArray * ba = new BufferArray; BufferArrayStreamBuf * bsb = new BufferArrayStreamBuf(ba); + CHECK(ba->getRefCount() == 2); // I can release my ref on the BA ba->release(); ba = NULL; - // release the implicit reference, causing the object to be released + // the stream buffer is not ref counted; deleting it drops the BufferArray's last reference delete bsb; bsb = NULL; } -TEST_CASE("BufferStream Tests: BufferArrayStream construction with empty BufferArray", "[BufferStream_Tests]") +TEST_CASE("BufferStream Tests: BufferArrayStream holds a reference on an empty BufferArray while it lives", "[BufferStream_Tests]") { // create a new ref counted BufferArray with implicit reference BufferArray * ba = new BufferArray; { - // create a new ref counted object with an implicit reference BufferArrayStream bas(ba); + CHECK(ba->getRefCount() == 2); } + CHECK(ba->getRefCount() == 1); // release the implicit reference, causing the object to be released ba->release(); @@ -151,9 +153,12 @@ TEST_CASE("BufferStream Tests: BufferArrayStreamBuf reads and appends and seeks UNSCOPED_INFO("showmanyc() picks up seekoff"); CHECK(bsb->showmanyc() == (c_len + 5 - 4)); CHECK(bsb->seekoff(0, std::ios_base::end, std::ios_base::in) == std::streampos(c_len + 4)); + // The seek to the end leaves the read position on the last character, the '!', + // so pbackfail() refuses the '!' and puts back the 'a' before it CHECK(tst_traits_t::eof() == bsb->pbackfail('!')); + CHECK(bsb->pbackfail('a') == 'a'); - // release the implicit reference, causing the object to be released + // the stream buffer is not ref counted; deleting it drops the BufferArray's last reference delete bsb; bsb = NULL; } diff --git a/indra/llcorehttp/tests/httprequest_test.cpp b/indra/llcorehttp/tests/httprequest_test.cpp index bb66f439e1..3e89f36e4f 100644 --- a/indra/llcorehttp/tests/httprequest_test.cpp +++ b/indra/llcorehttp/tests/httprequest_test.cpp @@ -38,7 +38,10 @@ #include #include "alregex.h" +#include +#include #include +#include #include "llcleanup.h" #include "llproxy.h" @@ -820,7 +823,7 @@ TEST_CASE_METHOD(HttpRequestTestData, "HttpRequest Tests: a GET to the peer is a } -TEST_CASE_METHOD(HttpRequestTestData, "HttpRequest Tests: requestGetByteRange with a zero range is answered 200", "[HttpRequest_Tests]") +TEST_CASE_METHOD(HttpRequestTestData, "HttpRequest Tests: a GET with a byte range sends its Range header and takes a 200 for the whole body", "[HttpRequest_Tests]") { ScopedCurlInit ready; @@ -835,6 +838,7 @@ TEST_CASE_METHOD(HttpRequestTestData, "HttpRequest Tests: requestGetByteRange wi mHandlerCalls = 0; HttpRequest * req = NULL; + HttpOptions::ptr_t opts; try { @@ -848,13 +852,22 @@ TEST_CASE_METHOD(HttpRequestTestData, "HttpRequest Tests: requestGetByteRange wi // create a new ref counted object with an implicit reference req = new HttpRequest(); - // Issue a GET that *can* connect + opts = HttpOptions::ptr_t(new HttpOptions()); + opts->setWantHeaders(true); + + // Issue a GET for 25 bytes from offset 100. The peer reflects the + // Range header it was sent and, ignoring it, answers 200 with the + // whole body, which has no Content-Range for llcorehttp to check. mStatus = HttpStatus(200); + handler.mHeadersRequired.push_back( + regex_container_t::value_type( + ALRegex("X-Reflect-range", ALRegex::ICASE), + ALRegex("bytes=100-124", ALRegex::ICASE))); HttpHandle handle = req->requestGetByteRange(HttpRequest::DEFAULT_POLICY_ID, - url_base, - 0, - 0, - HttpOptions::ptr_t(), + url_base + "reflect/", + 100, + 25, + opts, HttpHeaders::ptr_t(), handlerp); REQUIRE(handle != LLCORE_HTTP_HANDLE_INVALID); @@ -873,6 +886,7 @@ TEST_CASE_METHOD(HttpRequestTestData, "HttpRequest Tests: requestGetByteRange wi // Okay, request a shutdown of the servicing thread mStatus = HttpStatus(); + handler.mHeadersRequired.clear(); handle = req->requestStopThread(handlerp); REQUIRE(handle != LLCORE_HTTP_HANDLE_INVALID); @@ -897,6 +911,9 @@ TEST_CASE_METHOD(HttpRequestTestData, "HttpRequest Tests: requestGetByteRange wi } CHECK(HttpService::isStopped()); + // release options + opts.reset(); + // release the request object delete req; req = NULL; @@ -909,6 +926,7 @@ TEST_CASE_METHOD(HttpRequestTestData, "HttpRequest Tests: requestGetByteRange wi catch (...) { stop_thread(req); + opts.reset(); delete req; HttpRequest::destroyService(); throw; @@ -945,7 +963,7 @@ TEST_CASE_METHOD(HttpRequestTestData, "HttpRequest Tests: a PUT with a body to t // create a new ref counted object with an implicit reference req = new HttpRequest(); - // Issue a GET that *can* connect + // Issue a PUT that *can* connect static const char * body_text("Now is the time for all good men..."); body->append(body_text, strlen(body_text)); mStatus = HttpStatus(200); @@ -1050,7 +1068,7 @@ TEST_CASE_METHOD(HttpRequestTestData, "HttpRequest Tests: a POST with a body to // create a new ref counted object with an implicit reference req = new HttpRequest(); - // Issue a GET that *can* connect + // Issue a POST that *can* connect static const char * body_text("Now is the time for all good men..."); body->append(body_text, strlen(body_text)); mStatus = HttpStatus(200); @@ -1883,7 +1901,7 @@ TEST_CASE_METHOD(HttpRequestTestData, "HttpRequest Tests: a POST sends default h ALRegex(".*", ALRegex::ICASE))); handler.mHeadersDisallowed.push_back( regex_container_t::value_type( - ALRegex("X-Reflect-transfer_encoding", ALRegex::ICASE), + ALRegex("X-Reflect-transfer-encoding", ALRegex::ICASE), ALRegex(".*chunked.*", ALRegex::ICASE))); HttpHandle handle = req->requestPost(HttpRequest::DEFAULT_POLICY_ID, url_base + "reflect/", @@ -2401,7 +2419,7 @@ TEST_CASE_METHOD(HttpRequestTestData, "HttpRequest Tests: headers given to a POS ba = new BufferArray; ba->append(msg, strlen(msg)); - // Issue a default POST + // Issue a POST with modified headers mStatus = HttpStatus(200); handler.mHeadersRequired.push_back( regex_container_t::value_type( @@ -2613,7 +2631,7 @@ TEST_CASE_METHOD(HttpRequestTestData, "HttpRequest Tests: the last of several Co ba = new BufferArray; ba->append(msg, strlen(msg)); - // Issue a default PUT + // Issue a PUT with several Content-Types mStatus = HttpStatus(200); handler.mHeadersRequired.push_back( regex_container_t::value_type( @@ -2936,19 +2954,49 @@ TEST_CASE_METHOD(HttpRequestTestData, "HttpRequest Tests: BUG-2295 a 206 with a } } -TEST_CASE_METHOD(HttpRequestTestData, "HttpRequest Tests: GETs answered 503 with good and bad Retry-After headers complete", "[HttpRequest_Tests]") +// Records, by URL, when each response was delivered and the retries it reports +class RetryRecordingHandler : public TestHandler2 +{ +public: + struct Delivery + { + std::chrono::steady_clock::time_point mAt; + unsigned int mRetries; + unsigned int m503Retries; + }; + + RetryRecordingHandler(HttpRequestTestData * state, + const std::string & name) + : TestHandler2(state, name) + {} + + void onCompleted(HttpHandle handle, HttpResponse * response) override + { + TestHandler2::onCompleted(handle, response); + + Delivery delivery{ std::chrono::steady_clock::now(), 0, 0 }; + response->getRetries(&delivery.mRetries, &delivery.m503Retries); + mDeliveries[response->getRequestURL()] = delivery; + } + + std::map mDeliveries; +}; + +TEST_CASE_METHOD(HttpRequestTestData, "HttpRequest Tests: GETs answered 503 are retried once and a good Retry-After holds the retry back", "[HttpRequest_Tests]") { ScopedCurlInit ready; - // This tests mainly that the code doesn't fall over if - // various well- and mis-formed Retry-After headers are - // sent along with the response. Direct inspection of - // the parsing result isn't supported. + // Each of the peer's seven 503 answers, /503/0/ to /503/6/, carries + // a different Retry-After, well- or mis-formed, and the code must not + // fall over on any of them. The parsing result can't be inspected, + // but its effect can: only /503/0/'s "2" is a delay llcorehttp takes, + // and it holds that retry back two seconds where the others retry + // after the default backoff of one. // Handler can be stack-allocated *if* there are no dangling // references to it after completion of this method. // Create before memory record as the string copy will bump numbers. - TestHandler2 handler(this, "handler"); + RetryRecordingHandler handler(this, "handler"); LLCore::HttpHandler::ptr_t handlerp(&handler, NoOpDeletor); std::string url_base(get_base_url() + "/503/"); // path to 503 generators mHandlerCalls = 0; @@ -2972,13 +3020,16 @@ TEST_CASE_METHOD(HttpRequestTestData, "HttpRequest Tests: GETs answered 503 with opts->setRetries(1); // Retry once only opts->setUseRetryAfter(true); // Try to parse the retry-after header - // Issue a GET that 503s with valid retry-after + // Issue a GET to each of the peer's 503 answers mStatus = HttpStatus(503); - int url_limit(6); + const int url_limit(7); + std::vector urls; + const auto issued(std::chrono::steady_clock::now()); for (int i(0); i < url_limit; ++i) { std::ostringstream url; url << url_base << i << "/"; + urls.push_back(url.str()); HttpHandle handle = req->requestGetByteRange(HttpRequest::DEFAULT_POLICY_ID, url.str(), 0, @@ -3004,6 +3055,21 @@ TEST_CASE_METHOD(HttpRequestTestData, "HttpRequest Tests: GETs answered 503 with CHECK(count < limit); CHECK(mHandlerCalls == url_limit); + // Each GET was retried once, as a 503, whatever its Retry-After said, + // and the retry of the one with "Retry-After: 2" came no sooner than that + for (int i(0); i < url_limit; ++i) + { + CAPTURE(i); + const auto delivery(handler.mDeliveries.find(urls[i])); + REQUIRE(delivery != handler.mDeliveries.end()); + CHECK(delivery->second.mRetries == 1u); + CHECK(delivery->second.m503Retries == 1u); + if (0 == i) + { + CHECK(delivery->second.mAt - issued >= std::chrono::milliseconds(1900)); + } + } + // Okay, request a shutdown of the servicing thread mStatus = HttpStatus(); mHandlerCalls = 0; diff --git a/indra/llcorehttp/tests/httpstatus_test.cpp b/indra/llcorehttp/tests/httpstatus_test.cpp index 41add478d7..e2c0d20eb0 100644 --- a/indra/llcorehttp/tests/httpstatus_test.cpp +++ b/indra/llcorehttp/tests/httpstatus_test.cpp @@ -133,18 +133,18 @@ TEST_CASE_METHOD(HttpStatusTestData, "HttpStatus Tests: toString says something CHECK_FALSE(msg.empty()); } -TEST_CASE_METHOD(HttpStatusTestData, "HttpStatus Tests: statuses of different types compare unequal", "[HttpStatus_Tests]") +TEST_CASE_METHOD(HttpStatusTestData, "HttpStatus Tests: distinct statuses compare unequal when both succeed or both fail", "[HttpStatus_Tests]") { // Make certain equality/inequality tests do not pass // through the bool conversion. Distinct successful - // and error statuses should compare unequal. + // statuses, and distinct error statuses, should compare unequal. HttpStatus status1(HttpStatus::LLCORE, HE_SUCCESS); HttpStatus status2(HttpStatus::EXT_CURL_EASY, HE_SUCCESS); CHECK(status1 != status2); status1 = HttpStatus(HttpStatus::LLCORE, HE_REPLY_ERROR); - status1 = HttpStatus(HttpStatus::LLCORE, HE_SHUTTING_DOWN); + status2 = HttpStatus(HttpStatus::LLCORE, HE_SHUTTING_DOWN); CHECK(status1 != status2); } diff --git a/indra/llcorehttp/tests/refcounted_test.cpp b/indra/llcorehttp/tests/refcounted_test.cpp index d93a901576..5494f92ee2 100644 --- a/indra/llcorehttp/tests/refcounted_test.cpp +++ b/indra/llcorehttp/tests/refcounted_test.cpp @@ -33,6 +33,17 @@ using namespace LLCoreInt; namespace { + // A RefCounted the test may own: its destructor is public, so it can live on the stack + // after noRef() has taken it out of reference counting + struct OwnedRefCounted : public RefCounted + { + OwnedRefCounted() + : RefCounted(false) + {} + + ~OwnedRefCounted() override = default; + }; + TEST_CASE("RefCounted Tests: construction with an implicit count holds one reference", "[RefCounted_Tests]") { // create a new ref counted object with an implicit reference @@ -45,7 +56,7 @@ namespace TEST_CASE("RefCounted Tests: construction without an implicit count holds none", "[RefCounted_Tests]") { - // create a new ref counted object with an implicit reference + // create a new ref counted object without an implicit reference RefCounted * rc = new RefCounted(false); CHECK(rc->getRefCount() == 0); @@ -53,7 +64,7 @@ namespace rc->addRef(); CHECK(rc->getRefCount() == 1); - // release the implicit reference, causing the object to be released + // release the added reference, causing the object to be released rc->release(); } @@ -87,12 +98,13 @@ namespace TEST_CASE("RefCounted Tests: noRef marks the object not ref counted", "[RefCounted_Tests]") { - RefCounted * rc = new RefCounted(false); + // owned by the test, since an object out of reference counting cannot be released + OwnedRefCounted rc; // set the noRef - rc->noRef(); + rc.noRef(); - // with only one reference, isLastRef should be true - CHECK(rc->getRefCount() == RefCounted::NOT_REF_COUNTED); + // the count now says the object is not ref counted + CHECK(rc.getRefCount() == RefCounted::NOT_REF_COUNTED); } } diff --git a/indra/llcorehttp/tests/test_llcorehttp_peer.py b/indra/llcorehttp/tests/test_llcorehttp_peer.py index 7994c76037..7ecea58e65 100755 --- a/indra/llcorehttp/tests/test_llcorehttp_peer.py +++ b/indra/llcorehttp/tests/test_llcorehttp_peer.py @@ -63,7 +63,7 @@ class TestHTTPRequestHandler(BaseHTTPRequestHandler): -- '/bug2295/1/' "Content-Range: bytes 0-75/*" -- '/bug2295/2/' "Content-Range: bytes 0-75/2983", "Content-Length: 0" - -- '/bug2295/00000018/0/' Generates PARTIAL_FILE (18) error in libcurl. + -- '/bug2295/00000012/0/' Generates PARTIAL_FILE (18, 0x12) error in libcurl. "Content-Range: bytes 0-75/2983", "Content-Length: 76" -- '/bug2295/inv_cont_range/0/' Generates HE_INVALID_CONTENT_RANGE error in llcorehttp. @@ -183,7 +183,7 @@ def answer(self, data, withdata=True): self.send_header("Content-type", "text/plain") self.end_headers() if body: - self.wfile.write(body) + self.wfile.write(body.encode("utf-8")) elif "/bug2295/" in self.path: # Test for https://jira.secondlife.com/browse/BUG-2295 # diff --git a/indra/llmessage/tests/io_test.cpp b/indra/llmessage/tests/io_test.cpp index c726fe1ecd..b41653f832 100644 --- a/indra/llmessage/tests/io_test.cpp +++ b/indra/llmessage/tests/io_test.cpp @@ -74,6 +74,7 @@ namespace S32 count = mBuffer.countAfter(ch.in(), NULL); CHECK(count == str_len); LLBufferArray::segment_iterator_t it = mBuffer.beginSegment(); + REQUIRE(it != mBuffer.endSegment()); U8* first = (*it).data(); count = mBuffer.countAfter(ch.in(), first); CHECK(count == str_len - 1); @@ -89,6 +90,7 @@ namespace S32 count = mBuffer.countAfter(ch.in(), NULL); CHECK(count == 2 * str_len); LLBufferArray::segment_iterator_t it = mBuffer.beginSegment(); + REQUIRE(it != mBuffer.endSegment()); U8* first = (*it).data(); count = mBuffer.countAfter(ch.in(), first); CHECK(count == (2 * str_len) - 1); @@ -141,6 +143,7 @@ namespace char buffer[255]; /* Flawfinder: ignore */ S32 len = 5; LLBufferArray::segment_iterator_t it = mBuffer.beginSegment(); + REQUIRE(it != mBuffer.endSegment()); U8* addr = (*it).data(); mBuffer.readAfter(ch.in(), addr, (U8*)buffer, len); CHECK(len == 5); @@ -197,14 +200,14 @@ namespace std::string last_line; for(i = 0; i < LINE_COUNT; ++i) { + CAPTURE(i); len = BUFFER_LEN; last = mBuffer.readAfter(ch.in(), last, (U8*)buf, len); - char* newline = strchr((char*)buf, '\n'); + // buf holds len bytes and no terminator, so the line ends among those bytes or not at all. + const char* newline = static_cast(memchr(buf, '\n', len)); + REQUIRE(newline != NULL); S32 offset = -((len - 1) - (S32)(newline - buf)); - ++newline; - *newline = '\0'; - last_line.assign(buf); - CAPTURE(i); + last_line.assign(buf, newline - buf + 1); REQUIRE(last_line == lines[i]); last = mBuffer.seek(ch.in(), last, offset); } @@ -383,9 +386,11 @@ namespace mBuffer.splitAfter(last); LLBufferArray::segment_iterator_t it = mBuffer.beginSegment(); LLBufferArray::segment_iterator_t end = mBuffer.endSegment(); + REQUIRE(it != end); std::string first((char*)((*it).data()), (*it).size()); CHECK(first == std::string("zippity")); ++it; + REQUIRE(it != end); std::string second((char*)((*it).data()), (*it).size()); CHECK(second == std::string(" do da!")); ++it; @@ -450,8 +455,8 @@ namespace char buf[BUF_LEN]; /* Flawfinder: ignore */ S32 actual_len = BUF_LEN; (void) mBuffer.readAfter(ch.out(), NULL, (U8*)buf, actual_len); - buf[actual_len] = '\0'; - std::string actual(buf); + // The read may fill buf, which then has no room for a terminator. + std::string actual(buf, actual_len); std::string expected(ostr.str()); CHECK(actual.size() == expected.size()); CHECK(actual == expected); diff --git a/indra/llmessage/tests/llbuffer_test.cpp b/indra/llmessage/tests/llbuffer_test.cpp index b4de019d73..d60392b85f 100644 --- a/indra/llmessage/tests/llbuffer_test.cpp +++ b/indra/llmessage/tests/llbuffer_test.cpp @@ -86,7 +86,7 @@ namespace requestSize = 16384-1; CHECK(buf.createSegment(channel, requestSize, segment)); CHECK(segment.size() == requestSize); - // second request for remainign 1 byte + // second request for the remaining 1 byte requestSize = 1; CHECK(buf.createSegment(channel, requestSize, segment)); @@ -98,7 +98,7 @@ namespace LLHeapBuffer buf1(bigSize); - // requst for more than default size but less than total sizeit should fail now. + // a request for more than the default size but less than this buffer's size succeeds. requestSize = 16384 + 1; CHECK(buf1.createSegment(channel, requestSize, segment)); CHECK(segment.size() == requestSize); @@ -108,6 +108,7 @@ namespace CHECK(buf2.createSegment(channel, requestSize, segment)); REQUIRE(segment.size() == requestSize); CHECK(memcmp(segment.data(), (U8*) str, requestSize) == 0); + // it is full now, so it refuses any request, even one it would otherwise cut down to fit. requestSize = smallSize+1; CHECK_FALSE(buf2.createSegment(channel, requestSize, segment)); } @@ -190,6 +191,8 @@ namespace char buf[100]; S32 len2 = len; bufferArray1.readAfter(channelDescriptors.in(), NULL, (U8*)buf, len2); + // the terminator is the last byte read, so a short read leaves buf unterminated + REQUIRE(len2 == len); CHECK(buf == str); } @@ -225,7 +228,9 @@ namespace U8* last = bufferArray.readAfter(0, 0, (U8*)buf, len1); bufferArray.splitAfter(last); LLBufferArray::segment_iterator_t iterator = bufferArray.beginSegment(); + REQUIRE(iterator != bufferArray.endSegment()); ++iterator; + REQUIRE(iterator != bufferArray.endSegment()); std::string str(((char*)(*iterator).data()), (*iterator).size()); CHECK(str == std::string(" is a Virtual World")); } diff --git a/indra/llmessage/tests/llcoproceduremanager_test.cpp b/indra/llmessage/tests/llcoproceduremanager_test.cpp index 9325d0ef28..a7327acfaf 100644 --- a/indra/llmessage/tests/llcoproceduremanager_test.cpp +++ b/indra/llmessage/tests/llcoproceduremanager_test.cpp @@ -35,10 +35,6 @@ #include -#include -#include -#include - #include #include "../test/sync.h" @@ -101,73 +97,25 @@ namespace LLCoprocedureManager::instance().close("PoolName"); } - TEST_CASE_METHOD(coproceduremanager_test, "LLCoprocedureManager: a buffered channel hands a function to a fiber", "[LLCoprocedureManager]") - { - const size_t capacity = 2; - boost::fibers::buffered_channel> chan(capacity); - - boost::fibers::fiber worker([&chan]() { - chan.value_pop()(); - }); - - chan.push([]() { - LL_INFOS("Test") << "test 1" << LL_ENDL; - }); - - worker.join(); - } - - TEST_CASE_METHOD(coproceduremanager_test, "LLCoprocedureManager: an unbuffered channel hands a function to a fiber", "[LLCoprocedureManager]") - { - boost::fibers::unbuffered_channel> chan; - - boost::fibers::fiber worker([&chan]() { - chan.value_pop()(); - }); - - chan.push([]() { - LL_INFOS("Test") << "test 1" << LL_ENDL; - }); - - worker.join(); - } - - TEST_CASE_METHOD(coproceduremanager_test, "LLCoprocedureManager: a worker fiber runs every function until its channel closes", "[LLCoprocedureManager]") + TEST_CASE_METHOD(coproceduremanager_test, "LLCoprocedureManager: a pool runs every coprocedure enqueued on it", "[LLCoprocedureManager]") { - boost::fibers::buffered_channel> chan(4); - - boost::fibers::fiber worker([&chan]() { - std::function f; - - // using namespace std::chrono_literals; - // const auto timeout = 5s; - // boost::fibers::channel_op_status status; - while (chan.pop(f) != boost::fibers::channel_op_status::closed) - { - LL_INFOS("CoWorker") << "got coproc" << LL_ENDL; - f(); - } - LL_INFOS("CoWorker") << "got closed" << LL_ENDL; - }); - + // More than the pool has coroutines, so at least one of them goes back for another. + const int COPROC_COUNT = 12; + Sync sync; int counter = 0; - - for (int i = 0; i < 5; ++i) + LLCoprocedureManager::instance().initializePool("EveryCoproc"); + for (int i = 0; i < COPROC_COUNT; ++i) { - LL_INFOS("CoMain") << "pushing coproc " << i << LL_ENDL; - chan.push([&counter]() { - LL_INFOS("CoProc") << "in coproc" << LL_ENDL; - ++counter; - }); + LLCoprocedureManager::instance().enqueueCoprocedure("EveryCoproc", "Count", + [&counter, &sync] (LLCoreHttpUtil::HttpCoroutineAdapter::ptr_t & ptr, const LLUUID & id) { + ++counter; + sync.bump(); + }); } - LL_INFOS("CoMain") << "closing channel" << LL_ENDL; - chan.close(); - - LL_INFOS("CoMain") << "joining worker" << LL_ENDL; - worker.join(); + sync.yield_until(COPROC_COUNT); + CHECK(counter == COPROC_COUNT); - LL_INFOS("CoMain") << "checking count" << LL_ENDL; - CHECK(counter == 5); + LLCoprocedureManager::instance().close("EveryCoproc"); } } // anonymous namespace diff --git a/indra/llmessage/tests/lldatapacker_test.cpp b/indra/llmessage/tests/lldatapacker_test.cpp index cb8a508616..573ce8eef2 100644 --- a/indra/llmessage/tests/lldatapacker_test.cpp +++ b/indra/llmessage/tests/lldatapacker_test.cpp @@ -29,6 +29,7 @@ #include "linden_common.h" #include +#include #include #include "altestmatchers.h" @@ -65,7 +66,7 @@ namespace UNSCOPED_INFO("16 fraction bits"); CHECK_THAT(f_val2, ll_test::WithinBits(f_unpkval2, 16)); UNSCOPED_INFO("23 fraction bits"); - CHECK_THAT(f_val3, ll_test::WithinBits(f_unpkval3, 31)); + CHECK_THAT(f_val3, ll_test::WithinBits(f_unpkval3, 23)); } TEST_CASE("datapacker_test: LLDataPackerBinaryBuffer round-trips every packed type", "[datapacker_test]") @@ -75,6 +76,7 @@ namespace char str[] = "SecondLife is virtual World\0"; char strBinary[] = "SecondLife is virtual World"; char strBinaryFixed[] = "Fixed Data"; + S32 sizeBinary = sizeof(strBinary); S32 sizeBinaryFixed = sizeof(strBinaryFixed); U8 valU8 = 'C'; U16 valU16 = 0xFFFF; @@ -89,9 +91,11 @@ namespace LLUUID uuid; std::string unpkstr; - char unpkstrBinary[256]; - char unpkstrBinaryFixed[256]; - S32 unpksizeBinary; + // Zeroed, so a short or failed unpack reads as a short string + // rather than as whatever the stack held. + char unpkstrBinary[256] = {}; + char unpkstrBinaryFixed[256] = {}; + S32 unpksizeBinary = 0; U8 unpkvalU8; U16 unpkvalU16; U32 unpkvalU32; @@ -106,7 +110,7 @@ namespace LLDataPackerBinaryBuffer lldp(packbuf,1024); lldp.packString(str , "linden_lab_str"); - lldp.packBinaryData((U8*)strBinary, sizeof(strBinary), "linden_lab_bd"); + lldp.packBinaryData((U8*)strBinary, sizeBinary, "linden_lab_bd"); lldp.packBinaryDataFixed((U8*)strBinaryFixed, sizeBinaryFixed, "linden_lab_bdf"); lldp.packU8(valU8,"linden_lab_u8"); lldp.packU16(valU16,"linden_lab_u16"); @@ -140,6 +144,7 @@ namespace lldp1.unpackUUID(unpkuuid, "linden_lab_uuid"); CHECK(std::string_view(str) == std::string_view(unpkstr.c_str())); + CHECK(unpksizeBinary == sizeBinary); CHECK(std::string_view(strBinary) == std::string_view(unpkstrBinary)); CHECK(std::string_view(strBinaryFixed) == std::string_view(unpkstrBinaryFixed)); CHECK(valU8 == unpkvalU8); @@ -172,6 +177,7 @@ namespace TEST_CASE("datapacker_test: LLDataPackerBinaryBuffer freeBuffer leaves a zero buffer size", "[datapacker_test]") { + // freeBuffer deletes it, so it comes from new[] and is the packer's. U8* packbuf = new U8[128]; char str[] = "SecondLife is virtual World"; LLDataPackerBinaryBuffer lldp(packbuf, 128); @@ -183,13 +189,20 @@ namespace TEST_CASE("datapacker_test: LLDataPackerBinaryBuffer assignBuffer takes the new size and rewinds", "[datapacker_test]") { - U8* buf = new U8[]("SecondLife is virtual World"); - S32 size = sizeof(buf); - LLDataPackerBinaryBuffer lldp(buf, size); - U8* new_buf = new U8[]("Its Amazing"); - size = sizeof(new_buf)-1; - lldp.assignBuffer(new_buf, size); - CHECK(lldp.getBufferSize() == size); + const char first[] = "SecondLife is virtual World"; + const S32 first_size = sizeof(first); + const S32 second_size = sizeof("Its Amazing"); + + // assignBuffer deletes the buffer it replaces, so the first comes + // from new[] and is the packer's. Nothing deletes the second. + LLDataPackerBinaryBuffer lldp(new U8[first_size], first_size); + lldp.packString(first, "linden_lab"); + // Moved on, so that the rewind below has something to undo. + CHECK(lldp.getCurrentSize() == first_size); + + std::unique_ptr new_buf = std::make_unique(second_size); + lldp.assignBuffer(new_buf.get(), second_size); + CHECK(lldp.getBufferSize() == second_size); CHECK(lldp.getCurrentSize() == 0); } @@ -199,7 +212,9 @@ namespace char str[] = "SecondLife is virtual World"; LLDataPackerBinaryBuffer lldp(packbuf, 128); lldp.packString(str , "linden_lab"); - U8 new_buffer[128]; + // Zeroed, so that a copy that did not happen reads as an empty + // string rather than as whatever the stack held. + U8 new_buffer[128] = {}; std::string unpkbuf; LLDataPackerBinaryBuffer lldp1(new_buffer,128); lldp1 = lldp; @@ -229,6 +244,7 @@ namespace char str[] = "SecondLife is virtual World\0"; char strBinary[] = "SecondLife is virtual World"; char strBinaryFixed[] = "Fixed Data"; + S32 sizeBinary = sizeof(strBinary); S32 sizeBinaryFixed = sizeof(strBinaryFixed); U8 valU8 = 'C'; U16 valU16 = 0xFFFF; @@ -243,9 +259,11 @@ namespace LLUUID uuid; std::string unpkstr; - char unpkstrBinary[256]; - char unpkstrBinaryFixed[256]; - S32 unpksizeBinary; + // Zeroed, so a short or failed unpack reads as a short string + // rather than as whatever the stack held. + char unpkstrBinary[256] = {}; + char unpkstrBinaryFixed[256] = {}; + S32 unpksizeBinary = 0; U8 unpkvalU8; U16 unpkvalU16; U32 unpkvalU32; @@ -260,7 +278,7 @@ namespace LLDataPackerAsciiBuffer lldp(packbuf,1024); lldp.packString(str , "linden_lab_str"); - lldp.packBinaryData((U8*)strBinary, sizeof(strBinary), "linden_lab_bd"); + lldp.packBinaryData((U8*)strBinary, sizeBinary, "linden_lab_bd"); lldp.packBinaryDataFixed((U8*)strBinaryFixed, sizeBinaryFixed, "linden_lab_bdf"); lldp.packU8(valU8,"linden_lab_u8"); lldp.packU16(valU16,"linden_lab_u16"); @@ -294,6 +312,7 @@ namespace lldp1.unpackUUID(unpkuuid, "linden_lab_uuid"); CHECK(std::string_view(str) == std::string_view(unpkstr.c_str())); + CHECK(unpksizeBinary == sizeBinary); CHECK(std::string_view(strBinary) == std::string_view(unpkstrBinary)); CHECK(std::string_view(strBinaryFixed) == std::string_view(unpkstrBinaryFixed)); CHECK(valU8 == unpkvalU8); @@ -311,6 +330,7 @@ namespace TEST_CASE("datapacker_test: LLDataPackerAsciiBuffer freeBuffer leaves a zero buffer size", "[datapacker_test]") { + // freeBuffer deletes it, so it comes from new[] and is the packer's. char* packbuf = new char[128]; char str[] = "SecondLife is virtual World"; LLDataPackerAsciiBuffer lldp(packbuf, 128); @@ -324,10 +344,16 @@ namespace char buf[] = "SecondLife is virtual World"; S32 size = sizeof(buf); LLDataPackerAsciiBuffer lldp(buf, size); + lldp.packS32(42, "linden_lab"); + // Moved on, so that the rewind below has something to undo: "42\n", + // and the terminator an ascii buffer counts in its size. + CHECK(lldp.getCurrentSize() == 4); + char new_buf[] = "Its Amazing"; size = sizeof(new_buf); lldp.assignBuffer(new_buf, size); CHECK(lldp.getBufferSize() == size); + // Nothing but the terminator. CHECK(lldp.getCurrentSize() == 1); } @@ -365,6 +391,7 @@ namespace char str[] = "SecondLife is virtual World\0"; char strBinary[] = "SecondLife is virtual World"; char strBinaryFixed[] = "Fixed Data"; + S32 sizeBinary = sizeof(strBinary); S32 sizeBinaryFixed = sizeof(strBinaryFixed); U8 valU8 = 'C'; U16 valU16 = 0xFFFF; @@ -379,9 +406,11 @@ namespace LLUUID uuid; std::string unpkstr; - char unpkstrBinary[256]; - char unpkstrBinaryFixed[256]; - S32 unpksizeBinary; + // Zeroed, so a short or failed unpack reads as a short string + // rather than as whatever the stack held. + char unpkstrBinary[256] = {}; + char unpkstrBinaryFixed[256] = {}; + S32 unpksizeBinary = 0; U8 unpkvalU8; U16 unpkvalU16; U32 unpkvalU32; @@ -407,7 +436,7 @@ namespace LLDataPackerAsciiFile lldp(fp,2); lldp.packString(str , "linden_lab_str"); - lldp.packBinaryData((U8*)strBinary, sizeof(strBinary), "linden_lab_bd"); + lldp.packBinaryData((U8*)strBinary, sizeBinary, "linden_lab_bd"); lldp.packBinaryDataFixed((U8*)strBinaryFixed, sizeBinaryFixed, "linden_lab_bdf"); lldp.packU8(valU8,"linden_lab_u8"); lldp.packU16(valU16,"linden_lab_u16"); @@ -444,6 +473,7 @@ namespace fclose(fp); CHECK(std::string_view(str) == std::string_view(unpkstr.c_str())); + CHECK(unpksizeBinary == sizeBinary); CHECK(std::string_view(strBinary) == std::string_view(unpkstrBinary)); CHECK(std::string_view(strBinaryFixed) == std::string_view(unpkstrBinaryFixed)); CHECK(valU8 == unpkvalU8); @@ -480,6 +510,7 @@ namespace const char str[] = "SecondLife is virtual World"; const char strBinary[] = "SecondLife is virtual World"; const char strBinaryFixed[] = "Fixed Data"; + S32 sizeBinary = sizeof(strBinary); S32 sizeBinaryFixed = sizeof(strBinaryFixed); U8 valU8 = 'C'; U16 valU16 = 0xFFFF; @@ -494,9 +525,11 @@ namespace LLUUID uuid; std::string unpkstr; - char unpkstrBinary[256]; - char unpkstrBinaryFixed[256]; - S32 unpksizeBinary; + // Zeroed, so a short or failed unpack reads as a short string + // rather than as whatever the stack held. + char unpkstrBinary[256] = {}; + char unpkstrBinaryFixed[256] = {}; + S32 unpksizeBinary = 0; U8 unpkvalU8; U16 unpkvalU16; U32 unpkvalU32; @@ -513,7 +546,7 @@ namespace LLDataPackerAsciiFile lldp(ostr,2); lldp.packString(str , "linden_lab_str"); - lldp.packBinaryData((U8*)strBinary, sizeof(strBinary), "linden_lab_bd"); + lldp.packBinaryData((U8*)strBinary, sizeBinary, "linden_lab_bd"); lldp.packBinaryDataFixed((U8*)strBinaryFixed, sizeBinaryFixed, "linden_lab_bdf"); lldp.packU8(valU8,"linden_lab_u8"); lldp.packU16(valU16,"linden_lab_u16"); @@ -547,6 +580,7 @@ namespace lldp1.unpackUUID(unpkuuid, "linden_lab_uuid"); CHECK(std::string_view(str) == std::string_view(unpkstr.c_str())); + CHECK(unpksizeBinary == sizeBinary); CHECK(std::string_view(strBinary) == std::string_view(unpkstrBinary)); CHECK(std::string_view(strBinaryFixed) == std::string_view(unpkstrBinaryFixed)); CHECK(valU8 == unpkvalU8); diff --git a/indra/llmessage/tests/llhttpnode_test.cpp b/indra/llmessage/tests/llhttpnode_test.cpp index b38192eb22..f9bf4db5d0 100644 --- a/indra/llmessage/tests/llhttpnode_test.cpp +++ b/indra/llmessage/tests/llhttpnode_test.cpp @@ -32,6 +32,8 @@ #include "llhttpnode.h" #include "llsdhttpserver.h" +#include + namespace { struct HTTPNodeTestData @@ -215,13 +217,20 @@ namespace LLHTTPNode* alphaNode = new LLHTTPNode; LLHTTPNode* betaNode = new LLHTTPNode; LLHTTPNode* gammaNode = new LLHTTPNode; - LLHTTPNode* gamma2Node = new LLHTTPNode; + // addNode refuses a node for a path that has one, and the refused node stays its caller's. + auto gamma2Node = std::make_unique(); mRoot.addNode("greek/alpha", alphaNode); mRoot.addNode("greek/beta", betaNode); mRoot.addNode("greek/gamma", gammaNode); - mRoot.addNode("greek/gamma", gamma2Node); + mRoot.addNode("greek/gamma", gamma2Node.get()); + const bool refused = gamma2Node->rootNode() == gamma2Node.get(); + CHECK(refused); + if (!refused) + { + (void)gamma2Node.release(); // mRoot took it, and deletes it + } LLHTTPNode* alephNode = new LLHTTPNode; diff --git a/indra/llmessage/tests/llnamevalue_test.cpp b/indra/llmessage/tests/llnamevalue_test.cpp index d031585ee9..c03ec428d7 100644 --- a/indra/llmessage/tests/llnamevalue_test.cpp +++ b/indra/llmessage/tests/llnamevalue_test.cpp @@ -58,9 +58,6 @@ namespace CHECK(nValue.getTypeEnum() == NVT_NULL); CHECK(nValue.getClassEnum() == NVC_NULL); CHECK(nValue.getSendtoEnum() == NVS_NULL); - - LLNameValue nValue1(" SecondLife ASSET RW SIM 232324343"); - } // LLNameValue(const char* data); @@ -86,6 +83,13 @@ namespace CHECK_FALSE(nValue1.sendToData()); CHECK_FALSE(nValue1.sendToViewer()); + LLNameValue nValueSim(" SecondLife ASSET RW SIM 232324343"); + CHECK(std::string_view(nValueSim.mName) == "SecondLife"); + CHECK(nValueSim.getTypeEnum() == NVT_ASSET); + CHECK(nValueSim.getClassEnum() == NVC_READ_WRITE); + CHECK(nValueSim.getSendtoEnum() == NVS_SIM); + CHECK(std::string_view(nValueSim.getAsset()) == "232324343"); + LLNameValue nValue2("SecondLife", "23.5", "F32", "R", "DS"); CHECK(nValue2.getTypeEnum() == NVT_F32); CHECK(nValue2.getClassEnum() == NVC_READ_ONLY); @@ -394,7 +398,7 @@ namespace LLNameValue nValue4("SecondLife", "<1,2,3>", "VEC3", "R", "S"); // this should not have updated as it is read only. LLVector3 vec(4,5,6); - nValue3.setVec3(vec); + nValue4.setVec3(vec); LLVector3 vec1(1,2,3); CHECK(*nValue4.getVec3() == vec1); diff --git a/indra/llmessage/tests/llsdmessagebuilder_test.cpp b/indra/llmessage/tests/llsdmessagebuilder_test.cpp index 9f8aeb0e30..a820da2d09 100644 --- a/indra/llmessage/tests/llsdmessagebuilder_test.cpp +++ b/indra/llmessage/tests/llsdmessagebuilder_test.cpp @@ -47,15 +47,15 @@ namespace { static LLTemplateMessageBuilder::message_template_name_map_t templateNameMap; - LLMsgData* messageData = NULL; - LLMsgBlkData* messageBlockData = NULL; - struct LLSDMessageBuilderTestData { + // A legacy message holding one block, testBlock, for the + // copyFromMessageData cases to fill. The message owns the block. + LLMsgData messageData{"testMessage"}; + LLMsgBlkData* messageBlockData = new LLMsgBlkData("testBlock", 0); LLSDMessageBuilderTestData() { - messageData = new LLMsgData("testMessage"); - messageBlockData = new LLMsgBlkData("testBlock", 0); + messageData.addBlock(messageBlockData); } static LLSDMessageBuilder defaultBuilder() @@ -96,10 +96,10 @@ namespace return result; } - static LLTemplateMessageBuilder* defaultTemplateBuilder(LLMessageTemplate& messageTemplate, char* name = const_cast(_PREHASH_Test0)) + static std::unique_ptr defaultTemplateBuilder(LLMessageTemplate& messageTemplate, char* name = const_cast(_PREHASH_Test0)) { templateNameMap[_PREHASH_TestMessage] = &messageTemplate; - LLTemplateMessageBuilder* builder = new LLTemplateMessageBuilder(templateNameMap); + auto builder = std::make_unique(templateNameMap); builder->newMessage(_PREHASH_TestMessage); builder->nextBlock(name); return builder; @@ -113,10 +113,10 @@ namespace TEST_CASE_METHOD(LLSDMessageBuilderTestData, "LLSDMessageBuilder: a builder with a message and a block makes a reader", "[LLSDMessageBuilder]") - // construction and test of undefined { LLSDMessageBuilder builder = defaultBuilder(); LLSDMessageReader reader = setReader(builder); + CHECK(reader.getNumberOfBlocks("block") == 1); } TEST_CASE_METHOD(LLSDMessageBuilderTestData, "LLSDMessageBuilder: addBOOL round trips a bool through the reader", "[LLSDMessageBuilder]") @@ -294,12 +294,9 @@ namespace TEST_CASE_METHOD(LLSDMessageBuilderTestData, "LLSDMessageBuilder: copyFromMessageData creates the blocks of a legacy message", "[LLSDMessageBuilder]") { - LLMsgBlkData* mbd = new LLMsgBlkData("testBlock", 0); - LLMsgData* md = new LLMsgData("testMessage"); - md->addBlock(mbd); LLSDMessageBuilder builder = defaultBuilder(); - builder.copyFromMessageData(*md); + builder.copyFromMessageData(messageData); LLSD output = builder.getMessage(); CHECK(output["testBlock"].isDefined()); @@ -310,68 +307,57 @@ namespace char binData[] = "abcdefghijklmnop"; addValue(messageBlockData, (char *)"testBinData", &binData, MVT_FIXED, sizeof(binData)); - messageData->addBlock(messageBlockData); LLSDMessageBuilder builder = defaultBuilder(); - builder.copyFromMessageData(*messageData); + builder.copyFromMessageData(messageData); LLSD output = builder.getMessage(); std::vector v = output["testBlock"][0]["testBinData"].asBinary(); - REQUIRE(v.size() > 0); - - CHECK_THAT(ll_test::Memory(&v[0], sizeof(binData)), ll_test::MemoryMatches(binData, sizeof(binData))); + CHECK_THAT(ll_test::Memory(v.data(), v.size()), ll_test::MemoryMatches(binData, sizeof(binData))); } - TEST_CASE_METHOD(LLSDMessageBuilderTestData, "LLSDMessageBuilder: copyFromMessageData of MVT_VARIABLE data_size 1 is commented out", "[LLSDMessageBuilder]") + TEST_CASE_METHOD(LLSDMessageBuilderTestData, "LLSDMessageBuilder: copyFromMessageData copies MVT_VARIABLE data_size 1 with no closing NUL as binary", "[LLSDMessageBuilder]") { - /* U8 binData[] = "abcdefghijklmnop"; + // a data_size 1 field becomes a string only when it ends in a NUL; + // other bytes stay binary, a NUL inside them included + U8 binData[] = {'a', 'b', 0, 'c', 'd'}; - addValue(messageBlockData, "testBinData", &binData, MVT_VARIABLE, sizeof(binData), 1); - messageData->addBlock(messageBlockData); + addValue(messageBlockData, (char *)"testBinData", &binData, MVT_VARIABLE, sizeof(binData), 1); LLSDMessageBuilder builder = defaultBuilder(); - builder.copyFromMessageData(*messageData); + builder.copyFromMessageData(messageData); LLSD output = builder.getMessage(); std::vector v = output["testBlock"][0]["testBinData"].asBinary(); - ensure("Ensure MVT_S16Array data copied from legacy to llsd give a valid vector", v.size() > 0); - - ensure_memory_matches("Ensure MVT_VARIABLE U8 binary data works in a message copied from legacy to llsd", - &v[0], sizeof(binData), binData, sizeof(binData));*/ + CHECK_THAT(ll_test::Memory(v.data(), v.size()), ll_test::MemoryMatches(binData, sizeof(binData))); } TEST_CASE_METHOD(LLSDMessageBuilderTestData, "LLSDMessageBuilder: copyFromMessageData copies MVT_VARIABLE data_size 2 U16s as binary", "[LLSDMessageBuilder]") { U16 binData[] = {1,2,3,4,5,6,7,8,9}; //9 shorts - addValue(messageBlockData, (char *)"testBinData", &binData, MVT_VARIABLE, sizeof(binData) >> 1, 2); - messageData->addBlock(messageBlockData); + addValue(messageBlockData, (char *)"testBinData", &binData, MVT_VARIABLE, sizeof(binData), 2); LLSDMessageBuilder builder = defaultBuilder(); - builder.copyFromMessageData(*messageData); + builder.copyFromMessageData(messageData); LLSD output = builder.getMessage(); std::vector v = output["testBlock"][0]["testBinData"].asBinary(); - REQUIRE(v.size() > 0); - - CHECK_THAT(ll_test::Memory(&v[0], sizeof(binData) >> 1), ll_test::MemoryMatches(binData, sizeof(binData) >> 1)); + CHECK_THAT(ll_test::Memory(v.data(), v.size()), ll_test::MemoryMatches(binData, sizeof(binData))); } TEST_CASE_METHOD(LLSDMessageBuilderTestData, "LLSDMessageBuilder: copyFromMessageData copies MVT_VARIABLE data_size 4 U32s as binary", "[LLSDMessageBuilder]") { U32 binData[] = {9,8,7,6,5,4,3,2,1}; - addValue(messageBlockData, (char *)"testBinData", &binData, MVT_VARIABLE, sizeof(binData) >> 2, 4); - messageData->addBlock(messageBlockData); + addValue(messageBlockData, (char *)"testBinData", &binData, MVT_VARIABLE, sizeof(binData), 4); LLSDMessageBuilder builder = defaultBuilder(); - builder.copyFromMessageData(*messageData); + builder.copyFromMessageData(messageData); LLSD output = builder.getMessage(); std::vector v = output["testBlock"][0]["testBinData"].asBinary(); - REQUIRE(v.size() > 0); - - CHECK_THAT(ll_test::Memory(&v[0], sizeof(binData) >> 2), ll_test::MemoryMatches(binData, sizeof(binData) >> 2)); + CHECK_THAT(ll_test::Memory(v.data(), v.size()), ll_test::MemoryMatches(binData, sizeof(binData))); } TEST_CASE_METHOD(LLSDMessageBuilderTestData, "LLSDMessageBuilder: copyFromMessageData copies MVT_U8 as an integer", "[LLSDMessageBuilder]") @@ -379,10 +365,9 @@ namespace U8 data = 0xa5; addValue(messageBlockData, (char *)"testBinData", &data, MVT_U8, sizeof(data)); - messageData->addBlock(messageBlockData); LLSDMessageBuilder builder = defaultBuilder(); - builder.copyFromMessageData(*messageData); + builder.copyFromMessageData(messageData); LLSD output = builder.getMessage(); CHECK(output["testBlock"][0]["testBinData"].asInteger() == data); @@ -393,10 +378,9 @@ namespace U16 data = 0xa55a; addValue(messageBlockData, (char *)"testBinData", &data, MVT_U16, sizeof(data)); - messageData->addBlock(messageBlockData); LLSDMessageBuilder builder = defaultBuilder(); - builder.copyFromMessageData(*messageData); + builder.copyFromMessageData(messageData); LLSD output = builder.getMessage(); CHECK(output["testBlock"][0]["testBinData"].asInteger() == data); @@ -407,24 +391,22 @@ namespace U32 data = 0xa55a7117; addValue(messageBlockData, (char *)"testBinData", &data, MVT_U32, sizeof(data)); - messageData->addBlock(messageBlockData); LLSDMessageBuilder builder = defaultBuilder(); - builder.copyFromMessageData(*messageData); + builder.copyFromMessageData(messageData); LLSD output = builder.getMessage(); CHECK(ll_U32_from_sd(output["testBlock"][0]["testBinData"]) == data); } - // MVT_U64 - crush into an s32: LLSD does not support 64 bit values + // MVT_U64 - LLSD has no 64 bit integer, so ll_sd_from_U64 carries it as 8 bytes of binary TEST_CASE_METHOD(LLSDMessageBuilderTestData, "LLSDMessageBuilder: copyFromMessageData keeps all 64 bits of MVT_U64 for ll_U64_from_sd", "[LLSDMessageBuilder]") { U64 data = U64L(0xa55a711711223344); addValue(messageBlockData, (char *)"testBinData", &data, MVT_U64, sizeof(data)); - messageData->addBlock(messageBlockData); LLSDMessageBuilder builder = defaultBuilder(); - builder.copyFromMessageData(*messageData); + builder.copyFromMessageData(messageData); LLSD output = builder.getMessage(); CHECK(ll_U64_from_sd(output["testBlock"][0]["testBinData"]) == data); @@ -435,10 +417,9 @@ namespace S8 data = -31; addValue(messageBlockData, (char *)"testBinData", &data, MVT_S8, sizeof(data)); - messageData->addBlock(messageBlockData); LLSDMessageBuilder builder = defaultBuilder(); - builder.copyFromMessageData(*messageData); + builder.copyFromMessageData(messageData); LLSD output = builder.getMessage(); CHECK(output["testBlock"][0]["testBinData"].asInteger() == data); @@ -449,10 +430,9 @@ namespace S16 data = -31; addValue(messageBlockData, (char *)"testBinData", &data, MVT_S16, sizeof(data)); - messageData->addBlock(messageBlockData); LLSDMessageBuilder builder = defaultBuilder(); - builder.copyFromMessageData(*messageData); + builder.copyFromMessageData(messageData); LLSD output = builder.getMessage(); CHECK(output["testBlock"][0]["testBinData"].asInteger() == data); @@ -463,10 +443,9 @@ namespace S32 data = -3100; addValue(messageBlockData, (char *)"testBinData", &data, MVT_S32, sizeof(data)); - messageData->addBlock(messageBlockData); LLSDMessageBuilder builder = defaultBuilder(); - builder.copyFromMessageData(*messageData); + builder.copyFromMessageData(messageData); LLSD output = builder.getMessage(); CHECK(output["testBlock"][0]["testBinData"].asInteger() == data); @@ -478,10 +457,9 @@ namespace S64 data = -31003100; addValue(messageBlockData, (char *)"testBinData", &data, MVT_S64, sizeof(data)); - messageData->addBlock(messageBlockData); LLSDMessageBuilder builder = defaultBuilder(); - builder.copyFromMessageData(*messageData); + builder.copyFromMessageData(messageData); LLSD output = builder.getMessage(); CHECK(output["testBlock"][0]["testBinData"].asInteger() == (S32)data); @@ -492,10 +470,9 @@ namespace F32 data = 1234.1234f; addValue(messageBlockData, (char *)"testBinData", &data, MVT_F32, sizeof(data)); - messageData->addBlock(messageBlockData); LLSDMessageBuilder builder = defaultBuilder(); - builder.copyFromMessageData(*messageData); + builder.copyFromMessageData(messageData); LLSD output = builder.getMessage(); CHECK(output["testBlock"][0]["testBinData"].asReal() == data); @@ -506,10 +483,9 @@ namespace F64 data = 1234.1234; addValue(messageBlockData, (char *)"testBinData", &data, MVT_F64, sizeof(data)); - messageData->addBlock(messageBlockData); LLSDMessageBuilder builder = defaultBuilder(); - builder.copyFromMessageData(*messageData); + builder.copyFromMessageData(messageData); LLSD output = builder.getMessage(); CHECK(output["testBlock"][0]["testBinData"].asReal() == data); @@ -520,10 +496,9 @@ namespace LLVector3 data(1,2,3); addValue(messageBlockData, (char *)"testBinData", &data, MVT_LLVector3, sizeof(data)); - messageData->addBlock(messageBlockData); LLSDMessageBuilder builder = defaultBuilder(); - builder.copyFromMessageData(*messageData); + builder.copyFromMessageData(messageData); LLSD output = builder.getMessage(); CHECK(ll_vector3_from_sd(output["testBlock"][0]["testBinData"]) == data); @@ -534,10 +509,9 @@ namespace LLVector3d data(1,2,3); addValue(messageBlockData, (char *)"testBinData", &data, MVT_LLVector3d, sizeof(data)); - messageData->addBlock(messageBlockData); LLSDMessageBuilder builder = defaultBuilder(); - builder.copyFromMessageData(*messageData); + builder.copyFromMessageData(messageData); LLSD output = builder.getMessage(); CHECK(ll_vector3d_from_sd(output["testBlock"][0]["testBinData"]) == data); @@ -549,10 +523,9 @@ namespace LLSD v = ll_sd_from_vector4(data); addValue(messageBlockData, (char *)"testBinData", &data, MVT_LLVector4, sizeof(data)); - messageData->addBlock(messageBlockData); LLSDMessageBuilder builder = defaultBuilder(); - builder.copyFromMessageData(*messageData); + builder.copyFromMessageData(messageData); LLSD output = builder.getMessage(); CHECK_THAT(output["testBlock"][0]["testBinData"], ll_test::LLSDEquals(v)); @@ -560,19 +533,21 @@ namespace TEST_CASE_METHOD(LLSDMessageBuilderTestData, "LLSDMessageBuilder: copyFromMessageData unpacks MVT_LLQuaternion from a packed vector", "[LLSDMessageBuilder]") { - LLQuaternion data(0.3713907f, 0.5570861f, 0.7427813f,0.0f); + // one radian about a unit axis: w > 0, so the packing keeps it + LLQuaternion data(1.f, LLVector3(0.3713907f, 0.5570861f, 0.7427813f)); //we send a quaternion packed into a vec3 (w is infered) - so sizeof(vec) == 12 bytes not 16. LLVector3 vec = data.packToVector3(); addValue(messageBlockData, (char *)"testBinData", &vec, MVT_LLQuaternion, sizeof(vec)); - messageData->addBlock(messageBlockData); LLSDMessageBuilder builder = defaultBuilder(); - builder.copyFromMessageData(*messageData); + builder.copyFromMessageData(messageData); LLSD output = builder.getMessage(); - CHECK(ll_quaternion_from_sd(output["testBlock"][0]["testBinData"]) == data); + LLQuaternion unpacked = ll_quaternion_from_sd(output["testBlock"][0]["testBinData"]); + CAPTURE(data, unpacked); + CHECK(unpacked.isEqualEps(data, 1e-5f)); } TEST_CASE_METHOD(LLSDMessageBuilderTestData, "LLSDMessageBuilder: copyFromMessageData copies MVT_LLUUID as a UUID", "[LLSDMessageBuilder]") @@ -580,14 +555,11 @@ namespace LLUUID data("01234567-0123-0123-0123-234567abcdef"); addValue(messageBlockData, (char *)"testBinData", &data, MVT_LLUUID, sizeof(data)); - messageData->addBlock(messageBlockData); LLSDMessageBuilder builder = defaultBuilder(); - builder.copyFromMessageData(*messageData); + builder.copyFromMessageData(messageData); LLSD output = builder.getMessage(); - std::string v = output["testBlock"][0]["testBinData"].asUUID().asString(); - CHECK(output["testBlock"][0]["testBinData"].asUUID() == data); } @@ -596,14 +568,11 @@ namespace bool valueTrue = true; bool valueFalse = false; - LLMsgData* md = new LLMsgData("testMessage"); - LLMsgBlkData* mbd = new LLMsgBlkData("testBlock", 0); - addValue(mbd, (char *)"testBoolFalse", &valueFalse, MVT_BOOL, sizeof(bool)); - addValue(mbd, (char *)"testBoolTrue", &valueTrue, MVT_BOOL, sizeof(bool)); - md->addBlock(mbd); + addValue(messageBlockData, (char *)"testBoolFalse", &valueFalse, MVT_BOOL, sizeof(bool)); + addValue(messageBlockData, (char *)"testBoolTrue", &valueTrue, MVT_BOOL, sizeof(bool)); LLSDMessageBuilder builder = defaultBuilder(); - builder.copyFromMessageData(*md); + builder.copyFromMessageData(messageData); LLSD output = builder.getMessage(); CHECK(output["testBlock"][0]["testBoolTrue"].asBoolean()); @@ -616,10 +585,9 @@ namespace LLSD v = ll_sd_from_ipaddr(data); addValue(messageBlockData, (char *)"testBinData", &data, MVT_IP_ADDR, sizeof(data)); - messageData->addBlock(messageBlockData); LLSDMessageBuilder builder = defaultBuilder(); - builder.copyFromMessageData(*messageData); + builder.copyFromMessageData(messageData); LLSD output = builder.getMessage(); CHECK_THAT(output["testBlock"][0]["testBinData"], ll_test::LLSDEquals(v)); @@ -630,10 +598,9 @@ namespace U16 data = 0xff88; addValue(messageBlockData, (char *)"testBinData", &data, MVT_IP_PORT, sizeof(data)); - messageData->addBlock(messageBlockData); LLSDMessageBuilder builder = defaultBuilder(); - builder.copyFromMessageData(*messageData); + builder.copyFromMessageData(messageData); LLSD output = builder.getMessage(); CHECK(output["testBlock"][0]["testBinData"].asInteger() == data); @@ -644,16 +611,13 @@ namespace U16 data[3] = {0,1,2}; addValue(messageBlockData, (char *)"testBinData", &data, MVT_U16Vec3, sizeof(data)); - messageData->addBlock(messageBlockData); LLSDMessageBuilder builder = defaultBuilder(); - builder.copyFromMessageData(*messageData); + builder.copyFromMessageData(messageData); LLSD output = builder.getMessage(); std::vector v = output["testBlock"][0]["testBinData"].asBinary(); - REQUIRE(v.size() > 0); - - CHECK_THAT(ll_test::Memory((U16*)&v[0], 6), ll_test::MemoryMatches(data, 6)); + CHECK_THAT(ll_test::Memory(v.data(), v.size()), ll_test::MemoryMatches(data, 6)); } TEST_CASE_METHOD(LLSDMessageBuilderTestData, "LLSDMessageBuilder: copyFromMessageData copies MVT_U16Quat as eight bytes", "[LLSDMessageBuilder]") @@ -661,16 +625,13 @@ namespace U16 data[4] = {0,1,2,4}; addValue(messageBlockData, (char *)"testBinData", &data, MVT_U16Quat, sizeof(data)); - messageData->addBlock(messageBlockData); LLSDMessageBuilder builder = defaultBuilder(); - builder.copyFromMessageData(*messageData); + builder.copyFromMessageData(messageData); LLSD output = builder.getMessage(); std::vector v = output["testBlock"][0]["testBinData"].asBinary(); - REQUIRE(v.size() > 0); - - CHECK_THAT(ll_test::Memory((U16*)&v[0], 8), ll_test::MemoryMatches(data, 8)); + CHECK_THAT(ll_test::Memory(v.data(), v.size()), ll_test::MemoryMatches(data, 8)); } TEST_CASE_METHOD(LLSDMessageBuilderTestData, "LLSDMessageBuilder: copyFromMessageData copies MVT_S16Array as binary", "[LLSDMessageBuilder]") @@ -678,16 +639,13 @@ namespace S16 data[19] = {0,-1,2,-4,5,-6,7,-8,9,-10,11,-12,13,-14,15,16,17,18}; addValue(messageBlockData, (char *)"testBinData", &data, MVT_S16Array, sizeof(data)); - messageData->addBlock(messageBlockData); LLSDMessageBuilder builder = defaultBuilder(); - builder.copyFromMessageData(*messageData); + builder.copyFromMessageData(messageData); LLSD output = builder.getMessage(); std::vector v = output["testBlock"][0]["testBinData"].asBinary(); - REQUIRE(v.size() > 0); - - CHECK_THAT(ll_test::Memory((U16*)&v[0], 19), ll_test::MemoryMatches(data, 19)); + CHECK_THAT(ll_test::Memory(v.data(), v.size()), ll_test::MemoryMatches(data, sizeof(data))); } TEST_CASE_METHOD(LLSDMessageBuilderTestData, "LLSDMessageBuilder: copyFromMessageData copies a U8 from an LLTemplateMessageBuilder", "[LLSDMessageBuilder]") @@ -695,7 +653,7 @@ namespace LLMessageTemplate messageTemplate = defaultTemplate(); messageTemplate.addBlock(defaultTemplateBlock(MVT_U8, 1)); U8 inValue = 2; - LLTemplateMessageBuilder* template_builder = defaultTemplateBuilder(messageTemplate); + std::unique_ptr template_builder = defaultTemplateBuilder(messageTemplate); template_builder->addU8(_PREHASH_Test0, inValue); LLSDMessageBuilder builder; @@ -711,7 +669,7 @@ namespace LLMessageTemplate messageTemplate = defaultTemplate(); messageTemplate.addBlock(defaultTemplateBlock(MVT_VARIABLE, 1)); std::string inValue = "testing"; - LLTemplateMessageBuilder* builder = defaultTemplateBuilder(messageTemplate); + std::unique_ptr builder = defaultTemplateBuilder(messageTemplate); builder->addString(_PREHASH_Test0, inValue.c_str()); LLSDMessageBuilder sd_builder; @@ -727,10 +685,9 @@ namespace // and must yield an empty value instead of reading out of bounds { addValue(messageBlockData, (char*)"testBinData", nullptr, MVT_VARIABLE, 0, 1); - messageData->addBlock(messageBlockData); LLSDMessageBuilder builder = defaultBuilder(); - builder.copyFromMessageData(*messageData); + builder.copyFromMessageData(messageData); LLSD output = builder.getMessage(); CHECK(output["testBlock"][0]["testBinData"].isDefined()); @@ -748,7 +705,7 @@ namespace block->addVariable(const_cast(_PREHASH_Test1), MVT_U8, 1); messageTemplate.addBlock(block); - LLTemplateMessageBuilder* builder = defaultTemplateBuilder(messageTemplate); + std::unique_ptr builder = defaultTemplateBuilder(messageTemplate); builder->addU8(_PREHASH_Test0, 7); // Test1 left as a placeholder LLSDMessageBuilder sd_builder; diff --git a/indra/llmessage/tests/lltemplatemessagebuilder_test.cpp b/indra/llmessage/tests/lltemplatemessagebuilder_test.cpp index 93aa839121..8e76c43a31 100644 --- a/indra/llmessage/tests/lltemplatemessagebuilder_test.cpp +++ b/indra/llmessage/tests/lltemplatemessagebuilder_test.cpp @@ -108,6 +108,7 @@ namespace messageTemplate.addBlock(defaultBlock()); LLTemplateMessageBuilder* builder = defaultBuilder(messageTemplate); LLTemplateMessageReader* reader = setReader(messageTemplate, builder); + CHECK(reader->getNumberOfBlocks(_PREHASH_Test0) == 1); delete reader; } @@ -271,13 +272,16 @@ namespace { LLMessageTemplate messageTemplate = defaultTemplate(); messageTemplate.addBlock(defaultBlock(MVT_LLQuaternion, 12)); - LLQuaternion outValue, inValue = LLQuaternion(0.0f, LLVector3(0.3713907f, 0.5570861f, 0.7427813f)); + // one radian about a unit axis: w > 0, so the packing keeps it + LLQuaternion outValue, inValue = LLQuaternion(1.f, LLVector3(0.3713907f, 0.5570861f, 0.7427813f)); LLTemplateMessageBuilder* builder = defaultBuilder(messageTemplate); builder->addQuat(_PREHASH_Test0, inValue); LLTemplateMessageReader* reader = setReader(messageTemplate, builder); reader->getQuat(_PREHASH_Test0, _PREHASH_Test0, outValue); - CHECK(inValue == outValue); + // the wire carries x, y and z, and the reader recovers w from them + CAPTURE(inValue, outValue); + CHECK(outValue.isEqualEps(inValue, 1e-5f)); delete reader; } @@ -359,12 +363,12 @@ namespace delete builder; // build template: Test1 before Test0 - messageTemplate = defaultTemplate(); - messageTemplate.addBlock(createBlock(const_cast(_PREHASH_Test1), MVT_U32, 4, MBT_SINGLE)); - messageTemplate.addBlock(createBlock(const_cast(_PREHASH_Test0), MVT_U32, 4, MBT_SINGLE)); + LLMessageTemplate reorderedTemplate = defaultTemplate(); + reorderedTemplate.addBlock(createBlock(const_cast(_PREHASH_Test1), MVT_U32, 4, MBT_SINGLE)); + reorderedTemplate.addBlock(createBlock(const_cast(_PREHASH_Test0), MVT_U32, 4, MBT_SINGLE)); // build message: 1st declared block var == 0xaaaa, 2nd declared block var == 0xbbbb - builder = defaultBuilder(messageTemplate, const_cast(_PREHASH_Test1)); + builder = defaultBuilder(reorderedTemplate, const_cast(_PREHASH_Test1)); builder->addU32(_PREHASH_Test0, 0xaaaa); builder->nextBlock(_PREHASH_Test0); builder->addU32(_PREHASH_Test0, 0xbbbb); @@ -429,12 +433,12 @@ namespace delete builder; // Build template: Test0 before Test1 - messageTemplate = defaultTemplate(); - messageTemplate.addBlock(createBlock(const_cast(_PREHASH_Test0), MVT_U32, 4, MBT_SINGLE)); - messageTemplate.addBlock(createBlock(const_cast(_PREHASH_Test1), MVT_U32, 4, MBT_SINGLE)); + LLMessageTemplate longerTemplate = defaultTemplate(); + longerTemplate.addBlock(createBlock(const_cast(_PREHASH_Test0), MVT_U32, 4, MBT_SINGLE)); + longerTemplate.addBlock(createBlock(const_cast(_PREHASH_Test1), MVT_U32, 4, MBT_SINGLE)); // Build message - builder = defaultBuilder(messageTemplate, const_cast(_PREHASH_Test0)); + builder = defaultBuilder(longerTemplate, const_cast(_PREHASH_Test0)); builder->addU32(_PREHASH_Test0, 0xaaaa); builder->nextBlock(_PREHASH_Test1); builder->addU32(_PREHASH_Test0, 0xbbbb); @@ -509,10 +513,13 @@ namespace builder = defaultBuilder(messageTemplate); builder->newMessage(_PREHASH_TestMessage); reader->copyToBuilder(*builder); - U8 buffer[MAX_BUFFER_SIZE]; - builder->buildMessage(buffer, MAX_BUFFER_SIZE, 0); + delete reader; - delete builder; + // read the forwarded message + reader = setReader(messageTemplate, builder); + U32 outValue = 0; + reader->getU32(_PREHASH_Test0, _PREHASH_Test0, outValue); + CHECK(outValue == 42); delete reader; } @@ -523,6 +530,7 @@ namespace LLTemplateMessageBuilder* builder = defaultBuilder(messageTemplate); LLTemplateMessageReader* reader = setReader( messageTemplate, builder, 10); + CHECK(reader->getNumberOfBlocks(_PREHASH_Test0) == 1); delete reader; } @@ -698,13 +706,16 @@ namespace { LLMessageTemplate messageTemplate = defaultTemplate(); messageTemplate.addBlock(defaultBlock(MVT_LLQuaternion, 12)); - LLQuaternion outValue, inValue = LLQuaternion(0.0f, LLVector3(0.3713907f, 0.5570861f, 0.7427813f)); + // one radian about a unit axis: w > 0, so the packing keeps it + LLQuaternion outValue, inValue = LLQuaternion(1.f, LLVector3(0.3713907f, 0.5570861f, 0.7427813f)); LLTemplateMessageBuilder* builder = defaultBuilder(messageTemplate); builder->addQuat(_PREHASH_Test0, inValue); LLTemplateMessageReader* reader = setReader( messageTemplate, builder, 12); reader->getQuat(_PREHASH_Test0, _PREHASH_Test0, outValue); - CHECK(inValue == outValue); + // the wire carries x, y and z, and the reader recovers w from them + CAPTURE(inValue, outValue); + CHECK(outValue.isEqualEps(inValue, 1e-5f)); delete reader; } @@ -858,13 +869,12 @@ namespace reader->validateMessage(buffer, builtSize, LLHost()); reader->readMessage(buffer, LLHost()); reader->getU32(_PREHASH_Test0, _PREHASH_Test0, outValue); - (void)outValue; char outBuffer[bufferSize]; - memset(buffer, 0xcc, bufferSize); + memset(outBuffer, 0xcc, bufferSize); reader->getString(_PREHASH_Test1, _PREHASH_Test0, bufferSize, outBuffer); CHECK(outValue == inValue); - CHECK(strlen(outBuffer) == 0); + CHECK(outBuffer[0] == '\0'); delete reader; } @@ -908,8 +918,8 @@ namespace LLTemplateMessageBuilder* builder = defaultBuilder(messageTemplate); std::string in(300, 'x'); builder->addString(_PREHASH_Test0, in); + REQUIRE(in.size() == (size_t)300); CHECK(in[254] == 'x'); - CHECK(in.size() == (size_t)300); // round-trip: stored copy is clamped to 255 bytes and NUL-terminated LLTemplateMessageReader* reader = setReader(messageTemplate, builder); @@ -946,7 +956,7 @@ namespace builder->nextBlock(_PREHASH_Test1); builder->addU32(_PREHASH_Test0, 0xdeadbeef); - LLTemplateMessageReader* reader = setReader(messageTemplate, builder); + std::unique_ptr reader(setReader(messageTemplate, builder)); CHECK(reader->getNumberOfBlocks(_PREHASH_Test0) == NUM_BLOCKS); for (S32 i = 0; i < NUM_BLOCKS; ++i) @@ -959,7 +969,6 @@ namespace U32 single = 0; reader->getU32(_PREHASH_Test1, _PREHASH_Test0, single); CHECK(single == (U32)0xdeadbeef); - delete reader; } TEST_CASE_METHOD(LLTemplateMessageBuilderTestData, "LLTemplateMessageBuilder: one reader decodes messages of differing repeat counts in turn", "[LLTemplateMessageBuilder]") @@ -1035,7 +1044,7 @@ namespace nameMap[_PREHASH_TestMessage] = &messageTemplate; numberMap[1] = &messageTemplate; - LLTemplateMessageBuilder* builder = new LLTemplateMessageBuilder(nameMap); + auto builder = std::make_unique(nameMap); const U32 cap = 1024; for (S32 pass = 0; pass < 4; ++pass) @@ -1070,7 +1079,6 @@ namespace } builder->clearMessage(); } - delete builder; } } diff --git a/indra/llmessage/tests/lltemplatemessagedispatcher_test.cpp b/indra/llmessage/tests/lltemplatemessagedispatcher_test.cpp index a1e8210358..4c538b528e 100644 --- a/indra/llmessage/tests/lltemplatemessagedispatcher_test.cpp +++ b/indra/llmessage/tests/lltemplatemessagedispatcher_test.cpp @@ -1,6 +1,6 @@ /** - * @file lltrustedmessageservice_test.cpp - * @brief LLTrustedMessageService unit tests + * @file lltemplatemessagedispatcher_test.cpp + * @brief LLTemplateMessageDispatcher unit tests * * $LicenseInfo:firstyear=2009&license=viewerlgpl$ * Second Life Viewer Source Code @@ -79,6 +79,45 @@ const LLHost& LLMessageSystem::getSender() const return host; } +// The rest of the reader, which this binary does not link, so the dispatcher +// can be given a real one. The dispatcher calls none of it. +LLMessageReader::~LLMessageReader() {} +LLTemplateMessageReader::LLTemplateMessageReader(message_template_number_map_t& number_template_map) : + mReceiveSize(0), + mCurrentRMessageTemplate(nullptr), + mCurrentRMessageData(nullptr), + mMessageNumbers(number_template_map) +{ +} +LLTemplateMessageReader::~LLTemplateMessageReader() {} +void LLTemplateMessageReader::getBinaryData(const char*, const char*, void*, S32, S32, S32) {} +void LLTemplateMessageReader::getBOOL(const char*, const char*, bool&, S32) {} +void LLTemplateMessageReader::getS8(const char*, const char*, S8&, S32) {} +void LLTemplateMessageReader::getU8(const char*, const char*, U8&, S32) {} +void LLTemplateMessageReader::getS16(const char*, const char*, S16&, S32) {} +void LLTemplateMessageReader::getU16(const char*, const char*, U16&, S32) {} +void LLTemplateMessageReader::getS32(const char*, const char*, S32&, S32) {} +void LLTemplateMessageReader::getF32(const char*, const char*, F32&, S32) {} +void LLTemplateMessageReader::getU32(const char*, const char*, U32&, S32) {} +void LLTemplateMessageReader::getU64(const char*, const char*, U64&, S32) {} +void LLTemplateMessageReader::getF64(const char*, const char*, F64&, S32) {} +void LLTemplateMessageReader::getVector3(const char*, const char*, LLVector3&, S32) {} +void LLTemplateMessageReader::getVector4(const char*, const char*, LLVector4&, S32) {} +void LLTemplateMessageReader::getVector3d(const char*, const char*, LLVector3d&, S32) {} +void LLTemplateMessageReader::getQuat(const char*, const char*, LLQuaternion&, S32) {} +void LLTemplateMessageReader::getUUID(const char*, const char*, LLUUID&, S32) {} +void LLTemplateMessageReader::getIPAddr(const char*, const char*, U32&, S32) {} +void LLTemplateMessageReader::getIPPort(const char*, const char*, U16&, S32) {} +void LLTemplateMessageReader::getString(const char*, const char*, S32, char*, S32) {} +void LLTemplateMessageReader::getString(const char*, const char*, std::string&, S32) {} +S32 LLTemplateMessageReader::getNumberOfBlocks(const char*) { return 0; } +S32 LLTemplateMessageReader::getSize(const char*, const char*) { return 0; } +S32 LLTemplateMessageReader::getSize(const char*, S32, const char*) { return 0; } +void LLTemplateMessageReader::clearMessage() {} +const char* LLTemplateMessageReader::getMessageName() const { return ""; } +S32 LLTemplateMessageReader::getMessageSize() const { return 0; } +void LLTemplateMessageReader::copyToBuilder(LLMessageBuilder&) const {} + namespace { static LLTemplateMessageReader::message_template_number_map_t numberMap; @@ -91,9 +130,11 @@ namespace gUdpDispatchWasCalled = false; gClearRecvWasCalled = false; gValidateMessage = false; + gUdpDispatchedData[0] = '\0'; mMessage["body"]["binary-template-data"] = std::vector(); } + LLTemplateMessageReader mReader{numberMap}; LLSD mMessage; LLHTTPNode::ResponsePtr mResponsePtr; std::string mMessageName; @@ -105,18 +146,16 @@ namespace { TEST_CASE_METHOD(LLTemplateMessageDispatcherData, "LLTemplateMessageDispatcher: an empty message stops processing", "[LLTemplateMessageDispatcher]") { - LLTemplateMessageReader* pReader = NULL; - LLTemplateMessageDispatcher t(*pReader); + LLTemplateMessageDispatcher t(mReader); t.dispatch(mMessageName, mMessage, mResponsePtr); CHECK_FALSE(gUdpDispatchWasCalled); CHECK_FALSE(gClearRecvWasCalled); } - // does the disaptch invoke the udp send method? + // does dispatch hand a message that validates to the reader? TEST_CASE_METHOD(LLTemplateMessageDispatcherData, "LLTemplateMessageDispatcher: dispatch hands a valid message to the reader", "[LLTemplateMessageDispatcher]") { - LLTemplateMessageReader* pReader = NULL; - LLTemplateMessageDispatcher t(*pReader); + LLTemplateMessageDispatcher t(mReader); gValidateMessage = true; std::vector vector_data; fillVector(vector_data, gBinaryTemplateData); @@ -127,8 +166,7 @@ namespace TEST_CASE_METHOD(LLTemplateMessageDispatcherData, "LLTemplateMessageDispatcher: an invalid message gets cleared", "[LLTemplateMessageDispatcher]") { - LLTemplateMessageReader* pReader = NULL; - LLTemplateMessageDispatcher t(*pReader); + LLTemplateMessageDispatcher t(mReader); std::vector vector_data; fillVector(vector_data, gBinaryTemplateData); mMessage["body"]["binary-template-data"] = vector_data; @@ -139,8 +177,7 @@ namespace TEST_CASE_METHOD(LLTemplateMessageDispatcherData, "LLTemplateMessageDispatcher: the binary data passes through unchanged", "[LLTemplateMessageDispatcher]") { - LLTemplateMessageReader* pReader = NULL; - LLTemplateMessageDispatcher t(*pReader); + LLTemplateMessageDispatcher t(mReader); gValidateMessage = true; std::vector vector_data; fillVector(vector_data, gBinaryTemplateData); diff --git a/indra/llmessage/tests/llxorcipher_test.cpp b/indra/llmessage/tests/llxorcipher_test.cpp index 821d460282..f0977daa97 100644 --- a/indra/llmessage/tests/llxorcipher_test.cpp +++ b/indra/llmessage/tests/llxorcipher_test.cpp @@ -91,7 +91,7 @@ namespace // it should not be the same as original data! CHECK(memcmp(str, str1, str_len) != 0); xorCipher1.decrypt((U8 *) str, str_len); - // it should not be the same as original data! + // it should be the original data again CHECK_THAT(ll_test::Memory(str, str_len), ll_test::MemoryMatches(str1, str_len)); } } diff --git a/indra/llmessage/tests/message_test.cpp b/indra/llmessage/tests/message_test.cpp index acf1a380f4..c88e29fba1 100644 --- a/indra/llmessage/tests/message_test.cpp +++ b/indra/llmessage/tests/message_test.cpp @@ -1,7 +1,7 @@ /** - * @file lldatapacker_tut.cpp + * @file message_test.cpp * @date 2007-04 - * @brief LLDataPacker test cases. + * @brief LLMessageSystem test cases. * * $LicenseInfo:firstyear=2007&license=viewerlgpl$ * Second Life Viewer Source Code @@ -54,7 +54,7 @@ namespace } virtual void extendedResult(S32 code, const std::string& message, const LLSD& headers) { } virtual void extendedResult(S32 code, const LLSD& result, const LLSD& headers) { } - S32 mStatus; + S32 mStatus = 0; // no status() yet }; }