Skip to content

Commit

Permalink
Completes VirtualMemorySpace implementation (#428)
Browse files Browse the repository at this point in the history 
This PR fixes bugs and implements missing features in the VirtualMemorySpace:
- A bug caused returning more data read than the end of space when hitting the last variable in the last repeat of a group.
- Correctly trims reads and writes at the end of the space.
- Implements reading from the middle of a variable.
- Implements writing to a middle of a variable.
- Updates unittests with new test cases.

===

* Fixes bug where container.end() was dereferenced.

* Limits reads and writes to the maxAddress_.

* Adds tests to read at eof in repeated and non-repeated groups.

* Add debug statements.

* Implements reading and writing in the middle of a variable.

* Fix whitespace

* downgrade log statements.

* Adds async read-middle and r-m-w tests.
  • Loading branch information
@balazsracz
balazsracz authored Sep 6, 2020
1 parent c49aba6 commit 6013d0e
Show file tree
Hide file tree
Showing 2 changed files with 266 additions and 16 deletions.
196 changes: 196 additions & 0 deletions src/openlcb/VirtualMemorySpace.cxxtest
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,27 @@ TEST_F(TestSpaceTest, read_early)
EXPECT_EQ(3u, b->data()->payload.size());
}

/// Test reading a variable from an imprecise offset (too late -- middle of
/// variable).
TEST_F(TestSpaceTest, read_middle)
{
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("llo\0\0\0\0\0\0\0", 10);
EXPECT_EQ(exp, b->data()->payload);
EXPECT_EQ(10u, b->data()->payload.size());

arg2 = "abcdefghij";
b = invoke_flow(&client_, MemoryConfigClientRequest::READ_PART,
NodeHandle(node_->node_id()), SPACE, arg2_ofs + 2, 3);
ASSERT_EQ(0, b->data()->resultCode);
string exp2("cde", 3);
EXPECT_EQ(exp2, b->data()->payload);
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)
{
Expand All @@ -201,6 +222,25 @@ TEST_F(TestSpaceTest, read_hole)
EXPECT_EQ(string(), b->data()->payload);
}

/// Test reading beyond eof.
TEST_F(TestSpaceTest, read_eof)
{
unsigned reg_last = cfg.second().offset();
auto b = invoke_flow(&client_, MemoryConfigClientRequest::READ_PART,
NodeHandle(node_->node_id()), SPACE, reg_last, 50);
ASSERT_EQ(0, b->data()->resultCode);
EXPECT_EQ(20u, b->data()->payload.size());
b = invoke_flow(&client_, MemoryConfigClientRequest::READ_PART,
NodeHandle(node_->node_id()), SPACE, reg_last + 20, 50);
ASSERT_EQ(0, b->data()->resultCode);
EXPECT_EQ(0u, b->data()->payload.size());

b = invoke_flow(&client_, MemoryConfigClientRequest::READ_PART,
NodeHandle(node_->node_id()), SPACE, reg_last + 23, 50);
ASSERT_EQ(0, b->data()->resultCode);
EXPECT_EQ(0u, b->data()->payload.size());
}

/// Basic tests writing variables from the exact offset but not including
/// partial writes.
TEST_F(TestSpaceTest, write_payload)
Expand Down Expand Up @@ -234,6 +274,52 @@ TEST_F(TestSpaceTest, write_early)
EXPECT_EQ(4u, arg2.size());
}

