Skip to content

Commit

Permalink
Apply CPPLint to CRT Tests (apache#8844)
Browse files Browse the repository at this point in the history
This one was a bit trickier as there was more usage of dynamic arrays and less safe casts. I've tried to minimise the changes to just those required to passing linting.
  • Loading branch information
Mousius authored and Andrew Zhao Luo committed Sep 1, 2021
1 parent c757bd2 commit 39ca27f
Show file tree
Hide file tree
Showing 7 changed files with 39 additions and 31 deletions.
4 changes: 3 additions & 1 deletion tests/crt/buffer_write_stream.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
#include <tvm/runtime/crt/rpc_common/frame_buffer.h>
#include <tvm/runtime/crt/rpc_common/write_stream.h>

#include <string>

using ::tvm::runtime::micro_rpc::FrameBuffer;
using ::tvm::runtime::micro_rpc::WriteStream;

Expand Down Expand Up @@ -51,7 +53,7 @@ class BufferWriteStream : public WriteStream {

std::string BufferContents() { return std::string((const char*)buffer_data_, buffer_.Size()); }

static constexpr unsigned int capacity() { return N; };
static constexpr unsigned int capacity() { return N; }

private:
bool packet_done_{false};
Expand Down
18 changes: 10 additions & 8 deletions tests/crt/framing_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -150,23 +150,25 @@ TEST_F(UnframerTest, PacketTooLong) {
unframer_.Write(packet_length_bytes, sizeof(packet_length), &bytes_consumed));
EXPECT_EQ(sizeof(packet_length), bytes_consumed);

uint8_t long_payload[decltype(write_stream_)::capacity() + 1];
for (size_t i = 0; i < sizeof(long_payload); i++) {
unsigned int long_payload_len = decltype(write_stream_)::capacity() + 1;
auto long_payload = std::make_unique<uint8_t[]>(long_payload_len);
for (size_t i = 0; i < long_payload_len; i++) {
long_payload[i] = i & 0xff;
if (long_payload[i] == uint8_t(Escape::kEscapeStart)) {
long_payload[i] = 0;
}
}
crc = tvm::runtime::micro_rpc::crc16_compute(long_payload, sizeof(long_payload), &crc);
crc = tvm::runtime::micro_rpc::crc16_compute(long_payload.get(), long_payload_len, &crc);
EXPECT_EQ(kTvmErrorWriteStreamShortWrite,
unframer_.Write(long_payload, sizeof(long_payload), &bytes_consumed));
unframer_.Write(long_payload.get(), long_payload_len, &bytes_consumed));
EXPECT_EQ(write_stream_.capacity(), bytes_consumed);

EXPECT_EQ(kTvmErrorNoError, unframer_.Write((uint8_t*)&crc, sizeof(crc), &bytes_consumed));
EXPECT_EQ(kTvmErrorNoError,
unframer_.Write(reinterpret_cast<uint8_t*>(&crc), sizeof(crc), &bytes_consumed));
EXPECT_EQ(2UL, bytes_consumed); // 2, because framer is now in kFindPacketStart.
EXPECT_FALSE(write_stream_.packet_done());
EXPECT_FALSE(write_stream_.is_valid());
EXPECT_EQ(std::string((char*)long_payload, write_stream_.capacity()),
EXPECT_EQ(std::string(reinterpret_cast<char*>(long_payload.get()), write_stream_.capacity()),
write_stream_.BufferContents());

// Writing a smaller packet directly afterward should work.
Expand All @@ -177,7 +179,7 @@ TEST_F(UnframerTest, PacketTooLong) {
EXPECT_TRUE(write_stream_.packet_done());
EXPECT_TRUE(write_stream_.is_valid());
EXPECT_EQ(kPacket1.payload, write_stream_.BufferContents());
};
}

class UnframerTestParameterized : public UnframerTest,
public ::testing::WithParamInterface<const TestPacket*> {};
Expand Down Expand Up @@ -297,4 +299,4 @@ TEST_P(UnframerTestParameterized, TestArbitraryPacketReset) {
#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
INSTANTIATE_TEST_CASE_P(UnframerTests, UnframerTestParameterized,
::testing::ValuesIn(TestPacket::instances));
#pragma GCC diagnostic pop
#pragma GCC diagnostic pop
35 changes: 19 additions & 16 deletions tests/crt/func_registry_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -178,45 +178,47 @@ TEST(MutableFuncRegistry, Create) {

for (unsigned int rem = 0; rem < kTvmAverageFuncEntrySizeBytes; rem++) {
// test_function name will be used to test overfilling.
char test_function_name[kTvmAverageFunctionNameStrlenBytes + 2 + rem];
auto test_function_name =
std::make_unique<char[]>(kTvmAverageFunctionNameStrlenBytes + 2 + rem);
TVMMutableFuncRegistry reg;
memset(mem_buffer, 0, sizeof(mem_buffer));
EXPECT_EQ(kTvmErrorNoError, TVMMutableFuncRegistry_Create(
&reg, mem_buffer, kTvmAverageFuncEntrySizeBytes * 2 + rem));

snprintf_truncate(test_function_name, kTvmAverageFunctionNameStrlenBytes + 1,
snprintf_truncate(test_function_name.get(), kTvmAverageFunctionNameStrlenBytes + 1,
function_name_chars);

// Add function #1, and verify it can be retrieved.
EXPECT_EQ(kTvmErrorNoError,
TVMMutableFuncRegistry_Set(&reg, test_function_name, TestFunctionHandle(0x01), 0));
EXPECT_EQ(kTvmErrorNoError, TVMMutableFuncRegistry_Set(&reg, test_function_name.get(),
TestFunctionHandle(0x01), 0));

tvm_function_index_t func_index = 100;
EXPECT_EQ(kTvmErrorNoError,
TVMFuncRegistry_Lookup(&reg.registry, test_function_name, &func_index));
TVMFuncRegistry_Lookup(&reg.registry, test_function_name.get(), &func_index));
EXPECT_EQ(func_index, 0);

TVMBackendPackedCFunc func = NULL;
EXPECT_EQ(kTvmErrorNoError, TVMFuncRegistry_GetByIndex(&reg.registry, func_index, &func));
EXPECT_EQ(func, TestFunctionHandle(0x01));

// Ensure that overfilling `names` by 1 char is not allowed.
snprintf_truncate(test_function_name, kTvmAverageFunctionNameStrlenBytes + rem + 2,
snprintf_truncate(test_function_name.get(), kTvmAverageFunctionNameStrlenBytes + rem + 2,
function_name_chars + 1);

EXPECT_EQ(kTvmErrorFunctionRegistryFull,
TVMMutableFuncRegistry_Set(&reg, test_function_name, TestFunctionHandle(0x02), 0));
EXPECT_EQ(
kTvmErrorFunctionRegistryFull,
TVMMutableFuncRegistry_Set(&reg, test_function_name.get(), TestFunctionHandle(0x02), 0));
EXPECT_EQ(kTvmErrorFunctionNameNotFound,
TVMFuncRegistry_Lookup(&reg.registry, test_function_name, &func_index));
TVMFuncRegistry_Lookup(&reg.registry, test_function_name.get(), &func_index));

// Add function #2, with intentionally short (by 2 char) name. Verify it can be retrieved.
snprintf_truncate(test_function_name, kTvmAverageFunctionNameStrlenBytes - 2 + 1,
snprintf_truncate(test_function_name.get(), kTvmAverageFunctionNameStrlenBytes - 2 + 1,
function_name_chars + 1);
EXPECT_EQ(kTvmErrorNoError,
TVMMutableFuncRegistry_Set(&reg, test_function_name, TestFunctionHandle(0x02), 0));
EXPECT_EQ(kTvmErrorNoError, TVMMutableFuncRegistry_Set(&reg, test_function_name.get(),
TestFunctionHandle(0x02), 0));

EXPECT_EQ(kTvmErrorNoError,
TVMFuncRegistry_Lookup(&reg.registry, test_function_name, &func_index));
TVMFuncRegistry_Lookup(&reg.registry, test_function_name.get(), &func_index));
EXPECT_EQ(func_index, 1);

func = NULL;
Expand All @@ -226,7 +228,8 @@ TEST(MutableFuncRegistry, Create) {
// Try adding another function, which should fail due to lack of function pointers.
test_function_name[0] = 'a';
test_function_name[1] = 0;
EXPECT_EQ(kTvmErrorFunctionRegistryFull,
TVMMutableFuncRegistry_Set(&reg, test_function_name, TestFunctionHandle(0x03), 0));
EXPECT_EQ(
kTvmErrorFunctionRegistryFull,
TVMMutableFuncRegistry_Set(&reg, test_function_name.get(), TestFunctionHandle(0x03), 0));
}
}
}
5 changes: 3 additions & 2 deletions tests/crt/page_allocator_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,10 @@ class PageAllocatorTest : public ::testing::Test {
protected:
void SetUp() override {
memset(raw_memory_pool, 0, sizeof(raw_memory_pool));
memory_pool = (uint8_t*)(ROUND_UP(((uintptr_t)raw_memory_pool), (1 << kPageSizeBytesLog)));
memory_pool = reinterpret_cast<uint8_t*>(
ROUND_UP(((uintptr_t)raw_memory_pool), (1 << kPageSizeBytesLog)));
PageMemoryManagerCreate(&interface, memory_pool, kMemoryPoolSizeBytes, kPageSizeBytesLog);
mgr = (MemoryManager*)interface;
mgr = reinterpret_cast<MemoryManager*>(interface);
ASSERT_EQ(kNumUsablePages, mgr->ptable.max_pages);
dev_ = {kDLCPU, 0};
}
Expand Down
4 changes: 2 additions & 2 deletions tests/crt/session_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class ReceivedMessage {

class TestSession {
public:
TestSession(uint8_t initial_nonce)
explicit TestSession(uint8_t initial_nonce)
: framer{&framer_write_stream},
receive_buffer{receive_buffer_array, sizeof(receive_buffer_array)},
sess{&framer, &receive_buffer, TestSessionMessageReceivedThunk, this},
Expand Down Expand Up @@ -247,4 +247,4 @@ TEST_F(SessionTest, DoubleStart) {
bob_.ClearBuffers();
alice_.WriteTo(&bob_);
EXPECT_TRUE(bob_.sess.IsEstablished());
}
}
2 changes: 1 addition & 1 deletion tests/crt/stack_allocator_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -198,4 +198,4 @@ TEST(StackAllocatorTest, InitialMemoryMisAlignment) {

ASSERT_EQ(tvm_runtime_workspace.next_alloc, &model_memory_ptr[alignment_offset]);
ASSERT_EQ(tvm_runtime_workspace.workspace_size, sizeof(model_memory) - offset - alignment_offset);
}
}
2 changes: 1 addition & 1 deletion tests/lint/cpplint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,4 @@ python3 3rdparty/dmlc-core/scripts/lint.py vta cpp vta/include vta/src
python3 3rdparty/dmlc-core/scripts/lint.py tvm cpp \
include src \
examples/extension/src examples/graph_executor/src \
tests/cpp
tests/cpp tests/crt

0 comments on commit 39ca27f

Please sign in to comment.