From 3d5024647866a1c3811360ca9cd6431a68278b75 Mon Sep 17 00:00:00 2001 From: Balazs Racz Date: Tue, 11 Aug 2020 00:51:02 +0200 Subject: [PATCH 01/11] Adds VirtualMemorySpace a memory space where the values are not stored in a contiguous storage area but are read and written via callbacks. --- src/openlcb/VirtualMemorySpace.cxxtest | 195 +++++++++++++++ src/openlcb/VirtualMemorySpace.hxx | 323 +++++++++++++++++++++++++ 2 files changed, 518 insertions(+) create mode 100644 src/openlcb/VirtualMemorySpace.cxxtest create mode 100644 src/openlcb/VirtualMemorySpace.hxx diff --git a/src/openlcb/VirtualMemorySpace.cxxtest b/src/openlcb/VirtualMemorySpace.cxxtest new file mode 100644 index 000000000..db42345b4 --- /dev/null +++ b/src/openlcb/VirtualMemorySpace.cxxtest @@ -0,0 +1,195 @@ +/** \copyright + * Copyright (c) 2014, Balazs Racz + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * - Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * + * - Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + * + * \file VirtualMemorySpace.cxxtest + * + * Unit tests for the virtual memory space. + * + * @author Balazs Racz + * @date 10 Aug 2020 + */ + +#include "openlcb/VirtualMemorySpace.hxx" + +#include "openlcb/ConfigRepresentation.hxx" +#include "openlcb/MemoryConfigClient.hxx" +#include "utils/async_datagram_test_helper.hxx" + +namespace openlcb +{ + +string arg1; +string arg2; + +CDI_GROUP(ExampleMemorySpace); +CDI_GROUP_ENTRY(skipped, EmptyGroup<5>); +CDI_GROUP_ENTRY(first, StringConfigEntry<13>); +CDI_GROUP_ENTRY(skipped2, EmptyGroup<8>); +CDI_GROUP_ENTRY(second, StringConfigEntry<20>); +CDI_GROUP_END(); + +ExampleMemorySpace cfg(44); + +const unsigned arg1_ofs = 44 + 5; +const unsigned arg2_ofs = 44 + 5 + 13 + 8; + +class VirtualMemorySpaceTest : public AsyncDatagramTest +{ +protected: + ~VirtualMemorySpaceTest() + { + wait(); + } + + MemoryConfigHandler memCfg_ {&datagram_support_, node_, 3}; + std::unique_ptr space_; + MemoryConfigClient client_ {node_, &memCfg_}; +}; + +class TestSpace : public VirtualMemorySpace +{ +public: + TestSpace() + { + arg1.clear(); + arg2.clear(); + register_string( + cfg.first(), string_reader(&arg1), string_writer(&arg1)); + register_string( + cfg.second(), string_reader(&arg2), string_writer(&arg2)); + } + + std::function + string_reader(string *ptr) + { + return + [ptr](unsigned repeat, string *contents, BarrierNotifiable *done) { + *contents = *ptr; + done->notify(); + return true; + }; + } + + std::function + string_writer(string *ptr) + { + return + [ptr](unsigned repeat, string contents, BarrierNotifiable *done) { + *ptr = std::move(contents); + done->notify(); + }; + } +}; + +class TestSpaceTest : public VirtualMemorySpaceTest +{ +protected: + TestSpaceTest() + { + space_.reset(new TestSpace); + memCfg_.registry()->insert(node_, SPACE, space_.get()); + } + + const uint8_t SPACE = 0x52; +}; + +TEST_F(TestSpaceTest, create) +{ +} + +TEST_F(TestSpaceTest, read_payload) +{ + arg1 = "hello"; + auto b = invoke_flow(&client_, MemoryConfigClientRequest::READ_PART, + NodeHandle(node_->node_id()), SPACE, arg1_ofs, 13); + ASSERT_EQ(0, b->data()->resultCode); + EXPECT_STREQ("hello", b->data()->payload.c_str()); + EXPECT_EQ(13u, b->data()->payload.size()); + + b = invoke_flow(&client_, MemoryConfigClientRequest::READ_PART, + NodeHandle(node_->node_id()), SPACE, arg2_ofs, 20); + ASSERT_EQ(0, b->data()->resultCode); + EXPECT_STREQ("", b->data()->payload.c_str()); + EXPECT_EQ(20u, b->data()->payload.size()); + + // prefix read + b = invoke_flow(&client_, MemoryConfigClientRequest::READ_PART, + NodeHandle(node_->node_id()), SPACE, arg1_ofs, 3); + ASSERT_EQ(0, b->data()->resultCode); + EXPECT_EQ("hel", b->data()->payload); +} + +TEST_F(TestSpaceTest, read_early) +{ + arg1 = "hello"; + auto b = invoke_flow(&client_, MemoryConfigClientRequest::READ_PART, + NodeHandle(node_->node_id()), SPACE, arg1_ofs - 2, 10); + ASSERT_EQ(0, b->data()->resultCode); + string exp("\0\0hello\0\0\0", 10); + EXPECT_EQ(exp, b->data()->payload); + EXPECT_EQ(10u, b->data()->payload.size()); + + b = invoke_flow(&client_, MemoryConfigClientRequest::READ_PART, + NodeHandle(node_->node_id()), SPACE, arg2_ofs - 4, 3); + ASSERT_EQ(0, b->data()->resultCode); + string exp2("\0\0\0", 3); + EXPECT_EQ(exp2, b->data()->payload); + EXPECT_EQ(3u, b->data()->payload.size()); +} + +TEST_F(TestSpaceTest, write_payload) +{ + auto b = invoke_flow(&client_, MemoryConfigClientRequest::WRITE, + NodeHandle(node_->node_id()), SPACE, arg1_ofs, "xyzw"); + ASSERT_EQ(0, b->data()->resultCode); + EXPECT_EQ("xyzw", arg1); + EXPECT_EQ(4u, arg1.size()); + + b = invoke_flow(&client_, MemoryConfigClientRequest::WRITE, + NodeHandle(node_->node_id()), SPACE, arg2_ofs, "abcde"); + ASSERT_EQ(0, b->data()->resultCode); + EXPECT_EQ("abcde", arg2); + EXPECT_EQ(5u, arg2.size()); +} + +TEST_F(TestSpaceTest, write_early) +{ + auto b = invoke_flow(&client_, MemoryConfigClientRequest::WRITE, + NodeHandle(node_->node_id()), SPACE, arg1_ofs-2, "xyzw"); + ASSERT_EQ(0, b->data()->resultCode); + EXPECT_EQ("zw", arg1); + EXPECT_EQ(2u, arg1.size()); + + b = invoke_flow(&client_, MemoryConfigClientRequest::WRITE, + NodeHandle(node_->node_id()), SPACE, arg2_ofs-1, "qwert"); + ASSERT_EQ(0, b->data()->resultCode); + EXPECT_EQ("wert", arg2); + EXPECT_EQ(4u, arg2.size()); +} + +} // namespace openlcb diff --git a/src/openlcb/VirtualMemorySpace.hxx b/src/openlcb/VirtualMemorySpace.hxx new file mode 100644 index 000000000..79768d260 --- /dev/null +++ b/src/openlcb/VirtualMemorySpace.hxx @@ -0,0 +1,323 @@ +/** \copyright + * Copyright (c) 2020, Balazs Racz + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * - Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * + * - Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + * + * \file VirtualMemorySpace.hxx + * + * Implementation of a memory space where the values are not stored in a + * contiguous storage area but are read anf written via callbacks. + * + * @author Balazs Racz + * @date 10 Aug 2020 + */ + +#ifndef _OPENLCB_VIRTUALMEMORYSPACE_HXX +#define _OPENLCB_VIRTUALMEMORYSPACE_HXX + +#include "openlcb/ConfigEntry.hxx" +#include "openlcb/MemoryConfig.hxx" +#include "utils/SortedListMap.hxx" + +namespace openlcb +{ + +/// Implementation of a memory space where the values are not stored in a +/// contiguous storage area but are read anf written via callbacks. +class VirtualMemorySpace : public MemorySpace +{ +public: + /// @returns whether the memory space does not accept writes. + bool read_only() override + { + return isReadOnly_; + } + /// @returns the lowest address that's valid for this block. + address_t min_address() override + { + return minAddress_; + } + /// @returns the largest valid address for this block. A read of 1 from + /// this address should succeed in returning the last byte. + address_t max_address() override + { + return maxAddress_; + } + + /// @return the number of bytes successfully written (before hitting end + /// of space). + /// @param destination address to write to + /// @param data to write + /// @param len how many bytes to write + /// @param error if set to non-null, then the operation has failed. If the + /// operation needs to be continued, then sets error to + /// MemorySpace::ERROR_AGAIN, and calls the Notifiable + /// @param again when a re-try makes sense. The caller should call write + /// once more, with the offset adjusted with the previously returned + /// bytes. + size_t write(address_t destination, const uint8_t *data, size_t len, + errorcode_t *error, Notifiable *again) override + { + if (destination > maxAddress_ || destination + len <= minAddress_) + { + *error = MemoryConfigDefs::ERROR_OUT_OF_BOUNDS; + return 0; + } + *error = 0; + unsigned repeat; + const DataElement *element = nullptr; + ssize_t skip = find_data_element(destination, len, &element, &repeat); + string payload; + if (skip > 0) + { + // Will cause a new call be delivered with adjusted data and len. + return skip; + } + else if (skip < 0) + { + // We have some missing bytes that we need to read out first, then + // can perform the write. + DIE("unimplemented"); + // if (!element->readImpl_(repeat, &payload, + } + HASSERT(element); + payload.assign( + (const char *)data, std::min((size_t)len, (size_t)element->size_)); + size_t written_len = payload.size(); + bn_.reset(again); + element->writeImpl_(repeat, std::move(payload), bn_.new_child()); + if (!bn_.abort_if_almost_done()) + { + // did not succeed synchronously. + *error = MemorySpace::ERROR_AGAIN; + return 0; + } + return written_len; + } + + /** @returns the number of bytes successfully read (before hitting end of + * space). If *error is set to non-null, then the operation has failed. If + * the operation needs to be continued, then sets error to ERROR_AGAIN, and + * calls the Notifiable @param again when a re-try makes sense. The caller + * should call read once more, with the offset adjusted with the previously + * returned bytes. */ + size_t read(address_t source, uint8_t *dst, size_t len, errorcode_t *error, + Notifiable *again) override + { + if (source > maxAddress_ || source + len <= minAddress_) + { + *error = MemoryConfigDefs::ERROR_OUT_OF_BOUNDS; + return 0; + } + *error = 0; + unsigned repeat; + const DataElement *element = nullptr; + ssize_t skip = find_data_element(source, len, &element, &repeat); + if (skip > 0) + { + memset(dst, 0, skip); + return skip; + } + else if (skip < 0) + { + DIE("unimplemented"); + } + HASSERT(element); + string payload; + bn_.reset(again); + element->readImpl_(repeat, &payload, bn_.new_child()); + if (!bn_.abort_if_almost_done()) + { + // did not succeed synchronously. + *error = MemorySpace::ERROR_AGAIN; + return 0; + } + payload.resize(element->size_); // pads with zeroes + size_t data_len = std::min(payload.size(), len); + memcpy(dst, payload.data(), data_len); + return data_len; + } + +protected: + /// Function that will be called for writes. + /// @param repeat 0 to number of repeats if this is in a repeated + /// group. Always 0 if not repeated group. + /// @param contents data payload that needs to be written. The data + /// bytes of this container start at the address_. + /// @param done must be notified when the write is complete (possibly + /// inline). + using WriteFunction = std::function; + /// Function that will be called for reads. + /// @param repeat 0 to number of repeats if this is in a repeated + /// group. Always 0 if not repeated group. + /// @param contents the payload to be returned from this variable shall + /// be written here. Will be zero-padded to size_ bytes if shorter. + /// @param done must be notified when the read values are ready. The + /// call will be re-tried in this case. + /// @return true if the read was successful, false if the read needs to be + /// re-tried later, + using ReadFunction = std::function; + + /// Setup the address bounds from a single CDI group declaration. + /// @param group is an instance of a group, for example a segment. + template void set_bounds_from_group(const G &group) + { + minAddress_ = group.offset(); + maxAddress_ = group.offset() + group.size() - 1; + } + + /// Expand the address bounds from a single CDI group declaration. + /// @param group is an instance of a group, for example a segment. + template void expand_bounds_from_group(const G &group) + { + minAddress_ = std::min(minAddress_, group.offset()); + maxAddress_ = std::max(maxAddress_, group.offset() + group.size() - 1); + } + + /// Register an untyped element. + /// @param address the address in the memory space + /// @param size how many bytes this elements occupes + /// @param read_f will be called to read this data + /// @param write_f will be called to write this data + void register_element(address_t address, address_t size, + ReadFunction read_f, WriteFunction write_f) + { + elements_.insert(DataElement(address, size, read_f, write_f)); + } + + /// Registers a string typed element. + /// @param entry is the CDI ConfigRepresentation. + /// @param read_f will be called to read this data + /// @param write_f will be called to write this data + template + void register_string(const StringConfigEntry &entry, + ReadFunction read_f, WriteFunction write_f) + { + expand_bounds_from_group(entry); + register_element(entry.offset(), SIZE, read_f, write_f); + } + + /// Bounds for valid addresses. + address_t minAddress_ = 0xFFFFFFFFu; + /// Bounds for valid addresses. A read of length 1 from this address + /// should succeed in returning the last byte. + address_t maxAddress_ = 0; + /// Whether the space should report as RO. + bool isReadOnly_ = false; + +private: + /// We keep one of these for each variable that was declared. + struct DataElement + { + DataElement(address_t address, address_t size, ReadFunction read_f, + WriteFunction write_f) + : address_(address) + , size_(size) + , writeImpl_(write_f) + , readImpl_(read_f) + { + } + /// Base offset of this variable (first repeat only). + address_t address_; + /// Size of this variable. This is how many bytes of address space this + /// variable occupies. + address_t size_; + /// Function that will be called for writes. + WriteFunction writeImpl_; + /// Function that will be called for reads. + ReadFunction readImpl_; + }; + + struct Comparator + { + /// Sorting operator by address. + bool operator()(const DataElement &a, const DataElement &b) const + { + return a.address_ < b.address_; + } + /// Sorting operator by address. + bool operator()(unsigned a, const DataElement &b) const + { + return a < b.address_; + } + }; + + /// Look up the first matching data element given an address in the virtual + /// memory space. + /// @param address byte offset to look up. + /// @param len how many bytes long range to search from address. + /// @param ptr will be filled with a pointer to the data element when + /// found, or filled with nullptr if no data element overlaps with the + /// given range. + /// @param repeat output argument, filled with zero or the repetition + /// number. + /// @return 0 if an exact match is found. -N if a data element is found, + /// but N first bytes of this element are not covered. A number N in + /// [1..len-1] if a data element is found, but this many bytes need to be + /// skipped from address to arrive at the given data element's offset. len + /// if there was no data element found (in which case also set ptr to + /// null). + ssize_t find_data_element(address_t address, address_t len, + const DataElement **ptr, unsigned *repeat) + { + *repeat = 0; + *ptr = nullptr; + auto it = elements_.upper_bound(address); + if (it != elements_.begin()) + { + auto pit = it - 1; + // now: pit->address_ <= address + if (pit->address_ + pit->size_ > address) + { + // found overlap + *ptr = &*pit; + return (ssize_t)pit->address_ - + (ssize_t)address; // may be negative! + } + // else: no overlap, look at the next item + } + // now: it->address_ > address + if (address + len > it->address_) + { + // found overlap, but some data needs to be discarded. + *ptr = &*it; + return it->address_ - address; + } + /// @todo try repeated fields here first. + + // now: no overlap either before or after. + return len; + } + + /// Stores all the registered variables. + SortedListSet elements_; + /// Helper object in the function calls. + BarrierNotifiable bn_; +}; // class VirtualMemorySpace + +} // namespace openlcb + +#endif // _OPENLCB_VIRTUALMEMORYSPACE_HXX From ccd35c323b6aab5fdacdf01e970e9504494fa74b Mon Sep 17 00:00:00 2001 From: Balazs Racz Date: Wed, 12 Aug 2020 20:58:03 +0200 Subject: [PATCH 02/11] Adds comments. --- src/openlcb/VirtualMemorySpace.cxxtest | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/openlcb/VirtualMemorySpace.cxxtest b/src/openlcb/VirtualMemorySpace.cxxtest index db42345b4..08f3f26c3 100644 --- a/src/openlcb/VirtualMemorySpace.cxxtest +++ b/src/openlcb/VirtualMemorySpace.cxxtest @@ -82,6 +82,11 @@ public: cfg.second(), string_reader(&arg2), string_writer(&arg2)); } + /// Creates a ReaderFunction that just returns a string from a given + /// variable. + /// @param ptr the string whose contents to return as read value. Must stay + /// alive as long as the function is in use. + /// @return the ReaderFunction. std::function string_reader(string *ptr) @@ -94,6 +99,11 @@ public: }; } + /// Creates a WriterFunction that just stores the data in a given string + /// variable. + /// @param ptr the string whose contents to return as read value. Must stay + /// alive as long as the function is in use. + /// @return the ReaderFunction. std::function string_writer(string *ptr) @@ -115,6 +125,7 @@ protected: memCfg_.registry()->insert(node_, SPACE, space_.get()); } + /// Memory space number where the test space is registered. const uint8_t SPACE = 0x52; }; @@ -122,6 +133,8 @@ TEST_F(TestSpaceTest, create) { } +/// Basic tests reading variables from the exact offset including partial +/// reads. TEST_F(TestSpaceTest, read_payload) { arg1 = "hello"; @@ -144,6 +157,7 @@ TEST_F(TestSpaceTest, read_payload) EXPECT_EQ("hel", b->data()->payload); } +/// Test reading a variable from an imprecise offset (too early). TEST_F(TestSpaceTest, read_early) { arg1 = "hello"; @@ -162,6 +176,8 @@ TEST_F(TestSpaceTest, read_early) EXPECT_EQ(3u, b->data()->payload.size()); } +/// Basic tests writing variables from the exact offset but not including +/// partial writes. TEST_F(TestSpaceTest, write_payload) { auto b = invoke_flow(&client_, MemoryConfigClientRequest::WRITE, @@ -177,6 +193,7 @@ TEST_F(TestSpaceTest, write_payload) EXPECT_EQ(5u, arg2.size()); } +/// Test writing a variable to a offset that is too early. TEST_F(TestSpaceTest, write_early) { auto b = invoke_flow(&client_, MemoryConfigClientRequest::WRITE, From a52543d61f9b4e0b12b71ca34ddee87975e6617c Mon Sep 17 00:00:00 2001 From: Balazs Racz Date: Wed, 12 Aug 2020 21:03:35 +0200 Subject: [PATCH 03/11] Adds more tests. --- src/openlcb/VirtualMemorySpace.cxxtest | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/openlcb/VirtualMemorySpace.cxxtest b/src/openlcb/VirtualMemorySpace.cxxtest index 08f3f26c3..8923a2f9b 100644 --- a/src/openlcb/VirtualMemorySpace.cxxtest +++ b/src/openlcb/VirtualMemorySpace.cxxtest @@ -209,4 +209,21 @@ TEST_F(TestSpaceTest, write_early) EXPECT_EQ(4u, arg2.size()); } +/// Test writing a variable to a offset that is not covered at all. +TEST_F(TestSpaceTest, write_hole) +{ + auto b = invoke_flow(&client_, MemoryConfigClientRequest::WRITE, + NodeHandle(node_->node_id()), SPACE, arg2_ofs-5, "xyz"); + ASSERT_EQ(0, b->data()->resultCode); + + b = invoke_flow(&client_, MemoryConfigClientRequest::WRITE, + NodeHandle(node_->node_id()), SPACE, arg1_ofs-5, "qw"); + ASSERT_EQ(0, b->data()->resultCode); + + b = invoke_flow(&client_, MemoryConfigClientRequest::WRITE, + NodeHandle(node_->node_id()), SPACE, arg2_ofs+100, "qw"); + ASSERT_EQ(0, b->data()->resultCode); +} + + } // namespace openlcb From b7026048966372a449e56675ce44668249c889f5 Mon Sep 17 00:00:00 2001 From: Balazs Racz Date: Wed, 12 Aug 2020 21:22:10 +0200 Subject: [PATCH 04/11] Adds another test. --- src/openlcb/VirtualMemorySpace.cxxtest | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/openlcb/VirtualMemorySpace.cxxtest b/src/openlcb/VirtualMemorySpace.cxxtest index 8923a2f9b..5e216e273 100644 --- a/src/openlcb/VirtualMemorySpace.cxxtest +++ b/src/openlcb/VirtualMemorySpace.cxxtest @@ -176,6 +176,30 @@ TEST_F(TestSpaceTest, read_early) EXPECT_EQ(3u, b->data()->payload.size()); } +/// Test writing a variable to a offset that is not covered at all. +TEST_F(TestSpaceTest, read_hole) +{ + auto b = invoke_flow(&client_, MemoryConfigClientRequest::READ_PART, + NodeHandle(node_->node_id()), SPACE, arg2_ofs-5, 3); + ASSERT_EQ(0, b->data()->resultCode); + string exp("\0\0\0", 3); + EXPECT_EQ(exp, b->data()->payload); + + /// @todo this return seems to be wrong, although this address is out of + /// the memory space limits. + b = invoke_flow(&client_, MemoryConfigClientRequest::READ_PART, + NodeHandle(node_->node_id()), SPACE, arg1_ofs-5, 2); + ASSERT_EQ(0, b->data()->resultCode); + EXPECT_EQ(string(), b->data()->payload); + + /// @todo this return seems to be wrong, although this address is out of + /// the memory space limits. + b = invoke_flow(&client_, MemoryConfigClientRequest::READ_PART, + NodeHandle(node_->node_id()), SPACE, arg2_ofs + 100, 4); + ASSERT_EQ(0, b->data()->resultCode); + EXPECT_EQ(string(), b->data()->payload); +} + /// Basic tests writing variables from the exact offset but not including /// partial writes. TEST_F(TestSpaceTest, write_payload) From b8c6730b40d6163c994dcb3bb22a95374b1c0903 Mon Sep 17 00:00:00 2001 From: Balazs Racz Date: Wed, 12 Aug 2020 21:36:15 +0200 Subject: [PATCH 05/11] Fix copyright date. --- src/openlcb/VirtualMemorySpace.cxxtest | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/openlcb/VirtualMemorySpace.cxxtest b/src/openlcb/VirtualMemorySpace.cxxtest index 5e216e273..4463cf0be 100644 --- a/src/openlcb/VirtualMemorySpace.cxxtest +++ b/src/openlcb/VirtualMemorySpace.cxxtest @@ -1,5 +1,5 @@ /** \copyright - * Copyright (c) 2014, Balazs Racz + * Copyright (c) 2020, Balazs Racz * All rights reserved. * * Redistribution and use in source and binary forms, with or without From 26be39531b168869c71fa1dc8452814077284d99 Mon Sep 17 00:00:00 2001 From: Balazs Racz Date: Wed, 12 Aug 2020 21:44:55 +0200 Subject: [PATCH 06/11] fix typos. --- src/openlcb/VirtualMemorySpace.hxx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/openlcb/VirtualMemorySpace.hxx b/src/openlcb/VirtualMemorySpace.hxx index 79768d260..48d759c95 100644 --- a/src/openlcb/VirtualMemorySpace.hxx +++ b/src/openlcb/VirtualMemorySpace.hxx @@ -27,7 +27,7 @@ * \file VirtualMemorySpace.hxx * * Implementation of a memory space where the values are not stored in a - * contiguous storage area but are read anf written via callbacks. + * contiguous storage area but are read and written via callbacks. * * @author Balazs Racz * @date 10 Aug 2020 @@ -44,7 +44,7 @@ namespace openlcb { /// Implementation of a memory space where the values are not stored in a -/// contiguous storage area but are read anf written via callbacks. +/// contiguous storage area but are read and written via callbacks. class VirtualMemorySpace : public MemorySpace { public: From a845f8d70b8090cb248a31c4924cba139fd360da Mon Sep 17 00:00:00 2001 From: Balazs Racz Date: Thu, 13 Aug 2020 21:06:34 +0200 Subject: [PATCH 07/11] Adds unit tests for asynchronous execution on the memory space. Fixes a respective bug in the implementation. --- src/openlcb/VirtualMemorySpace.cxxtest | 131 +++++++++++++++++++++++-- src/openlcb/VirtualMemorySpace.hxx | 14 ++- 2 files changed, 134 insertions(+), 11 deletions(-) diff --git a/src/openlcb/VirtualMemorySpace.cxxtest b/src/openlcb/VirtualMemorySpace.cxxtest index 4463cf0be..ba84e044c 100644 --- a/src/openlcb/VirtualMemorySpace.cxxtest +++ b/src/openlcb/VirtualMemorySpace.cxxtest @@ -180,7 +180,7 @@ TEST_F(TestSpaceTest, read_early) TEST_F(TestSpaceTest, read_hole) { auto b = invoke_flow(&client_, MemoryConfigClientRequest::READ_PART, - NodeHandle(node_->node_id()), SPACE, arg2_ofs-5, 3); + NodeHandle(node_->node_id()), SPACE, arg2_ofs - 5, 3); ASSERT_EQ(0, b->data()->resultCode); string exp("\0\0\0", 3); EXPECT_EQ(exp, b->data()->payload); @@ -188,7 +188,7 @@ TEST_F(TestSpaceTest, read_hole) /// @todo this return seems to be wrong, although this address is out of /// the memory space limits. b = invoke_flow(&client_, MemoryConfigClientRequest::READ_PART, - NodeHandle(node_->node_id()), SPACE, arg1_ofs-5, 2); + NodeHandle(node_->node_id()), SPACE, arg1_ofs - 5, 2); ASSERT_EQ(0, b->data()->resultCode); EXPECT_EQ(string(), b->data()->payload); @@ -221,13 +221,13 @@ TEST_F(TestSpaceTest, write_payload) TEST_F(TestSpaceTest, write_early) { auto b = invoke_flow(&client_, MemoryConfigClientRequest::WRITE, - NodeHandle(node_->node_id()), SPACE, arg1_ofs-2, "xyzw"); + NodeHandle(node_->node_id()), SPACE, arg1_ofs - 2, "xyzw"); ASSERT_EQ(0, b->data()->resultCode); EXPECT_EQ("zw", arg1); EXPECT_EQ(2u, arg1.size()); b = invoke_flow(&client_, MemoryConfigClientRequest::WRITE, - NodeHandle(node_->node_id()), SPACE, arg2_ofs-1, "qwert"); + NodeHandle(node_->node_id()), SPACE, arg2_ofs - 1, "qwert"); ASSERT_EQ(0, b->data()->resultCode); EXPECT_EQ("wert", arg2); EXPECT_EQ(4u, arg2.size()); @@ -237,17 +237,134 @@ TEST_F(TestSpaceTest, write_early) TEST_F(TestSpaceTest, write_hole) { auto b = invoke_flow(&client_, MemoryConfigClientRequest::WRITE, - NodeHandle(node_->node_id()), SPACE, arg2_ofs-5, "xyz"); + NodeHandle(node_->node_id()), SPACE, arg2_ofs - 5, "xyz"); ASSERT_EQ(0, b->data()->resultCode); b = invoke_flow(&client_, MemoryConfigClientRequest::WRITE, - NodeHandle(node_->node_id()), SPACE, arg1_ofs-5, "qw"); + NodeHandle(node_->node_id()), SPACE, arg1_ofs - 5, "qw"); ASSERT_EQ(0, b->data()->resultCode); b = invoke_flow(&client_, MemoryConfigClientRequest::WRITE, - NodeHandle(node_->node_id()), SPACE, arg2_ofs+100, "qw"); + NodeHandle(node_->node_id()), SPACE, arg2_ofs + 100, "qw"); ASSERT_EQ(0, b->data()->resultCode); } +class TestSpaceAsync : public VirtualMemorySpace +{ +public: + TestSpaceAsync() + { + arg1.clear(); + arg2.clear(); + register_string( + cfg.first(), string_reader(&arg1), string_writer(&arg1)); + register_string( + cfg.second(), string_reader(&arg2), string_writer(&arg2)); + } + + /// Creates a ReaderFunction that just returns a string from a given + /// variable. + /// @param ptr the string whose contents to return as read value. Must stay + /// alive as long as the function is in use. + /// @return the ReaderFunction. + std::function + string_reader(string *ptr) + { + return [this, ptr]( + unsigned repeat, string *contents, BarrierNotifiable *done) { + attempt++; + if ((attempt & 1) == 0) + { + *contents = *ptr; + done->notify(); + return true; + } + else + { + g_executor.add( + new CallbackExecutable([done]() { done->notify(); })); + return false; + } + }; + } + + /// Creates a WriterFunction that just stores the data in a given string + /// variable. + /// @param ptr the string whose contents to return as read value. Must stay + /// alive as long as the function is in use. + /// @return the ReaderFunction. + std::function + string_writer(string *ptr) + { + return [this, ptr]( + unsigned repeat, string contents, BarrierNotifiable *done) { + attempt++; + if ((attempt & 1) == 0) + { + *ptr = std::move(contents); + done->notify(); + return true; + } + else + { + g_executor.add( + new CallbackExecutable([done]() { done->notify(); })); + return false; + } + }; + } + +private: + size_t attempt = 0; +}; + +class TestSpaceAsyncTest : public VirtualMemorySpaceTest +{ +protected: + TestSpaceAsyncTest() + { + space_.reset(new TestSpaceAsync); + memCfg_.registry()->insert(node_, SPACE, space_.get()); + } + + /// Memory space number where the test space is registered. + const uint8_t SPACE = 0x52; +}; + +/// Basic tests reading variables from async space. +TEST_F(TestSpaceAsyncTest, read_payload_async) +{ + arg1 = "hello"; + auto b = invoke_flow(&client_, MemoryConfigClientRequest::READ_PART, + NodeHandle(node_->node_id()), SPACE, arg1_ofs, 13); + ASSERT_EQ(0, b->data()->resultCode); + EXPECT_STREQ("hello", b->data()->payload.c_str()); + EXPECT_EQ(13u, b->data()->payload.size()); + + b = invoke_flow(&client_, MemoryConfigClientRequest::READ_PART, + NodeHandle(node_->node_id()), SPACE, arg2_ofs, 20); + ASSERT_EQ(0, b->data()->resultCode); + EXPECT_STREQ("", b->data()->payload.c_str()); + EXPECT_EQ(20u, b->data()->payload.size()); +} + +/// Basic tests writing variables from the exact offset but not including +/// partial writes. +TEST_F(TestSpaceAsyncTest, write_payload_async) +{ + auto b = invoke_flow(&client_, MemoryConfigClientRequest::WRITE, + NodeHandle(node_->node_id()), SPACE, arg1_ofs, "xyzw"); + ASSERT_EQ(0, b->data()->resultCode); + EXPECT_EQ("xyzw", arg1); + EXPECT_EQ(4u, arg1.size()); + + b = invoke_flow(&client_, MemoryConfigClientRequest::WRITE, + NodeHandle(node_->node_id()), SPACE, arg2_ofs, "abcde"); + ASSERT_EQ(0, b->data()->resultCode); + EXPECT_EQ("abcde", arg2); + EXPECT_EQ(5u, arg2.size()); +} } // namespace openlcb diff --git a/src/openlcb/VirtualMemorySpace.hxx b/src/openlcb/VirtualMemorySpace.hxx index 48d759c95..0ba99e4e1 100644 --- a/src/openlcb/VirtualMemorySpace.hxx +++ b/src/openlcb/VirtualMemorySpace.hxx @@ -107,13 +107,17 @@ public: size_t written_len = payload.size(); bn_.reset(again); element->writeImpl_(repeat, std::move(payload), bn_.new_child()); - if (!bn_.abort_if_almost_done()) + if (bn_.abort_if_almost_done()) + { + return written_len; + } + else { // did not succeed synchronously. + bn_.notify(); // our slice *error = MemorySpace::ERROR_AGAIN; return 0; } - return written_len; } /** @returns the number of bytes successfully read (before hitting end of @@ -150,6 +154,7 @@ public: if (!bn_.abort_if_almost_done()) { // did not succeed synchronously. + bn_.notify(); // our slice *error = MemorySpace::ERROR_AGAIN; return 0; } @@ -193,8 +198,9 @@ protected: /// @param group is an instance of a group, for example a segment. template void expand_bounds_from_group(const G &group) { - minAddress_ = std::min(minAddress_, group.offset()); - maxAddress_ = std::max(maxAddress_, group.offset() + group.size() - 1); + minAddress_ = std::min(minAddress_, (address_t)group.offset()); + maxAddress_ = std::max( + maxAddress_, (address_t)(group.offset() + group.size() - 1)); } /// Register an untyped element. From 36ede35051fa66b1e5114f62f726b5d32cac7cc4 Mon Sep 17 00:00:00 2001 From: Balazs Racz Date: Sat, 15 Aug 2020 01:09:29 +0200 Subject: [PATCH 08/11] Adds handling of repetitions to the virtual memory space. --- src/openlcb/VirtualMemorySpace.cxxtest | 204 +++++++++++++++++++++++++ src/openlcb/VirtualMemorySpace.hxx | 132 +++++++++++++--- 2 files changed, 318 insertions(+), 18 deletions(-) diff --git a/src/openlcb/VirtualMemorySpace.cxxtest b/src/openlcb/VirtualMemorySpace.cxxtest index ba84e044c..6d92ef448 100644 --- a/src/openlcb/VirtualMemorySpace.cxxtest +++ b/src/openlcb/VirtualMemorySpace.cxxtest @@ -49,6 +49,7 @@ CDI_GROUP_ENTRY(skipped, EmptyGroup<5>); CDI_GROUP_ENTRY(first, StringConfigEntry<13>); CDI_GROUP_ENTRY(skipped2, EmptyGroup<8>); CDI_GROUP_ENTRY(second, StringConfigEntry<20>); +CDI_GROUP_ENTRY(skipped3, EmptyGroup<8>); CDI_GROUP_END(); ExampleMemorySpace cfg(44); @@ -367,4 +368,207 @@ TEST_F(TestSpaceAsyncTest, write_payload_async) EXPECT_EQ(5u, arg2.size()); } +CDI_GROUP(RepeatMemoryDef); +CDI_GROUP_ENTRY(skipped, EmptyGroup<5>); +CDI_GROUP_ENTRY(before, StringConfigEntry<13>); +CDI_GROUP_ENTRY(skipped2, EmptyGroup<8>); +using GroupRept = RepeatedGroup; +CDI_GROUP_ENTRY(grp, GroupRept); +CDI_GROUP_ENTRY(skipped3, EmptyGroup<8>); +CDI_GROUP_ENTRY(after, StringConfigEntry<20>); +CDI_GROUP_END(); + +RepeatMemoryDef spacerept(22); + +class SpaceWithRepeat : public VirtualMemorySpace +{ +public: + SpaceWithRepeat() + { + arg1.clear(); + arg2.clear(); + register_string(spacerept.grp().entry<0>().first(), + string_reader(&arg1), string_writer(&arg1)); + register_string(spacerept.grp().entry<0>().second(), + string_reader(&arg2), string_writer(&arg2)); + register_string(spacerept.before(), string_reader(&before_), + string_writer(&before_)); + register_string( + spacerept.after(), string_reader(&after_), string_writer(&after_)); + register_repeat(spacerept.grp()); + } + + /// Creates a ReaderFunction that just returns a string from a given + /// variable. + /// @param ptr the string whose contents to return as read value. Must stay + /// alive as long as the function is in use. + /// @return the ReaderFunction. + std::function + string_reader(string *ptr) + { + return [this, ptr]( + unsigned repeat, string *contents, BarrierNotifiable *done) { + lastRepeat_ = repeat; + *contents = *ptr; + done->notify(); + return true; + }; + } + + /// Creates a WriterFunction that just stores the data in a given string + /// variable. + /// @param ptr the string whose contents to return as read value. Must stay + /// alive as long as the function is in use. + /// @return the ReaderFunction. + std::function + string_writer(string *ptr) + { + return [this, ptr]( + unsigned repeat, string contents, BarrierNotifiable *done) { + lastRepeat_ = repeat; + *ptr = std::move(contents); + done->notify(); + }; + } + + /// Saves the last repeat variable into this value. + unsigned lastRepeat_; + /// Storage variable for a field. + string before_; + /// Storage variable for a field. + string after_; +}; + +class ReptSpaceTest : public VirtualMemorySpaceTest +{ +protected: + ReptSpaceTest() + { + memCfg_.registry()->insert(node_, SPACE, &s_); + } + + /// Memory space number where the test space is registered. + const uint8_t SPACE = 0x52; + SpaceWithRepeat s_; +}; + +TEST_F(ReptSpaceTest, create) +{ +} + +// Looks for a field that is before the repeated group. +TEST_F(ReptSpaceTest, before) +{ + s_.before_ = "hello"; + auto b = invoke_flow(&client_, MemoryConfigClientRequest::READ_PART, + NodeHandle(node_->node_id()), SPACE, spacerept.before().offset(), 13); + ASSERT_EQ(0, b->data()->resultCode); + EXPECT_STREQ("hello", b->data()->payload.c_str()); + EXPECT_EQ(13u, b->data()->payload.size()); + EXPECT_EQ(0u, s_.lastRepeat_); + + b = invoke_flow(&client_, MemoryConfigClientRequest::READ_PART, + NodeHandle(node_->node_id()), SPACE, spacerept.before().offset() - 2, + 5); + ASSERT_EQ(0, b->data()->resultCode); + EXPECT_EQ(string("\0\0hel", 5), b->data()->payload); + EXPECT_EQ(0u, s_.lastRepeat_); +} + +// Looks for a field in the first repetition of the group. +TEST_F(ReptSpaceTest, first_repeat) +{ + arg1 = "world"; + auto b = invoke_flow(&client_, MemoryConfigClientRequest::READ_PART, + NodeHandle(node_->node_id()), SPACE, + spacerept.grp().entry<0>().first().offset(), 13); + ASSERT_EQ(0, b->data()->resultCode); + EXPECT_STREQ("world", b->data()->payload.c_str()); + EXPECT_EQ(13u, b->data()->payload.size()); + EXPECT_EQ(0u, s_.lastRepeat_); + + // Start offset within the group. + b = invoke_flow(&client_, MemoryConfigClientRequest::READ_PART, + NodeHandle(node_->node_id()), SPACE, + spacerept.grp().entry<0>().first().offset() - 2, 5); + ASSERT_EQ(0, b->data()->resultCode); + EXPECT_EQ(string("\0\0wor", 5), b->data()->payload); + EXPECT_EQ(0u, s_.lastRepeat_); + + // Start offset _before_ the group. + b = invoke_flow(&client_, MemoryConfigClientRequest::READ_PART, + NodeHandle(node_->node_id()), SPACE, + spacerept.grp().entry<0>().first().offset() - 7, 10); + ASSERT_EQ(0, b->data()->resultCode); + EXPECT_EQ(string("\0\0\0\0\0\0\0wor", 10), b->data()->payload); + EXPECT_EQ(0u, s_.lastRepeat_); + + arg2 = "ahoi"; + // Second field, exact match + b = invoke_flow(&client_, MemoryConfigClientRequest::READ_PART, + NodeHandle(node_->node_id()), SPACE, + spacerept.grp().entry<0>().second().offset(), 13); + ASSERT_EQ(0, b->data()->resultCode); + EXPECT_STREQ("ahoi", b->data()->payload.c_str()); + EXPECT_EQ(13u, b->data()->payload.size()); + EXPECT_EQ(0u, s_.lastRepeat_); + + // Second field, before match + b = invoke_flow(&client_, MemoryConfigClientRequest::READ_PART, + NodeHandle(node_->node_id()), SPACE, + spacerept.grp().entry<0>().second().offset() - 2, 5); + ASSERT_EQ(0, b->data()->resultCode); + EXPECT_EQ(string("\0\0aho", 5), b->data()->payload); + EXPECT_EQ(0u, s_.lastRepeat_); +} + +// Looks for a field in the first repetition of the group. +TEST_F(ReptSpaceTest, mid_repeat) +{ + arg1 = "world"; + auto b = invoke_flow(&client_, MemoryConfigClientRequest::READ_PART, + NodeHandle(node_->node_id()), SPACE, + spacerept.grp().entry<2>().first().offset(), 13); + ASSERT_EQ(0, b->data()->resultCode); + EXPECT_STREQ("world", b->data()->payload.c_str()); + EXPECT_EQ(13u, b->data()->payload.size()); + EXPECT_EQ(2u, s_.lastRepeat_); + + // Start offset within the group. + b = invoke_flow(&client_, MemoryConfigClientRequest::READ_PART, + NodeHandle(node_->node_id()), SPACE, + spacerept.grp().entry<2>().first().offset() - 2, 5); + ASSERT_EQ(0, b->data()->resultCode); + EXPECT_EQ(string("\0\0wor", 5), b->data()->payload); + EXPECT_EQ(2u, s_.lastRepeat_); + + // Start offset in the previous group repeat. + b = invoke_flow(&client_, MemoryConfigClientRequest::READ_PART, + NodeHandle(node_->node_id()), SPACE, + spacerept.grp().entry<2>().first().offset() - 7, 10); + ASSERT_EQ(0, b->data()->resultCode); + EXPECT_EQ(string("\0\0\0\0\0\0\0wor", 10), b->data()->payload); + EXPECT_EQ(2u, s_.lastRepeat_); + + arg2 = "ahoi"; + // Second field, exact match + b = invoke_flow(&client_, MemoryConfigClientRequest::READ_PART, + NodeHandle(node_->node_id()), SPACE, + spacerept.grp().entry<2>().second().offset(), 13); + ASSERT_EQ(0, b->data()->resultCode); + EXPECT_STREQ("ahoi", b->data()->payload.c_str()); + EXPECT_EQ(13u, b->data()->payload.size()); + EXPECT_EQ(2u, s_.lastRepeat_); + + // Second field, before match + b = invoke_flow(&client_, MemoryConfigClientRequest::READ_PART, + NodeHandle(node_->node_id()), SPACE, + spacerept.grp().entry<2>().second().offset() - 2, 5); + ASSERT_EQ(0, b->data()->resultCode); + EXPECT_EQ(string("\0\0aho", 5), b->data()->payload); + EXPECT_EQ(2u, s_.lastRepeat_); +} + } // namespace openlcb diff --git a/src/openlcb/VirtualMemorySpace.hxx b/src/openlcb/VirtualMemorySpace.hxx index 0ba99e4e1..c31a07b9d 100644 --- a/src/openlcb/VirtualMemorySpace.hxx +++ b/src/openlcb/VirtualMemorySpace.hxx @@ -37,6 +37,7 @@ #define _OPENLCB_VIRTUALMEMORYSPACE_HXX #include "openlcb/ConfigEntry.hxx" +#include "openlcb/ConfigRepresentation.hxx" #include "openlcb/MemoryConfig.hxx" #include "utils/SortedListMap.hxx" @@ -226,6 +227,17 @@ protected: register_element(entry.offset(), SIZE, read_f, write_f); } + template + void register_repeat(const RepeatedGroup &group) + { + RepeatElement re; + re.start_ = group.offset(); + re.end_ = group.end_offset(); + re.repeatSize_ = Group::size(); + HASSERT(re.repeatSize_ * N == re.end_ - re.start_); + repeats_.insert(std::move(re)); + } + /// Bounds for valid addresses. address_t minAddress_ = 0xFFFFFFFFu; /// Bounds for valid addresses. A read of length 1 from this address @@ -257,7 +269,7 @@ private: ReadFunction readImpl_; }; - struct Comparator + struct DataComparator { /// Sorting operator by address. bool operator()(const DataElement &a, const DataElement &b) const @@ -269,6 +281,37 @@ private: { return a < b.address_; } + /// Sorting operator by address. + bool operator()(const DataElement &a, unsigned b) const + { + return a.address_ < b; + } + }; + + /// Represents a repeated group. + struct RepeatElement + { + /// Offset of the repeated group (first repeat). + uint32_t start_; + /// Address bytes per repeat. + uint32_t repeatSize_; + /// Address byte after the last repeat. + uint32_t end_; + }; + + /// Comparator operator for RepeatElements. + struct RepeatComparator + { + /// Sorting operator by end address. + bool operator()(const RepeatElement &a, const RepeatElement &b) const + { + return a.end_ < b.end_; + } + /// Sorting operator by end address against a lookup key. + bool operator()(uint32_t a, const RepeatElement &b) const + { + return a < b.end_; + } }; /// Look up the first matching data element given an address in the virtual @@ -291,35 +334,88 @@ private: { *repeat = 0; *ptr = nullptr; - auto it = elements_.upper_bound(address); - if (it != elements_.begin()) + bool in_repeat = false; + address_t original_address = address; + ElementsType::iterator b = elements_.begin(); + ElementsType::iterator e = elements_.end(); + // Align in the known repetitions first. + auto rit = repeats_.upper_bound(address); + if (rit == repeats_.end()) + { + // not a repeat. + // LOG(INFO, "not a repeat"); + } + else { - auto pit = it - 1; - // now: pit->address_ <= address - if (pit->address_ + pit->size_ > address) + // LOG(INFO, "have rept it"); + if (rit->start_ <= address && rit->end_ > address) { - // found overlap - *ptr = &*pit; - return (ssize_t)pit->address_ - - (ssize_t)address; // may be negative! + // LOG(INFO, "in rept"); + // we are in the repeat. + unsigned cnt = (address - rit->start_) / rit->repeatSize_; + *repeat = cnt; + // re-aligns address to the first repetition. + address -= cnt * rit->repeatSize_; + in_repeat = true; + b = elements_.lower_bound(rit->start_); + e = elements_.lower_bound(rit->start_ + rit->repeatSize_); } - // else: no overlap, look at the next item } - // now: it->address_ > address - if (address + len > it->address_) + + for (int is_repeat = 0; is_repeat <= 1; ++is_repeat) { - // found overlap, but some data needs to be discarded. - *ptr = &*it; - return it->address_ - address; + auto it = std::upper_bound(b, e, address, DataComparator()); + if (it != elements_.begin()) + { + auto pit = it - 1; + // now: pit->address_ <= address + if (pit->address_ + pit->size_ > address) + { + // found overlap + *ptr = &*pit; + return (ssize_t)pit->address_ - + (ssize_t)address; // may be negative! + } + // else: no overlap, look at the next item + } + // now: it->address_ > address + if (address + len > it->address_) + { + // found overlap, but some data needs to be discarded. + *ptr = &*it; + return it->address_ - address; + } + + if (in_repeat) + { + // We might be too close to the end of a repetition, we will + // try with the next repeat instead. + address -= rit->repeatSize_; + *repeat += 1; + if (original_address + rit->repeatSize_ >= rit->end_) + { + // We ran out of repeats. Look at the range beyond the + // group instead. + b = elements_.lower_bound(rit->end_); + e = elements_.end(); + *repeat = 0; + } + } + else + { + break; + } } - /// @todo try repeated fields here first. // now: no overlap either before or after. return len; } + typedef SortedListSet ElementsType; + /// Stores all the registered variables. + ElementsType elements_; /// Stores all the registered variables. - SortedListSet elements_; + SortedListSet repeats_; /// Helper object in the function calls. BarrierNotifiable bn_; }; // class VirtualMemorySpace From 6ed7e6e1f7b32413ef52a2aaaee254230767ecbd Mon Sep 17 00:00:00 2001 From: Balazs Racz Date: Tue, 18 Aug 2020 21:57:03 +0200 Subject: [PATCH 09/11] Adds more comments. --- src/openlcb/VirtualMemorySpace.hxx | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/openlcb/VirtualMemorySpace.hxx b/src/openlcb/VirtualMemorySpace.hxx index c31a07b9d..8d15ab44e 100644 --- a/src/openlcb/VirtualMemorySpace.hxx +++ b/src/openlcb/VirtualMemorySpace.hxx @@ -227,6 +227,14 @@ protected: register_element(entry.offset(), SIZE, read_f, write_f); } + /// Registers a repeated group. Calling this function means that the + /// virtual memory space of the group will be looped onto the first + /// repetition. The correct usage is to register the elements of the first + /// repetition, then register the repetition itself using this call. Nested + /// repetitions are not supported (either the outer or the inner repetition + /// needs to be unrolled and registered for each repeat there). + /// @param group is the repeated group instance. Will take the start + /// offset, repeat count and repeat size from it. template void register_repeat(const RepeatedGroup &group) { @@ -269,6 +277,7 @@ private: ReadFunction readImpl_; }; + /// STL-compatible comparator function for sorting DataElements. struct DataComparator { /// Sorting operator by address. @@ -299,7 +308,8 @@ private: uint32_t end_; }; - /// Comparator operator for RepeatElements. + /// STL-compatible comparator function for sorting RepeatElements. Sorts + /// repeats by the end_ as the key. struct RepeatComparator { /// Sorting operator by end address. @@ -411,6 +421,7 @@ private: return len; } + /// Container type for storing the data elements. typedef SortedListSet ElementsType; /// Stores all the registered variables. ElementsType elements_; From 31f135be0bac055ef18dbb03920d2098976764bc Mon Sep 17 00:00:00 2001 From: Balazs Racz Date: Thu, 27 Aug 2020 23:19:17 +0200 Subject: [PATCH 10/11] removes info logs. --- src/openlcb/VirtualMemorySpace.hxx | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/openlcb/VirtualMemorySpace.hxx b/src/openlcb/VirtualMemorySpace.hxx index 8d15ab44e..f1948e9f3 100644 --- a/src/openlcb/VirtualMemorySpace.hxx +++ b/src/openlcb/VirtualMemorySpace.hxx @@ -353,14 +353,11 @@ private: if (rit == repeats_.end()) { // not a repeat. - // LOG(INFO, "not a repeat"); } else { - // LOG(INFO, "have rept it"); if (rit->start_ <= address && rit->end_ > address) { - // LOG(INFO, "in rept"); // we are in the repeat. unsigned cnt = (address - rit->start_) / rit->repeatSize_; *repeat = cnt; From 9aa55770fcf897357e51d4d3f3bde43e4188b3c2 Mon Sep 17 00:00:00 2001 From: Balazs Racz Date: Thu, 27 Aug 2020 23:21:01 +0200 Subject: [PATCH 11/11] fully parenthesizes complex expressions. --- src/openlcb/VirtualMemorySpace.hxx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/openlcb/VirtualMemorySpace.hxx b/src/openlcb/VirtualMemorySpace.hxx index 0ba99e4e1..fc6e48f30 100644 --- a/src/openlcb/VirtualMemorySpace.hxx +++ b/src/openlcb/VirtualMemorySpace.hxx @@ -79,7 +79,7 @@ public: size_t write(address_t destination, const uint8_t *data, size_t len, errorcode_t *error, Notifiable *again) override { - if (destination > maxAddress_ || destination + len <= minAddress_) + if ((destination > maxAddress_) || ((destination + len) <= minAddress_)) { *error = MemoryConfigDefs::ERROR_OUT_OF_BOUNDS; return 0; @@ -129,7 +129,7 @@ public: size_t read(address_t source, uint8_t *dst, size_t len, errorcode_t *error, Notifiable *again) override { - if (source > maxAddress_ || source + len <= minAddress_) + if ((source > maxAddress_) || ((source + len) <= minAddress_)) { *error = MemoryConfigDefs::ERROR_OUT_OF_BOUNDS; return 0;