Closes #3 Validate DBC frame definitions and report error locations

README.md

@@ -505,6 +505,36 @@ Parsed fields:
 
 ---
 
+### Frame validation and parse errors
+
+`BO_` definitions require a decimal CAN ID, a nonempty single-token frame name,
+a colon, a decimal payload length, and a transmitter token. Spaces or tabs may
+separate fields, and whitespace before the colon is accepted. Additional trailing
+fields are rejected.
+
+Numeric fields must contain decimal digits only; signs, hexadecimal notation,
+fractional values, suffixes, and overflow are rejected. CAN IDs may contain the
+29-bit identifier plus the optional DBC extended flag (bit 31). Bits 29 and 30
+are rejected rather than silently discarded. For compatibility, unflagged IDs
+above `0x7FF` are still treated as extended.
+
+The payload length must fit `uint8_t` (`0..255`). This is a storage-range check,
+not CAN or CAN FD protocol validation; no new 8-byte or 64-byte limit is imposed.
+
+Parsing stops at the first detected error. Exceptions raised while parsing a
+line are reported as `std::runtime_error` with the supplied file path and the
+one-based physical line number, including blank lines:
+
+```text
+example.dbc:3: Invalid CAN ID: invalid
+```
+
+This change validates frame definitions. Signal syntax and unsupported DBC
+constructs retain their existing behavior; comprehensive validation is still
+planned. File-open errors contain the path but have no line number.
+
+---
+
 ## Signal definition
 
 Example:
@@ -871,5 +901,6 @@ This gives FrameTap a much better foundation for real use, because both browsing
 ## Regression tests
 
 See [tests/README.md](tests/README.md) for compiler and qmake commands.
-Tests cover Intel and Motorola extraction, signed and unsigned 64-bit boundaries,
+Tests cover frame validation and error locations, Intel and Motorola extraction,
+signed and unsigned 64-bit boundaries,
 physical conversion, numeric output, and invalid signal payloads.

dbc_parser.cpp

@@ -15,8 +15,21 @@
 #include <sstream>
 #include <stdexcept>
 #include <cctype>