/// Test writing into to a offset that is in the middle of a variable.
TEST_F(TestSpaceTest, write_middle)
{
arg1 = "hellllo";
auto b = invoke_flow(&client_, MemoryConfigClientRequest::WRITE,
NodeHandle(node_->node_id()), SPACE, arg1_ofs + 3, "xy");
ASSERT_EQ(0, b->data()->resultCode);
EXPECT_EQ(string("helxylo\0\0\0\0\0\0", 13), arg1);

// Writes to middle then beyond the end of the variable.
string payload(19, 'a');
payload += "bbbbbbbbb";
arg2 = "0123456789i123456789";
b = invoke_flow(&client_, MemoryConfigClientRequest::WRITE,
NodeHandle(node_->node_id()), SPACE, arg1_ofs + 3, payload);
ASSERT_EQ(0, b->data()->resultCode);
EXPECT_EQ("helaaaaaaaaaa", arg1);
EXPECT_EQ(13u, arg1.size());
EXPECT_EQ("abbbbbbbbb", arg2);
EXPECT_EQ(10u, arg2.size());

// Writes a string in multiple datagrams.
payload = "0123456789abcdef";
payload.push_back(0);

b = invoke_flow(&client_, MemoryConfigClientRequest::WRITE,
NodeHandle(node_->node_id()), SPACE, arg2_ofs, payload.substr(0, 10));
ASSERT_EQ(0, b->data()->resultCode);
EXPECT_EQ("0123456789", arg2);

b = invoke_flow(&client_, MemoryConfigClientRequest::WRITE,
NodeHandle(node_->node_id()), SPACE, arg2_ofs + 10,
payload.substr(10, 3));
ASSERT_EQ(0, b->data()->resultCode);
EXPECT_STREQ("0123456789abc", arg2.c_str());
b = invoke_flow(&client_, MemoryConfigClientRequest::WRITE,
NodeHandle(node_->node_id()), SPACE, arg2_ofs + 13,
payload.substr(13, 2));
ASSERT_EQ(0, b->data()->resultCode);
EXPECT_STREQ("0123456789abcde", arg2.c_str());
b = invoke_flow(&client_, MemoryConfigClientRequest::WRITE,
NodeHandle(node_->node_id()), SPACE, arg2_ofs + 15, payload.substr(15));
ASSERT_EQ(0, b->data()->resultCode);
EXPECT_STREQ("0123456789abcdef", arg2.c_str());
}

/// Test writing a variable to a offset that is not covered at all.
TEST_F(TestSpaceTest, write_hole)
{
Expand Down Expand Up @@ -453,6 +539,27 @@ TEST_F(TestSpaceAsyncTest, read_payload_async)
EXPECT_EQ(20u, b->data()->payload.size());
}

/// Test reading a variable from an imprecise offset (too late -- middle of
/// variable).
TEST_F(TestSpaceAsyncTest, read_middle)
{
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("llo\0\0\0\0\0\0\0", 10);
EXPECT_EQ(exp, b->data()->payload);
EXPECT_EQ(10u, b->data()->payload.size());

arg2 = "abcdefghij";
b = invoke_flow(&client_, MemoryConfigClientRequest::READ_PART,
NodeHandle(node_->node_id()), SPACE, arg2_ofs + 2, 3);
ASSERT_EQ(0, b->data()->resultCode);
string exp2("cde", 3);
EXPECT_EQ(exp2, b->data()->payload);
EXPECT_EQ(3u, b->data()->payload.size());
}

/// Basic tests writing variables from the exact offset but not including
/// partial writes.
TEST_F(TestSpaceAsyncTest, write_payload_async)
Expand All @@ -470,6 +577,52 @@ TEST_F(TestSpaceAsyncTest, write_payload_async)
EXPECT_EQ(5u, arg2.size());
}

/// Test writing into to a offset that is in the middle of a variable.
TEST_F(TestSpaceAsyncTest, write_middle)
{
arg1 = "hellllo";
auto b = invoke_flow(&client_, MemoryConfigClientRequest::WRITE,
NodeHandle(node_->node_id()), SPACE, arg1_ofs + 3, "xy");
ASSERT_EQ(0, b->data()->resultCode);
EXPECT_EQ(string("helxylo\0\0\0\0\0\0", 13), arg1);

// Writes to middle then beyond the end of the variable.
string payload(19, 'a');
payload += "bbbbbbbbb";
arg2 = "0123456789i123456789";
b = invoke_flow(&client_, MemoryConfigClientRequest::WRITE,
NodeHandle(node_->node_id()), SPACE, arg1_ofs + 3, payload);
ASSERT_EQ(0, b->data()->resultCode);
EXPECT_EQ("helaaaaaaaaaa", arg1);
EXPECT_EQ(13u, arg1.size());
EXPECT_EQ("abbbbbbbbb", arg2);
EXPECT_EQ(10u, arg2.size());

// Writes a string in multiple datagrams.
payload = "0123456789abcdef";
payload.push_back(0);

b = invoke_flow(&client_, MemoryConfigClientRequest::WRITE,
NodeHandle(node_->node_id()), SPACE, arg2_ofs, payload.substr(0, 10));
ASSERT_EQ(0, b->data()->resultCode);
EXPECT_EQ("0123456789", arg2);

b = invoke_flow(&client_, MemoryConfigClientRequest::WRITE,
NodeHandle(node_->node_id()), SPACE, arg2_ofs + 10,
payload.substr(10, 3));
ASSERT_EQ(0, b->data()->resultCode);
EXPECT_STREQ("0123456789abc", arg2.c_str());
b = invoke_flow(&client_, MemoryConfigClientRequest::WRITE,
NodeHandle(node_->node_id()), SPACE, arg2_ofs + 13,
payload.substr(13, 2));
ASSERT_EQ(0, b->data()->resultCode);
EXPECT_STREQ("0123456789abcde", arg2.c_str());
b = invoke_flow(&client_, MemoryConfigClientRequest::WRITE,
NodeHandle(node_->node_id()), SPACE, arg2_ofs + 15, payload.substr(15));
ASSERT_EQ(0, b->data()->resultCode);
EXPECT_STREQ("0123456789abcdef", arg2.c_str());
}

