diff --git a/.gitignore b/.gitignore index 8a9d35c..642e3ec 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,2 @@ *.user +.project_rules diff --git a/README.md b/README.md index 491e5f1..0d14855 100644 --- a/README.md +++ b/README.md @@ -336,7 +336,8 @@ This matters because the same numeric identifier must not be confused between st Immutable runtime snapshot with a private frame index. Build it with `DbcDecodeBuilder::Build(parsed)` or construct it from `std::vector`. -Construction validates all definitions before publishing the snapshot and throws +Construction validates all definitions and duplicate keys before building the lookup +index or publishing the snapshot, and throws `std::invalid_argument` for invalid metadata, including manually supplied data: - Normalized CAN IDs must fit 11 bits for standard frames or 29 bits for extended diff --git a/changelog.md b/changelog.md index 4a539f3..3efb19c 100644 --- a/changelog.md +++ b/changelog.md @@ -6,6 +6,20 @@ existing Git tag; subsequent changes are listed as unreleased. ## Unreleased +### 2026-10-14 + +#### Changed + +- Validate duplicate frame keys before building the runtime lookup index, alongside + CAN IDs, signal layouts, numeric parameters, and multiplexing metadata. + +#### Added + +- Exhaustive layout checks for both byte orders at representative payload sizes + (0, 1, 8, 64, and 255 bytes), including invalid lengths and uint32 overflow inputs. +- Regression coverage for builder input mutation/destruction and failed replacement + with duplicate frame or signal keys, preserving existing lookups. + ### 2026-10-13 #### Added diff --git a/core/decode_database.cpp b/core/decode_database.cpp index bb8fc67..9956e43 100644 --- a/core/decode_database.cpp +++ b/core/decode_database.cpp @@ -89,12 +89,16 @@ namespace { DecodeDatabase::Storage::Storage(const std::vector &definitions) : frames(definitions) { // Validate the complete snapshot before creating its lookup index. - for (const DecodeFrame &frame: frames) + std::unordered_set keys; + keys.reserve(frames.size()); + for (const DecodeFrame &frame: frames) { ValidateFrame(frame); + if (!keys.insert(frame.GetKey()).second) + throw std::invalid_argument("Duplicate runtime frame key"); + } frameIndexByKey.reserve(frames.size()); for (std::size_t index = 0U; index < frames.size(); ++index) { - if (!frameIndexByKey.emplace(frames[index].GetKey(), index).second) - throw std::invalid_argument("Duplicate runtime frame key"); + frameIndexByKey.emplace(frames[index].GetKey(), index); } } diff --git a/tests/README.md b/tests/README.md index ffdefd8..225da77 100644 --- a/tests/README.md +++ b/tests/README.md @@ -160,3 +160,10 @@ lengths 1–64. Snapshot tests mutate retained frame, signal, and description al after rvalue construction, and verify unchanged lookups and metadata. Failed constructor/builder replacement preserves the active snapshot; clear removes its index while saved handles remain alive. + +Layout validation is also compared with independent DBC bit traversal for every +start position through one byte beyond payload sizes 0, 1, 8, 64, and 255, and +all lengths 1–64, for both byte orders. Zero/65-bit lengths and uint32 maximum +inputs are rejected. Builder snapshots survive input mutation and destruction; +failed replacement with duplicate frame or signal keys preserves the existing +frame and signal lookups. diff --git a/tests/dbc_validation_test.cpp b/tests/dbc_validation_test.cpp index 6f1c574..5625424 100644 --- a/tests/dbc_validation_test.cpp +++ b/tests/dbc_validation_test.cpp @@ -1,6 +1,7 @@ // SPDX-License-Identifier: MIT #include "dbc_decode_builder.h" #include "dbc_decoder.h" +#include "signal_layout.h" #include #include #include @@ -53,6 +54,8 @@ private Q_SLOTS: void numericParameters_data(); void numericParameters(); void validBoundaries(); + void layoutBoundaries(); + void builderSnapshot(); void multiplexingBoundaries(); void independentSnapshot(); void failedReplacement(); @@ -267,6 +270,50 @@ void DbcValidationTest::validBoundaries() { QVERIFY(DbcDecodeBuilder().Build(DbcDatabase()).GetFrames().empty()); } +void DbcValidationTest::layoutBoundaries() { + // Compare the shared coordinate calculation with independent DBC bit traversal. + for (const std::uint8_t dlc: {std::uint8_t(0U), std::uint8_t(1U), std::uint8_t(8U), + std::uint8_t(64U), std::uint8_t(255U)}) { + const std::uint64_t payloadBits = std::uint64_t(dlc) * 8U; + for (const bool littleEndian: {true, false}) { + for (std::uint32_t start = 0U; start < payloadBits + 8U; ++start) { + std::uint64_t bit = start; + bool fits = true; + QVERIFY(!IsValidSignalLayout(start, 0U, littleEndian, dlc)); + for (std::uint32_t length = 1U; length <= 64U; ++length) { + fits = fits && bit < payloadBits; + QCOMPARE(IsValidSignalLayout(start, length, littleEndian, dlc), fits); + if (littleEndian) + ++bit; + else + bit = bit % 8U == 0U ? bit + 15U : bit - 1U; + } + QVERIFY(!IsValidSignalLayout(start, 65U, littleEndian, dlc)); + } + const std::uint32_t maximum = std::numeric_limits::max(); + QVERIFY(!IsValidSignalLayout(maximum, 1U, littleEndian, dlc)); + QVERIFY(!IsValidSignalLayout(maximum, maximum, littleEndian, dlc)); + QVERIFY(!IsValidSignalLayout(0U, maximum, littleEndian, dlc)); + } + } +} + +void DbcValidationTest::builderSnapshot() { + DbcDatabase source = ParsedDefinitions({ValidFrame()}); + source.frames[0].signals[0].valueDescriptions.emplace(std::uint64_t(1U), "One"); + const FrameKey key(source.frames[0].canId, source.frames[0].isExtended); + const DecodeDatabase database = DbcDecodeBuilder().Build(source); + source.frames[0].canId = 0x800U; + source.frames[0].signals[0].name = "Changed"; + source.frames[0].signals[0].valueDescriptions.begin()->second = "Changed"; + source.frames.clear(); + const std::shared_ptr signal = database.FindSignal(SignalKey(key, "Value")); + QVERIFY(signal); + QCOMPARE(signal->valueDescriptions.begin()->second, std::string("One")); + QVERIFY(database.FindFrame(key)); + QVERIFY(!database.FindFrame(FrameKey(0x800U, false))); +} + void DbcValidationTest::multiplexingBoundaries() { DecodeFrame frame = ValidFrame(); frame.dlc = 9U; @@ -333,6 +380,14 @@ void DbcValidationTest::failedReplacement() { QVERIFY_THROWS_EXCEPTION(std::invalid_argument, database = DbcDecodeBuilder().Build(ParsedDefinitions({invalid}))); QCOMPARE(database.FindFrame(valid.GetKey()).get(), original.get()); + QVERIFY_THROWS_EXCEPTION(std::invalid_argument, database = DecodeDatabase({valid, valid})); + QCOMPARE(database.FindFrame(valid.GetKey()).get(), original.get()); + invalid = valid; + invalid.signals.push_back(invalid.signals[0]); + QVERIFY_THROWS_EXCEPTION(std::invalid_argument, + database = DbcDecodeBuilder().Build(ParsedDefinitions({invalid}))); + QCOMPARE(database.FindFrame(valid.GetKey()).get(), original.get()); + QVERIFY(database.FindSignal(SignalKey(valid.GetKey(), "Value"))); QCOMPARE(database.GetFrames().size(), std::size_t(1U)); database = DecodeDatabase(); QVERIFY(database.GetFrames().empty());