Support void trailer type, i.e. frame consisting of only header and data

matthiasrichter · matthiasrichter · commit 49f6fcb70cbc · 2018-01-17T14:36:27.000+01:00
The trailer type as second template argument of the ForwardParser is now
optional, 'parse' function can drop trailer check function in that case.

Adding further test cases for format error and non-consistent format.
diff --git a/Algorithm/include/Algorithm/Parser.h b/Algorithm/include/Algorithm/Parser.h
@@ -86,15 +86,36 @@ class ForwardParser {
   static const size_t tailOffset = typesize<TrailerType>::size;
   static const size_t totalOffset = headOffset + tailOffset;
 
+  // alias for callback checking the header, return true if the object
+  // is a valid header
   using CheckHeaderFct = std::function<bool(const HeaderType&)>;
-  using CheckTrailerFct = std::function<bool(const TrailerType&)>;
+
+  // alias for the argument type to be used in the CheckTrailer function
+  // have to forward to a valid type in case of void TrailerType in order
+  // to allow passing by reference
+  using CheckTrailerFctArgumentT = typename std::conditional<
+    !std::is_void<TrailerType>::value, TrailerType, int>::type;
+
+  // alias for callback checking the trailer, takes reference to trailer
+  // object if TrailerType is a valid type, no argument otherwise 
+  template <typename U>
+  using CheckTrailerFct = typename std::conditional<
+    !std::is_void<U>::value,
+    std::function<bool(const CheckTrailerFctArgumentT&)>,
+    std::function<bool()>>::type;
+
+  // alias for callback to get the complete frame size including header,
+  // trailer and the data
   using GetFrameSizeFct = std::function<size_t(const HeaderType& )>;
+
+  // function callback to insert/handle one frame into, sequentially called
+  // for all frames if the whole block has a valid format
   using InsertFct = std::function<bool(FrameInfo&)>;
 
   template<typename InputType>
   int parse(const InputType* buffer, size_t bufferSize,
             CheckHeaderFct checkHeader,
-            CheckTrailerFct checkTrailer,
+            CheckTrailerFct<TrailerType> checkTrailer,
             GetFrameSizeFct getFrameSize,
             InsertFct insert) {
     static_assert(sizeof(InputType) == 1,
@@ -126,7 +147,7 @@ class ForwardParser {
       } else {
         auto trailerStart = buffer + position + frameSize - tailOffset;
         entry.trailer = reinterpret_cast<const TrailerType*>(trailerStart);
-        if (!checkTrailer(*entry.trailer)) break;
+        if (!CheckTrailer(entry, checkTrailer)) break;
       }
 
       // store the extracted frame info and continue with remaining buffer
@@ -150,6 +171,29 @@ class ForwardParser {
     // TODO: decide about error policy
     return -1;
   }
+
+  template<typename InputType, typename U = TrailerType>
+  typename std::enable_if<std::is_void<U>::value, int>::type
+      parse(const InputType* buffer, size_t bufferSize,
+            CheckHeaderFct checkHeader,
+            GetFrameSizeFct getFrameSize,
+            InsertFct insert) {
+    auto checkTrailer = [] () {return true;};
+    return parse(buffer, bufferSize, checkHeader, checkTrailer, getFrameSize, insert);
+  }
+
+private:
+  template <typename U = TrailerType>
+  typename std::enable_if<!std::is_void<U>::value, bool>::type
+  CheckTrailer(const FrameInfo& entry, CheckTrailerFct<TrailerType>& checkTrailer) const {
+    return checkTrailer(*entry.trailer);
+  }
+
+  template <typename U = TrailerType>
+  typename std::enable_if<std::is_void<U>::value, bool>::type
+  CheckTrailer(const FrameInfo&, CheckTrailerFct<TrailerType>&) const {
+    return true;
+  }
 };
 
 /**
@@ -198,9 +242,11 @@ class ReverseParser {
     PtrT payload = nullptr;
     size_t length = 0;
   };
-  // the length offset due to header and trailer
+  /// the length offset due to header
   static const size_t headOffset = typesize<HeaderType>::size;
+  /// the length offset due to trailer
   static const size_t tailOffset = typesize<TrailerType>::size;
+  /// total length offset due to header and trailer
   static const size_t totalOffset = headOffset + tailOffset;
 
   using CheckHeaderFct = std::function<bool(const HeaderType&)>;
diff --git a/Algorithm/test/StaticSequenceAllocator.h b/Algorithm/test/StaticSequenceAllocator.h
@@ -21,21 +21,38 @@ namespace algorithm {
  * and a trailer
  */
 template <typename HeaderT
-          , typename TrailerT>
+          , typename TrailerT = void>
 struct Composite {
   using HeaderType = HeaderT;
   using TrailerType = TrailerT;
   size_t compositeLength = 0;
+  size_t trailerLength = 0;
   size_t dataLength = 0;
 
-  template<size_t N>
-  constexpr Composite(const HeaderType& h, const char  (&d)[N], const TrailerType& t)
+  template<size_t N,
+	   typename U = TrailerType>
+  constexpr Composite(const HeaderType h, const char  (&d)[N],
+		      typename std::conditional<!std::is_void<U>::value, const TrailerType, int>::type t,
+		      typename std::enable_if<!std::is_void<U>::value>::type* = nullptr)
     : header(h)
     , data(d)
     , trailer(t)
   {
     dataLength = N;
-    compositeLength = sizeof(HeaderType) + dataLength + sizeof(TrailerType);
+    trailerLength = sizeof(TrailerType);
+    compositeLength = sizeof(HeaderType) + dataLength + trailerLength;
+  }
+
+  template<size_t N,
+	   typename U = TrailerType>
+  constexpr Composite(const HeaderType& h, const char  (&d)[N],
+		      typename std::enable_if<std::is_void<U>::value>::type* = nullptr)
+    : header(h)
+    , data(d)
+  {
+    dataLength = N;
+    trailerLength = 0;
+    compositeLength = sizeof(HeaderType) + dataLength + trailerLength;
   }
 
   constexpr size_t getLength() const noexcept {
@@ -54,14 +71,16 @@ struct Composite {
     length += sizeof(HeaderType);
     memcpy(buffer + length, data, dataLength);
     length += dataLength;
-    memcpy(buffer + length, &trailer, sizeof(TrailerType));
-    length += sizeof(TrailerType);
+    if (trailerLength > 0) {
+      memcpy(buffer + length, &trailer, trailerLength);
+      length += trailerLength;
+    }
     return length;
   }
 
-  const HeaderType& header;
+  const HeaderType header;
   const char* data = nullptr;
-  const TrailerType& trailer;
+  typename std::conditional<!std::is_void<TrailerType>::value, const TrailerType, int>::type trailer;
 };
 
 /// recursively calculate the length of the sequence
diff --git a/Algorithm/test/parser.cxx b/Algorithm/test/parser.cxx
@@ -68,6 +68,7 @@ BOOST_AUTO_TEST_CASE(test_forwardparser_header_and_trailer)
     return trailer.identifier == 0xaaffee00;
   };
   auto getFrameSize = [] (const typename ParserT::HeaderType& header) {
+    // frame size includes total offset from header and trailer
     return header.payloadSize + ParserT::totalOffset;
   };
 
@@ -93,7 +94,126 @@ BOOST_AUTO_TEST_CASE(test_forwardparser_header_and_trailer)
   BOOST_CHECK(memcmp(frames[2].payload, "dummydata",       frames[2].length) == 0);
 }
 
-// TODO: make a test of a composite with header and payload
+BOOST_AUTO_TEST_CASE(test_forwardparser_header_and_void_trailer)
+{
+  using FrameT = o2::algorithm::Composite<Header>;
+  // note: the length of the data is set in the header word
+  using TestFrame = o2::algorithm::StaticSequenceAllocator;
+  TestFrame tf(FrameT(16, "lotsofsillydata"),
+               FrameT(5,  "test"),
+               FrameT(10, "dummydata")
+               );
+
+  using ParserT = o2::algorithm::ForwardParser<typename FrameT::HeaderType,
+                                               typename FrameT::TrailerType>;
+
+  auto checkHeader = [] (const typename FrameT::HeaderType& header) {
+    return header.identifier == 0xdeadbeef;
+  };
+
+  auto getFrameSize = [] (const typename ParserT::HeaderType& header) {
+    // frame size includes total offset from header and trailer
+    return header.payloadSize + ParserT::totalOffset;
+  };
+
+  std::vector<typename ParserT::FrameInfo> frames;
+  auto insert = [&frames] (typename ParserT::FrameInfo& info) {
+    frames.emplace_back(info);
+    return true;
+  };
+
+  ParserT parser;
+  auto result = parser.parse(tf.buffer.get(), tf.size(),
+                             checkHeader,
+                             getFrameSize,
+                             insert
+                             );
+
+  BOOST_REQUIRE(result == 3);
+  BOOST_REQUIRE(frames.size() == 3);
+
+  BOOST_CHECK(memcmp(frames[0].payload, "lotsofsillydata", frames[0].length) == 0);
+  BOOST_CHECK(memcmp(frames[1].payload, "test",            frames[1].length) == 0);
+  BOOST_CHECK(memcmp(frames[2].payload, "dummydata",       frames[2].length) == 0);
+}
+
+BOOST_AUTO_TEST_CASE(test_forwardparser_no_frames)
+{
+  using FrameT = o2::algorithm::Composite<Header>;
+  // note: the length of the data is set in the header word
+  using TestFrame = o2::algorithm::StaticSequenceAllocator;
+  TestFrame tf(FrameT(16, "lotsofsillydata"),
+               FrameT(5,  "test"),
+               FrameT(10, "dummydata")
+               );
+
+  using ParserT = o2::algorithm::ForwardParser<typename FrameT::HeaderType,
+                                               typename FrameT::TrailerType>;
+
+  auto checkHeader = [] (const typename FrameT::HeaderType& header) {
+    // simply indicate invalid header to read no frames
+    return false;
+  };
+
+  auto getFrameSize = [] (const typename ParserT::HeaderType& header) {
+    // frame size includes total offset from header and trailer
+    return header.payloadSize + ParserT::totalOffset;
+  };
+
+  std::vector<typename ParserT::FrameInfo> frames;
+  auto insert = [&frames] (typename ParserT::FrameInfo& info) {
+    frames.emplace_back(info);
+    return true;
+  };
+
+  ParserT parser;
+  auto result = parser.parse(tf.buffer.get(), tf.size(),
+                             checkHeader,
+                             getFrameSize,
+                             insert
+                             );
+
+  // check that there are really no frames found
+  BOOST_REQUIRE(result == 0);
+}
+
+BOOST_AUTO_TEST_CASE(test_forwardparser_format_error)
+{
+  using FrameT = o2::algorithm::Composite<Header>;
+  // note: the length of the data is set in the header word
+  using TestFrame = o2::algorithm::StaticSequenceAllocator;
+  TestFrame tf(FrameT(16, "lotsofsillydata"),
+               FrameT(4,  "test"), // <- note wrong size
+               FrameT(10, "dummydata")
+               );
+
+  using ParserT = o2::algorithm::ForwardParser<typename FrameT::HeaderType,
+                                               typename FrameT::TrailerType>;
+
+  auto checkHeader = [] (const typename FrameT::HeaderType& header) {
+    return header.identifier == 0xdeadbeef;
+  };
+
+  auto getFrameSize = [] (const typename ParserT::HeaderType& header) {
+    // frame size includes total offset from header and trailer
+    return header.payloadSize + ParserT::totalOffset;
+  };
+
+  std::vector<typename ParserT::FrameInfo> frames;
+  auto insert = [&frames] (typename ParserT::FrameInfo& info) {
+    frames.emplace_back(info);
+    return true;
+  };
+
+  ParserT parser;
+  auto result = parser.parse(tf.buffer.get(), tf.size(),
+                             checkHeader,
+                             getFrameSize,
+                             insert
+                             );
+
+  BOOST_REQUIRE(result == -1);
+}
 
 BOOST_AUTO_TEST_CASE(test_reverseparser)
 {
diff --git a/Algorithm/test/tableview.cxx b/Algorithm/test/tableview.cxx
@@ -120,3 +120,37 @@ BOOST_AUTO_TEST_CASE(test_tableview_reverse)
     BOOST_CHECK(rowidx == requiredNofRowsInColumn[colidx]);
   }
 }
