From: Andrei Pavel Date: Thu, 21 Mar 2024 08:48:18 +0000 (+0200) Subject: [#3210] address review X-Git-Tag: Kea-2.5.7~10 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=fc59957aefa583a713ad037802ba3990f3896c14;p=thirdparty%2Fkea.git [#3210] address review --- diff --git a/src/lib/util/filesystem.cc b/src/lib/util/filesystem.cc index c23ee5a335..5571231562 100644 --- a/src/lib/util/filesystem.cc +++ b/src/lib/util/filesystem.cc @@ -42,7 +42,7 @@ getContent(string const& file_name) { } string content; file >> content; - return content; + return (content); } bool @@ -148,7 +148,7 @@ Path::replaceExtension(string const& replacement) { extension_ = trimmed_replacement.substr(last_dot); } } - return *this; + return (*this); } Path& @@ -161,7 +161,7 @@ Path::replaceParentPath(string const& replacement) { } else { parent_path_ = trimmed_replacement + '/'; } - return *this; + return (*this); } } // namespace file diff --git a/src/lib/util/filesystem.h b/src/lib/util/filesystem.h index edf16c4ff2..1f20003f39 100644 --- a/src/lib/util/filesystem.h +++ b/src/lib/util/filesystem.h @@ -73,7 +73,7 @@ struct Path { /// /// Counterpart for std::filesystem::path::stem. /// - /// \return the base name of the file without the extension. + /// \return the base name of current path without the extension. std::string stem() const; /// \brief Get the extension of the file. @@ -83,9 +83,9 @@ struct Path { /// \return extension of current path. std::string extension() const; - /// \brief Get the extension of the file. + /// \brief Get the name of the file, extension included. /// - /// Counterpart for std::filesystem::path::extension. + /// Counterpart for std::filesystem::path::filename. /// /// \return name + extension of current path. std::string filename() const; diff --git a/src/lib/util/io.h b/src/lib/util/io.h index ce64422060..4e5f8c41bd 100644 --- a/src/lib/util/io.h +++ b/src/lib/util/io.h @@ -23,7 +23,7 @@ namespace util { /// /// \return Value of the integer. template -uint_t +constexpr uint_t readUint(void const* const buffer, size_t const length) { constexpr size_t size(sizeof(uint_t)); if (length < size) { @@ -33,10 +33,11 @@ readUint(void const* const buffer, size_t const length) { } uint8_t const* const byte_buffer(static_cast(buffer)); - uint_t result; - uint8_t* pointer_to_result(static_cast(static_cast(&result))); + uint_t result(0); - std::reverse_copy(byte_buffer, byte_buffer + size, pointer_to_result); + for (size_t i = 0; i < size; ++i) { + result |= (static_cast(byte_buffer[i])) << (8 * (size - (i + 1))); + } return (result); } @@ -51,7 +52,7 @@ readUint(void const* const buffer, size_t const length) { /// /// \return pointer to the next byte after stored value template -uint8_t* +constexpr uint8_t* writeUint(uint_t const value, void* const buffer, size_t const length) { constexpr size_t size(sizeof(uint_t)); if (length < size) { @@ -60,48 +61,50 @@ writeUint(uint_t const value, void* const buffer, size_t const length) { << (length == 1 ? "" : "s") << " instead"); } - uint8_t const* pointer_to_value(static_cast(static_cast(&value))); uint8_t* byte_buffer(static_cast(buffer)); - std::reverse_copy(pointer_to_value, pointer_to_value + size, byte_buffer); + for (size_t i = 0; i < size; ++i) { + uint8_t const shift_by(8 * (size - (i + 1))); + byte_buffer[i] = uint8_t((value & (uint_t(0xff) << shift_by)) >> shift_by); + } return (byte_buffer + size); } /// \brief uint16_t wrapper over readUint. -inline uint16_t +constexpr inline uint16_t readUint16(void const* const buffer, size_t const length) { - return readUint(buffer, length); + return (readUint(buffer, length)); } /// \brief uint32_t wrapper over readUint. -inline uint32_t +constexpr inline uint32_t readUint32(void const* const buffer, size_t const length) { - return readUint(buffer, length); + return (readUint(buffer, length)); } /// \brief uint16_t wrapper over readUint. -inline uint64_t +constexpr inline uint64_t readUint64(void const* const buffer, size_t const length) { - return readUint(buffer, length); + return (readUint(buffer, length)); } /// \brief uint16_t wrapper over writeUint. -inline uint8_t* +constexpr inline uint8_t* writeUint16(uint16_t const value, void* const buffer, size_t const length) { - return writeUint(value, buffer, length); + return (writeUint(value, buffer, length)); } /// \brief uint32_t wrapper over writeUint. -inline uint8_t* +constexpr inline uint8_t* writeUint32(uint32_t const value, void* const buffer, size_t const length) { - return writeUint(value, buffer, length); + return (writeUint(value, buffer, length)); } /// \brief uint64_t wrapper over writeUint. -inline uint8_t* +constexpr inline uint8_t* writeUint64(uint64_t const value, void* const buffer, size_t const length) { - return writeUint(value, buffer, length); + return (writeUint(value, buffer, length)); } } // namespace util diff --git a/src/lib/util/str.cc b/src/lib/util/str.cc index 9c3a3b857a..093cd0580b 100644 --- a/src/lib/util/str.cc +++ b/src/lib/util/str.cc @@ -31,21 +31,21 @@ namespace str { string trim(const string& input) { if (input.empty()) { - return string(); + return (string()); } static const char* blanks = " \t\n"; // Search for first non-blank character in the string. size_t const first(input.find_first_not_of(blanks)); if (first == string::npos) { - return string(); + return (string()); } // String not all blanks, so look for last character. size_t const last(input.find_last_not_of(blanks)); // Extract the trimmed substring. - return input.substr(first, (last - first + 1)); + return (input.substr(first, (last - first + 1))); } vector @@ -329,7 +329,7 @@ isPrintable(const vector& content) { string dumpAsHex(const uint8_t* data, size_t length) { stringstream output; - for (unsigned int i = 0; i < length; i++) { + for (size_t i = 0; i < length; ++i) { if (i) { output << ":"; } diff --git a/src/lib/util/tests/io_unittests.cc b/src/lib/util/tests/io_unittests.cc index c4b6a36c7f..320060ffa1 100644 --- a/src/lib/util/tests/io_unittests.cc +++ b/src/lib/util/tests/io_unittests.cc @@ -15,90 +15,116 @@ #include #include +#include #include #include +#include #include #include using namespace isc; using namespace isc::util; +using namespace std; namespace { +struct ReadWriteUintTest : ::testing::Test { + template + string showDiff(uint_t const left, uint8_t* const right) { + bitset<8 * sizeof(uint_t)> const bitset_left(left); + bitset<8 * sizeof(uint_t)> const bitset_right(*(reinterpret_cast(right))); + string const bitstring_left(separateBytes(bitset_left.to_string())); + string const bitstring_right(separateBytes(bitset_right.to_string())); + string const diff(::testing::internal::edit_distance::CreateUnifiedDiff({bitstring_left}, + {bitstring_right})); + return (diff); + } + +private: + string separateBytes(string const& input) { + size_t const position(8); + stringstream ss; + for (size_t i = 0; i < input.size(); i = i + position) { + if (i) { + ss << " "; + } + ss << input.substr(i, position); + } + return (ss.str()); + } +}; + /// @brief Check whether uint16_t can be read from a buffer properly. -TEST(asioutil, readUint16) { - // Reference buffer - uint8_t data[2] = {0, 0}; - InputBuffer buffer(data, sizeof(data)); - - // Avoid possible compiler warnings by only setting uint8_t variables to - // uint8_t values. - uint8_t i8 = 0; - uint8_t j8 = 0; - for (int i = 0; i < (2 << 8); ++i, ++i8) { - for (int j = 0; j < (2 << 8); ++j, ++j8) { - data[0] = i8; - data[1] = j8; - buffer.setPosition(0); - EXPECT_EQ(buffer.readUint16(), readUint16(data, sizeof(data))); +TEST_F(ReadWriteUintTest, readUint16) { + uint16_t const test16[] = {0, 1, 2000, 0x8000, 0xffff}; + uint8_t data[] = {0, 0, 0, 0}; + + // Make sure that we can read data, regardless of the memory alignment. + // That is why we need to repeat it 2 times. + for (size_t offset = 0; offset < sizeof(uint16_t); ++offset) { + for (size_t i = 0; i < sizeof(test16) / sizeof(uint16_t); ++i) { + SCOPED_TRACE("offset " + to_string(offset) + ", iteration " + to_string(i)); + + uint16_t tmp = htons(test16[i]); + memcpy(&data[offset], &tmp, sizeof(uint16_t)); + + EXPECT_EQ(test16[i], readUint16(&data[offset], sizeof(uint16_t))); } } } /// @brief Check whether reading an uint16_t results in an exception. -TEST(asioutil, readUint16OutOfRange) { - uint8_t data = 0; - EXPECT_THROW_MSG(readUint16(&data, sizeof(data)), OutOfRange, +TEST_F(ReadWriteUintTest, readUint16OutOfRange) { + uint8_t data[] = {0}; + EXPECT_THROW_MSG(readUint16(data, 1), OutOfRange, "Expected buffer to be long enough to read a 2-byte integer, but got 1 byte " "instead"); } /// @brief Check whether uint16_t can be written to a buffer properly. -TEST(asioutil, writeUint16) { - // Reference buffer - OutputBuffer buffer(2); - uint8_t test[2]; - - // Avoid possible compiler warnings by only setting uint16_t variables to - // uint16_t values. - uint16_t i16 = 0; - for (uint32_t i = 0; i < (2 << 16); ++i, ++i16) { - // Write the reference data - buffer.clear(); - buffer.writeUint16(i16); - - // ... and the test data - writeUint16(i16, test, sizeof(test)); - - // ... and compare - const uint8_t* ref = static_cast(buffer.getData()); - EXPECT_EQ(ref[0], test[0]); - EXPECT_EQ(ref[1], test[1]); +TEST_F(ReadWriteUintTest, writeUint16) { + uint16_t const test16[] = {0, 1, 2000, 0x8000, 0xffff}; + uint8_t data[4]; + + // Make sure that we can write data, regardless of the memory alignment. + // That's why we need to repeat 2 times. + for (size_t offset = 0; offset < sizeof(uint16_t); ++offset) { + for (size_t i = 0; i < sizeof(test16) / sizeof(uint16_t); ++i) { + SCOPED_TRACE("offset " + to_string(offset) + ", iteration " + to_string(i)); + + uint8_t* ptr = writeUint16(test16[i], &data[offset], sizeof(uint16_t)); + + EXPECT_EQ(&data[offset] + sizeof(uint16_t), ptr); + + uint16_t tmp = htons(test16[i]); + + EXPECT_EQ(0, memcmp(&tmp, &data[offset], sizeof(uint16_t))) + << showDiff(tmp, &data[offset]); + } } } /// @brief Check whether writing an uint16_t results in an exception. -TEST(asioutil, writeUint16OutOfRange) { +TEST_F(ReadWriteUintTest, writeUint16OutOfRange) { uint16_t i16 = 42; - uint8_t data; - EXPECT_THROW_MSG(writeUint16(i16, &data, sizeof(data)), OutOfRange, + uint8_t data[1]; + EXPECT_THROW_MSG(writeUint16(i16, data, sizeof(data)), OutOfRange, "Expected buffer to be long enough to write a 2-byte integer, but got 1 byte " "instead"); } -/// @brief Test data shared amount readUint32 and writeUint32 tests. -const static uint32_t test32[] = {0, 1, 2000, 0x80000000, 0xffffffff}; - /// @brief Check whether uint32_t can be read from a buffer properly. -TEST(asioutil, readUint32) { - uint8_t data[8] = {0, 0, 0, 0, 0, 0, 0, 0}; - - // Make sure that we can read data, regardless of - // the memory alignment. That is why we need to repeat - // it 4 times. - for (int offset = 0; offset < 4; offset++) { - for (size_t i = 0; i < sizeof(test32) / sizeof(uint32_t); i++) { +TEST_F(ReadWriteUintTest, readUint32) { + uint32_t const test32[] = {0, 1, 2000, 0x80000000, 0xffffffff}; + uint8_t data[] = {0, 0, 0, 0, 0, 0, 0, 0}; + + // Make sure that we can read data, regardless of the memory alignment. + // That is why we need to repeat it 4 times. + for (size_t offset = 0; offset < sizeof(uint32_t); ++offset) { + for (size_t i = 0; i < sizeof(test32) / sizeof(uint32_t); ++i) { + SCOPED_TRACE("offset " + to_string(offset) + ", iteration " + to_string(i)); + uint32_t tmp = htonl(test32[i]); memcpy(&data[offset], &tmp, sizeof(uint32_t)); @@ -108,35 +134,38 @@ TEST(asioutil, readUint32) { } /// @brief Check whether reading an uint32_t results in an exception. -TEST(asioutil, readUint32OutOfRange) { - uint8_t data[3] = {0, 0, 0}; - EXPECT_THROW_MSG(readUint32(data, sizeof(data)), OutOfRange, +TEST_F(ReadWriteUintTest, readUint32OutOfRange) { + uint8_t data[] = {0, 0, 0}; + EXPECT_THROW_MSG(readUint32(data, 3), OutOfRange, "Expected buffer to be long enough to read a 4-byte integer, but got 3 bytes " "instead"); } /// @brief Check whether uint32_t can be written to a buffer properly. -TEST(asioutil, writeUint32) { +TEST_F(ReadWriteUintTest, writeUint32) { + uint32_t const test32[] = {0, 1, 2000, 0x80000000, 0xffffffff}; uint8_t data[8]; - // make sure that we can write data, regardless of - // the memory alignment. That's why we need to repeat - // it 4 times. - for (int offset = 0; offset < 4; offset++) { - for (size_t i = 0; i < sizeof(test32) / sizeof(uint32_t); i++) { + // Make sure that we can write data, regardless of the memory alignment. + // That's why we need to repeat 4 times. + for (size_t offset = 0; offset < sizeof(uint32_t); ++offset) { + for (size_t i = 0; i < sizeof(test32) / sizeof(uint32_t); ++i) { + SCOPED_TRACE("offset " + to_string(offset) + ", iteration " + to_string(i)); + uint8_t* ptr = writeUint32(test32[i], &data[offset], sizeof(uint32_t)); EXPECT_EQ(&data[offset] + sizeof(uint32_t), ptr); uint32_t tmp = htonl(test32[i]); - EXPECT_EQ(0, memcmp(&tmp, &data[offset], sizeof(uint32_t))); + EXPECT_EQ(0, memcmp(&tmp, &data[offset], sizeof(uint32_t))) + << showDiff(tmp, &data[offset]); } } } /// @brief Check whether writing an uint32_t results in an exception. -TEST(asioutil, writeUint32OutOfRange) { +TEST_F(ReadWriteUintTest, writeUint32OutOfRange) { uint32_t i32 = 28; uint8_t data[3]; EXPECT_THROW_MSG(writeUint32(i32, data, sizeof(data)), OutOfRange, @@ -145,65 +174,62 @@ TEST(asioutil, writeUint32OutOfRange) { } /// @brief Check whether uint64_t can be read from a buffer properly. -TEST(asioutil, readUint64) { - uint8_t buf[8]; - for (size_t offset = 0; offset < sizeof(buf); offset++) { - buf[offset] = offset + 1; - } - - // Now check if a real value could be read. - const uint64_t exp_val = 0x0102030405060708ul; - uint64_t val; +TEST_F(ReadWriteUintTest, readUint64) { + uint64_t const test64[] = {0, 1, 2000, 0x80000000, 0xffffffff, 0xfffffffffffffff}; + uint8_t data[] = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}; - EXPECT_NO_THROW_LOG(val = readUint64(buf, 8)); - EXPECT_EQ(val, exp_val); + // Make sure that we can read data, regardless of the memory alignment. + // That is why we need to repeat it 8 times. + for (size_t offset = 0; offset < sizeof(uint64_t); ++offset) { + for (size_t i = 0; i < sizeof(test64) / sizeof(uint64_t); ++i) { + SCOPED_TRACE("offset " + to_string(offset) + ", iteration " + to_string(i)); - // Now check if there are no buffer overflows. - memset(buf, 0xff, 8); + uint64_t tmp = (uint64_t(htonl(test64[i])) << 32) | htonl(test64[i] >> 32); + memcpy(&data[offset], &tmp, sizeof(uint64_t)); - EXPECT_NO_THROW_LOG(val = readUint64(buf, 8)); - EXPECT_EQ(0xfffffffffffffffful, val); + EXPECT_EQ(test64[i], readUint64(&data[offset], sizeof(uint64_t))); + } + } } /// @brief Check whether reading an uint64_t results in an exception. -TEST(asioutil, readUint64OutOfRange) { - uint8_t buf[8]; +TEST_F(ReadWriteUintTest, readUint64OutOfRange) { + uint8_t buf[] = {0, 0, 0, 0, 0, 0, 0}; - // Let's do some simple sanity checks first. - EXPECT_THROW_MSG(readUint64(NULL, 0), OutOfRange, - "Expected buffer to be long enough to read a 8-byte integer, but got 0 bytes " - "instead"); EXPECT_THROW_MSG(readUint64(buf, 7), OutOfRange, "Expected buffer to be long enough to read a 8-byte integer, but got 7 bytes " "instead"); } /// @brief Check whether uint64 can be written to a buffer properly. -TEST(asioutil, writeUint64) { - uint8_t data[8]; +TEST_F(ReadWriteUintTest, writeUint64) { + uint64_t const test64[] = {0, 1, 2000, 0x80000000, 0xffffffff, 0xfffffffffffffff}; + uint8_t data[16]; - // make sure that we can write data, regardless of - // the memory alignment. That's why we need to repeat - // it 4 times. - for (int offset = 0; offset < 4; offset++) { - for (size_t i = 0; i < sizeof(test32) / sizeof(uint32_t); i++) { - uint8_t* ptr = writeUint32(test32[i], &data[offset], sizeof(uint32_t)); + // Make sure that we can write data, regardless of the memory alignment. + // That's why we need to repeat 8 times. + for (size_t offset = 0; offset < sizeof(uint64_t); ++offset) { + for (size_t i = 0; i < sizeof(test64) / sizeof(uint64_t); ++i) { + SCOPED_TRACE("offset " + to_string(offset) + ", iteration " + to_string(i)); - EXPECT_EQ(&data[offset] + sizeof(uint32_t), ptr); + uint8_t* ptr = writeUint64(test64[i], &data[offset], sizeof(uint64_t)); - uint32_t tmp = htonl(test32[i]); + EXPECT_EQ(&data[offset] + sizeof(uint64_t), ptr); + + uint64_t tmp = (uint64_t(htonl(test64[i])) << 32) | htonl(test64[i] >> 32); - EXPECT_EQ(0, memcmp(&tmp, &data[offset], sizeof(uint32_t))); + EXPECT_EQ(0, memcmp(&tmp, &data[offset], sizeof(uint64_t))) + << showDiff(tmp, &data[offset]); } } } /// @brief Check whether writing an uint64_t results in an exception. -TEST(asioutil, writeUint64OutOfRange) { - uint32_t i32 = 28; - uint8_t data[3]; - EXPECT_THROW_MSG(writeUint32(i32, data, sizeof(data)), OutOfRange, - "Expected buffer to be long enough to write a 4-byte integer, but got 3 bytes " +TEST_F(ReadWriteUintTest, writeUint64OutOfRange) { + uint64_t i64 = 28; + uint8_t data[7]; + EXPECT_THROW_MSG(writeUint64(i64, data, sizeof(data)), OutOfRange, + "Expected buffer to be long enough to write a 8-byte integer, but got 7 bytes " "instead"); }