/// Tests reading and writing numeric variables with endianness.
TEST_F(TestSpaceAsyncTest, rw_numeric_async)
{
Expand Down Expand Up @@ -565,6 +718,7 @@ using GroupRept = RepeatedGroup<ExampleMemorySpace, 3>;
CDI_GROUP_ENTRY(grp, GroupRept);
CDI_GROUP_ENTRY(skipped3, EmptyGroup<8>);
CDI_GROUP_ENTRY(after, StringConfigEntry<20>);
CDI_GROUP_ENTRY(grp2, GroupRept);
CDI_GROUP_END();

RepeatMemoryDef spacerept(22);
Expand All @@ -585,6 +739,9 @@ public:
register_string(
spacerept.after(), string_reader(&after_), string_writer(&after_));
register_repeat(spacerept.grp());
register_string(spacerept.grp2().entry<0>().second(),
string_reader(&arg2), string_writer(&arg2));
register_repeat(spacerept.grp2());
}

/// Creates a ReaderFunction that just returns a string from a given
Expand Down Expand Up @@ -760,4 +917,43 @@ TEST_F(ReptSpaceTest, mid_repeat)
EXPECT_EQ(2u, s_.lastRepeat_);
}

/// Test reading beyond eof. The end of the space there is a repeated group
/// where the registry entries do not cover all bytes. This is a difficult
/// cornercase and we test that all bytes until the end of the repetition can
/// be read but not beyond.
TEST_F(ReptSpaceTest, read_eof)
{
unsigned reg_last = spacerept.grp2().entry<2>().second().offset();
// EOF is 28 bytes away from here.
EXPECT_EQ(reg_last + 27, s_.max_address());
auto b = invoke_flow(&client_, MemoryConfigClientRequest::READ_PART,
NodeHandle(node_->node_id()), SPACE, reg_last, 50);
ASSERT_EQ(0, b->data()->resultCode);
EXPECT_EQ(28u, b->data()->payload.size());
b = invoke_flow(&client_, MemoryConfigClientRequest::READ_PART,
NodeHandle(node_->node_id()), SPACE, reg_last + 20, 50);
ASSERT_EQ(0, b->data()->resultCode);
EXPECT_EQ(8u, b->data()->payload.size());

b = invoke_flow(&client_, MemoryConfigClientRequest::READ_PART,
NodeHandle(node_->node_id()), SPACE, reg_last + 23, 50);
ASSERT_EQ(0, b->data()->resultCode);
EXPECT_EQ(5u, b->data()->payload.size());

b = invoke_flow(&client_, MemoryConfigClientRequest::READ_PART,
NodeHandle(node_->node_id()), SPACE, reg_last + 27, 50);
ASSERT_EQ(0, b->data()->resultCode);
EXPECT_EQ(1u, b->data()->payload.size());

b = invoke_flow(&client_, MemoryConfigClientRequest::READ_PART,
NodeHandle(node_->node_id()), SPACE, reg_last + 28, 50);
ASSERT_EQ(0, b->data()->resultCode);
EXPECT_EQ(0u, b->data()->payload.size());

b = invoke_flow(&client_, MemoryConfigClientRequest::READ_PART,
NodeHandle(node_->node_id()), SPACE, reg_last + 29, 50);
ASSERT_EQ(0, b->data()->resultCode);
EXPECT_EQ(0u, b->data()->payload.size());
}

} // namespace openlcb
Loading

0 comments on commit 6013d0e

Please sign in to comment.