From c2405f00bf59ce9f84164949ffb248a82ff29139 Mon Sep 17 00:00:00 2001
From: Matthias Richter <Matthias.Richter@scieq.net>
Date: Tue, 15 May 2018 22:08:54 +0200
Subject: [PATCH] Bugfix: set next-header flag in the last header of Stack as
 parameter

The argument list to built up a header stack can be an argument pack
of stacks and BaseHeader-derived headers. Since stacks consist of a
collection of headers, the next-header flag must be set in the last
header of the sequence. Until now, the flag was only set in the first
header (where it was already before).

Stack itsself is more generic and would also support e.g. arrays and
strings. However, in order to support header navigation, Stack is
restricted to contain only BaseHeader derived structures. Enforcing
this restriction now with a static_assert.
---
 .../Headers/include/Headers/DataHeader.h      | 31 ++++++++++++++---
 DataFormats/Headers/test/testDataHeader.cxx   | 34 +++++++++++++++++++
 2 files changed, 60 insertions(+), 5 deletions(-)

diff --git a/DataFormats/Headers/include/Headers/DataHeader.h b/DataFormats/Headers/include/Headers/DataHeader.h
index 746be57fc5e28..e72ade5904bbd 100644
--- a/DataFormats/Headers/include/Headers/DataHeader.h
+++ b/DataFormats/Headers/include/Headers/DataHeader.h
@@ -387,21 +387,35 @@ struct BaseHeader
   ///
   /// this is to guess if the buffer starting at b looks like a header
   inline static const BaseHeader* get(const byte* b, size_t /*len*/=0) {
-    return (*(reinterpret_cast<const uint32_t*>(b))==sMagicString) ?
-      reinterpret_cast<const BaseHeader*>(b) :
-      nullptr;
+    return (b != nullptr && *(reinterpret_cast<const uint32_t*>(b)) == sMagicString)
+             ? reinterpret_cast<const BaseHeader*>(b)
+             : nullptr;
+  }
+
+  /// @brief access header in buffer
+  ///
+  /// this is to guess if the buffer starting at b looks like a header
+  inline static BaseHeader* get(byte* b, size_t /*len*/ = 0)
+  {
+    return (b != nullptr && *(reinterpret_cast<uint32_t*>(b)) == sMagicString) ? reinterpret_cast<BaseHeader*>(b)
+                                                                               : nullptr;
   }
 
   inline uint32_t size() const noexcept { return headerSize; }
   inline const byte* data() const noexcept { return reinterpret_cast<const byte*>(this); }
 
-  /// get the next header if any
+  /// get the next header if any (const version)
   inline const BaseHeader* next() const noexcept {
     return (flagsNextHeader) ?
       reinterpret_cast<const BaseHeader*>(reinterpret_cast<const byte*>(this)+headerSize) :
       nullptr;
   }
 
+  /// get the next header if any (non-const version)
+  inline BaseHeader* next() noexcept
+  {
+    return (flagsNextHeader) ? reinterpret_cast<BaseHeader*>(reinterpret_cast<byte*>(this) + headerSize) : nullptr;
+  }
 };
 
 /// find a header of type HeaderType in a buffer
@@ -500,6 +514,8 @@ struct Stack {
 
   template<typename T>
   static byte* inject(byte* here, const T& h) noexcept {
+    static_assert(std::is_base_of<BaseHeader, T>::value == true || std::is_same<Stack, T>::value == true,
+                  "header stack parameters are restricted to stacks and headers derived from BaseHeader");
     std::copy(h.data(), h.data()+h.size(), here);
     return here + h.size();
   }
@@ -507,7 +523,12 @@ struct Stack {
   template<typename T, typename... Args>
   static byte* inject(byte* here, const T& h, const Args... args) noexcept {
     auto alsohere = inject(here, h);
-    (reinterpret_cast<BaseHeader*>(here))->flagsNextHeader = true;
+    // the type might be a stack itself, loop through headers and set the flag in the last one
+    BaseHeader* next = BaseHeader::get(here);
+    while (next->flagsNextHeader) {
+      next = next->next();
+    }
+    next->flagsNextHeader = true;
     return inject(alsohere, args...);
   }
 };
diff --git a/DataFormats/Headers/test/testDataHeader.cxx b/DataFormats/Headers/test/testDataHeader.cxx
index 2b032fd394d09..69ac27342c844 100644
--- a/DataFormats/Headers/test/testDataHeader.cxx
+++ b/DataFormats/Headers/test/testDataHeader.cxx
@@ -22,6 +22,29 @@
 using system_clock = std::chrono::system_clock;
 using TimeScale = std::chrono::nanoseconds;
 
+namespace o2
+{
+namespace header
+{
+namespace test
+{
+struct MetaHeader : public BaseHeader {
+  // Required to do the lookup
+  static const o2::header::HeaderType sHeaderType;
+  static const uint32_t sVersion = 1;
+
+  MetaHeader(uint32_t v)
+    : BaseHeader(sizeof(MetaHeader), sHeaderType, o2::header::gSerializationMethodNone, sVersion), secret(v)
+  {
+  }
+
+  uint64_t secret;
+};
+constexpr o2::header::HeaderType MetaHeader::sHeaderType = "MetaHead";
+}
+}
+}
+
 namespace o2 {
   namespace header {
 
@@ -176,6 +199,17 @@ namespace o2 {
       BOOST_CHECK(h2->serialization == gSerializationMethodNone);
       BOOST_CHECK(h2->size() == sizeof(NameHeader<9>));
       BOOST_CHECK(h2->getNameLength() == 9);
+
+      auto meta = test::MetaHeader{ 42 };
+      Stack s2{ s1, meta };
+      BOOST_CHECK(s2.size() == s1.size() + sizeof(decltype(meta)));
+
+      auto* h3 = get<test::MetaHeader*>(s1.data());
+      BOOST_CHECK(h3 == nullptr);
+      h3 = get<test::MetaHeader*>(s2.data());
+      BOOST_CHECK(h3 != nullptr);
+      h1 = get<DataHeader*>(s2.data());
+      BOOST_CHECK(h1 != nullptr);
     }
 
     BOOST_AUTO_TEST_CASE(Descriptor_benchmark)