+#include <charconv>
+#include <limits>
 
 namespace {
+  std::uint32_t ParseDecimal (const std::string &text,
+                              std::uint32_t maximum,
+                              const char *field) {
+    std::uint32_t value = 0U;
+    const auto parsed = std::from_chars (text.data(), text.data() + text.size(), value);
+    if (text.empty() || parsed.ec != std::errc() ||
+        parsed.ptr != text.data() + text.size() || value > maximum)
+      throw std::runtime_error (std::string ("Invalid ") + field + ": " + text);
+    return value;
+  }
+
   /**
    * @brief Remove leading and trailing spaces.
    * @param text Input text.
@@ -45,31 +58,39 @@ DbcDatabase DbcParser::ParseFile (const std::string &filePath) const {
   DbcDatabase database;
   FrameInfo *currentFrame = nullptr;
 
+  std::size_t lineNumber = 0U;
   std::string line;
   while (std::getline (input, line)) {
+    ++lineNumber;
     line = Trim (line);
     if (line.empty())
       continue;
 
-    if (IsFrameLine (line)) {
-      FrameInfo frame = ParseFrameLine (line);
-      database.frames.push_back (frame);
-      currentFrame = &database.frames.back();
-    } else if (IsSignalLine (line)) {
-      if (currentFrame == nullptr)
-        throw std::runtime_error ("Signal found before any frame definition.");
+    try {
+      if (IsFrameLine (line)) {
+        FrameInfo frame = ParseFrameLine (line);
+        database.frames.push_back (frame);
+        currentFrame = &database.frames.back();
+      } else if (IsSignalLine (line)) {
+        if (currentFrame == nullptr)
+          throw std::runtime_error ("Signal found before any frame definition.");
 
-      SignalInfo signal = ParseSignalLine (line);
-      currentFrame->signals.push_back (signal);
-    } else if (IsCommentLine (line))
-      ParseCommentLine (line, database);
+        SignalInfo signal = ParseSignalLine (line);
+        currentFrame->signals.push_back (signal);
+      } else if (IsCommentLine (line))
+        ParseCommentLine (line, database);
+    } catch (const std::exception &error) {
+      throw std::runtime_error (filePath + ":" + std::to_string (lineNumber) +
+                                ": " + error.what());
+    }
   }
 
   return database;
 }
 
 bool DbcParser::IsFrameLine (const std::string &line) {
-  return (line.size() >= 4U) && (line.compare (0U, 4U, "BO_ ") == 0);
+  return (line.compare (0U, 3U, "BO_") == 0) &&
+    ((line.size() == 3U) || std::isspace (static_cast<unsigned char> (line[3U])));
 }
 
 bool DbcParser::IsSignalLine (const std::string &line) {
@@ -146,33 +167,37 @@ FrameInfo DbcParser::ParseFrameLine (const std::string &line) {
    */
 
   std::istringstream stream (line);
-  std::string token;
+  std::string keyword;
+  std::string idText;
+  if (!(stream >> keyword >> idText) || keyword != "BO_")
+    throw std::runtime_error ("Missing CAN ID in frame definition.");
+
+  const std::uint32_t rawCanId = ParseDecimal (
+    idText, std::numeric_limits<std::uint32_t>::max(), "CAN ID");
+  // Only bit 31 may accompany the actual 29-bit identifier.
+  if ((rawCanId & 0x60000000U) != 0U)
+    throw std::runtime_error ("CAN ID exceeds the 29-bit identifier range.");
+
+  std::string nameText;
+  if (!std::getline (stream, nameText, ':') || stream.eof())
+    throw std::runtime_error ("Missing ':' in frame definition.");
+  std::istringstream nameStream (nameText);
   FrameInfo frame;
-  std::uint32_t rawCanId = 0U;
+  std::string extra;
+  if (!(nameStream >> frame.name) || (nameStream >> extra))
+    throw std::runtime_error ("Missing or invalid frame name.");
 
-  stream >> token;
-  if (token != "BO_")
-    throw std::runtime_error ("Invalid frame line: " + line);
+  std::string lengthText;
+  if (!(stream >> lengthText))
+    throw std::runtime_error ("Missing payload length in frame definition.");
+  frame.dlc = static_cast<std::uint8_t> (ParseDecimal (
+    lengthText, std::numeric_limits<std::uint8_t>::max(), "payload length"));
+  if (!(stream >> frame.transmitter))
+    throw std::runtime_error ("Missing transmitter in frame definition.");
+  if (stream >> extra)
+    throw std::runtime_error ("Unexpected trailing field in frame definition: " + extra);
 
-  stream >> rawCanId;
   NormalizeCanId (rawCanId, frame.canId, frame.isExtended);
-
-  stream >> token;
-  if (token.empty())
-    throw std::runtime_error ("Missing frame name: " + line);
-
-  if (token[token.size() - 1U] == ':')
-    token.erase (token.size() - 1U, 1U);
-
-  frame.name = token;
-
-  {
-    unsigned int dlcValue = 0U;
-    stream >> dlcValue;
-    frame.dlc = static_cast<std::uint8_t> (dlcValue);
-  }
-
-  stream >> frame.transmitter;
   frame.pgn = TryExtractPgn (frame.canId, frame.isExtended, frame.hasPgn);
 
   return frame;

dbc_parser.h

@@ -39,6 +39,7 @@ class DbcParser {
      * @param filePath Path to input file.
      * @return Parsed database.
      * @throws std::runtime_error on file or parse errors.
+     * Line parse errors include the file path and one-based line number.
      */
     DbcDatabase ParseFile (const std::string &filePath) const;
 

tests/README.md

@@ -26,3 +26,22 @@ signals, signed values, scaling, 64-bit extraction, signed/unsigned boundary val
 (including `INT64_MIN`, `INT64_MAX`, and `UINT64_MAX`), numeric output, invalid lengths, insufficient
 payloads, and Intel regression cases. Frame validity means the definition was
 found; insufficient payloads are checked through signal validity.
+
+## Parser regression tests
+
+Run from the repository root:
+
+```sh
+g++ -std=c++17 -Wall -Wextra -Wpedantic -Werror -I. \
+  tests/dbc_parser_test.cpp dbc_parser.cpp -o /tmp/dbc_parser_test
+/tmp/dbc_parser_test
+```
+
+For qmake, use `tests/dbc_parser_test.pro` in a separate build directory,
+following the decoder instructions above, then run `./dbc_parser_test`.
+
+Parser tests cover standard and extended CAN IDs, whitespace, payload storage
+boundaries, missing fields, malformed and overflowing numbers, reserved ID bits,
+error paths and line numbers, and integration with signals and comments.
+Temporary DBC fixtures are created in the system temporary directory and removed
+automatically.

tests/dbc_parser_test.cpp

@@ -0,0 +1,94 @@
+#include "dbc_parser.h"
+
+#include <chrono>
+#include <filesystem>
+#include <fstream>
+#include <iostream>
+#include <stdexcept>
+
+struct Fixture {
+  std::filesystem::path path = std::filesystem::temp_directory_path() /
+    ("dbc_parser_test_" + std::to_string (
+      std::chrono::steady_clock::now().time_since_epoch().count()) + ".dbc");
+  ~Fixture() { std::error_code error; std::filesystem::remove (path, error); }
+  void Write (const std::string &text) const {
+    std::ofstream output (path);
+    output << text;
+    if (!output)
+      throw std::runtime_error ("Cannot write test fixture.");
+  }
+};
+
+int main() {
+  Fixture fixture;
+  int failures = 0;
+  std::size_t count = 0U;
+  const auto check = [&] (bool passed, const std::string &name) {
+    ++count;
+    if (!passed) { ++failures; std::cerr << "FAIL: " << name << '\n'; }
+  };
+  struct ValidCase { const char *line; std::uint32_t id; bool extended; unsigned length; };
+  const ValidCase valid[] = {
+    {"BO_ 0 Frame: 0 ECU", 0U, false, 0U},
+    {"BO_ 2047 Frame: 8 ECU", 2047U, false, 8U},
+    {"BO_ 2048 Frame: 64 ECU", 2048U, true, 64U},
+    {"BO_ 536870911 Frame: 255 ECU", 0x1FFFFFFFU, true, 255U},
+    {"BO_ 2147483649 Frame: 8 ECU", 1U, true, 8U},
+    {"BO_ 2684354559 Frame: 8 ECU", 0x1FFFFFFFU, true, 8U},
+    {"  BO_\t256\tFrame :\t8\tECU  \r", 256U, false, 8U}
+  };
+  for (const auto &test : valid) {
+    fixture.Write (std::string (test.line) + '\n');
+    try {
+      const auto database = DbcParser().ParseFile (fixture.path.string());
+      check (database.frames.size() == 1U && database.frames[0].name == "Frame" &&
+             database.frames[0].canId == test.id &&
+             database.frames[0].isExtended == test.extended &&
+             database.frames[0].dlc == test.length &&
+             database.frames[0].transmitter == "ECU", test.line);
+    } catch (const std::exception &error) { check (false, error.what()); }
+  }
+  const char *invalid[] = {
+    "BO_ invalid", "BO_", "BO_ 1", "BO_ 1 : 8 ECU", "BO_ 1 Frame 8 ECU",
+    "BO_ 1 Frame:", "BO_ 1 Frame: 8", "BO_ 1 Frame: 8 ECU extra",
+    "BO_ 1 Two Names: 8 ECU", "BO_ -1 Frame: 8 ECU", "BO_ +1 Frame: 8 ECU",
+    "BO_ 1x Frame: 8 ECU", "BO_ 0x123 Frame: 8 ECU", "BO_ 1.5 Frame: 8 ECU",
+    "BO_ 4294967296 Frame: 8 ECU", "BO_ 999999999999999999999 Frame: 8 ECU",
+    "BO_ 536870912 Frame: 8 ECU", "BO_ 1073741824 Frame: 8 ECU",
+    "BO_ 2684354560 Frame: 8 ECU", "BO_ 4294967295 Frame: 8 ECU",
+    "BO_ 1 Frame: -1 ECU", "BO_ 1 Frame: +8 ECU", "BO_ 1 Frame: 8x ECU",
+    "BO_ 1 Frame: 8.5 ECU", "BO_ 1 Frame: 256 ECU",
+    "BO_ 1 Frame: 4294967296 ECU", "BO_\tinvalid"
+  };
+  for (const auto *line : invalid) {
+    fixture.Write (std::string ("VERSION \"test\"\n\n") + line + '\n');
+    try {
+      DbcParser().ParseFile (fixture.path.string());
+      check (false, line);
+    } catch (const std::runtime_error &error) {
+      const std::string prefix = fixture.path.string() + ":3: ";
+      check (std::string (error.what()).find (prefix) == 0U &&
+             std::string (error.what()).size() > prefix.size(), line);
+    }
+  }
+  fixture.Write (
+    "BO_TX_BU_ 256 : ECU;\nBO_ 256 Frame: 8 ECU\n"
+    " SG_ Speed : 0|16@1+ (0.5,1) [0|100] \"rpm\" ECU\n"
+    "CM_ BO_ 256 \"Frame comment\";\nCM_ SG_ 256 Speed \"Signal comment\";\n"
+    "BO_ 257 Second: 8 ECU\n");
+  const auto database = DbcParser().ParseFile (fixture.path.string());
+  check (database.frames.size() == 2U && database.frames[0].signals.size() == 1U &&
+         database.frames[0].comment == "Frame comment" &&
+         database.frames[0].signals[0].comment == "Signal comment", "Integration");
+  fixture.Write ("\nSG_ Speed : invalid\n");
+  try {
+    DbcParser().ParseFile (fixture.path.string());
+    check (false, "Signal error location");
+  } catch (const std::runtime_error &error) {
+    check (std::string (error.what()).find (fixture.path.string() + ":2: ") == 0U,
+           "Signal error location");
+  }
+  if (failures != 0) return 1;
+  std::cout << "All " << count << " parser cases passed.\n";
+  return 0;
+}

tests/dbc_parser_test.pro

@@ -0,0 +1,7 @@
+TEMPLATE = app
+TARGET = dbc_parser_test
+CONFIG += console c++17
+CONFIG -= app_bundle qt
+INCLUDEPATH += ..
+SOURCES += dbc_parser_test.cpp ../dbc_parser.cpp
+HEADERS += ../dbc_parser.h ../dbc_database.h ../frame_info.h ../signal_info.h