+
+BOOST_AUTO_TEST_CASE(test_tableview_formaterror)
+{
+  using FrameT = o2::algorithm::Composite<HeartbeatHeader, HeartbeatTrailer>;
+  using TestFrame = o2::algorithm::StaticSequenceAllocator;
+  // note: the length of the data is set in the trailer word
+  // specifying wrong length in the second entry, no frames should be added
+  TestFrame tf1(FrameT({0x1100000000000000}, "heartbeatdata", {0x510000000000000e}),
+                FrameT({0x1100000000000001}, "test", {0x5100000000000004}),
+                FrameT({0x1100000000000003}, "dummydata", {0x510000000000000a})
+                );
+
+  // the payload length is set in the trailer, so we need a reverse parser
+  using ParserT = o2::algorithm::ReverseParser<typename FrameT::HeaderType,
+                                               typename FrameT::TrailerType>;
+
+  // define the view type for DataHeader as row descriptor,
+  // HeartbeatHeader as column descriptor and the reverse parser
+  using ViewType = o2::algorithm::TableView<o2::header::DataHeader,
+                                            o2::header::HeartbeatHeader,
+                                            ParserT>;
+  ViewType heartbeatview;
+
+  o2::Header::DataHeader dh;
+  dh.dataDescription = o2::Header::DataDescription("FIRSTSLOT");
+  dh.dataOrigin = o2::Header::DataOrigin("TST");
+  dh.subSpecification = 0;
+  dh.payloadSize = 0;
+
+  heartbeatview.addRow(dh, tf1.buffer.get(), tf1.size());
+
+  BOOST_CHECK(heartbeatview.getNRows() == 0);
+  BOOST_CHECK(heartbeatview.getNColumns() == 0);
+}
