WIP: generalization, more diagnostics and checks

src/tl/tl/tlProtocolBuffer.cc

@@ -26,9 +26,6 @@
 namespace tl
 {
 
-// @@@
-// Missing: readers should check for proper wire type (i.e. read(int64)->either VARINT or I64, not I32
-
 // ----------------------------------------------------------------------------------
 
 ProtocolBufferReader::ProtocolBufferReader (tl::InputStream &input)
@@ -92,6 +89,10 @@ ProtocolBufferReader::read (float &f)
 void
 ProtocolBufferReader::read (std::string &s)
 {
+  if (m_type != PB_LEN) {
+    error (tl::to_string (tr ("Expected a LEN wire type for a string")));
+  }
+
   size_t value = 0;
   read (value);
 
@@ -109,7 +110,7 @@ ProtocolBufferReader::read (std::string &s)
 void
 ProtocolBufferReader::read (uint32_t &ui32)
 {
-  if (m_type != PB_I32) {
+  if (m_type == PB_VARINT || m_type == PB_LEN) {
 
     pb_varint ui64 = read_varint ();
     if (ui64 > std::numeric_limits<uint32_t>::max ()) {
@@ -118,7 +119,7 @@ ProtocolBufferReader::read (uint32_t &ui32)
 
     ui32 = uint32_t (ui64);
 
-  } else {
+  } else if (m_type == PB_I32) {
 
     ui32 = 0;
     const char *cp = get (sizeof (ui32));
@@ -127,6 +128,8 @@ ProtocolBufferReader::read (uint32_t &ui32)
       ui32 |= (unsigned char) cp [sizeof (ui32) - 1 - i];
     }
 
+  } else {
+    error (tl::to_string (tr ("Expected a VARINT or I32 wire type")));
   }
 }
 
@@ -150,11 +153,11 @@ ProtocolBufferReader::read (int32_t &i32)
 void
 ProtocolBufferReader::read (uint64_t &ui64)
 {
-  if (m_type != PB_I64) {
+  if (m_type == PB_VARINT || m_type == PB_LEN) {
 
     ui64 = read_varint ();
 
-  } else {
+  } else if (m_type == PB_I64) {
 
     ui64 = 0;
     const char *cp = get (sizeof (ui64));
@@ -163,6 +166,8 @@ ProtocolBufferReader::read (uint64_t &ui64)
       ui64 |= (unsigned char) cp [sizeof (ui64) - 1 - i];
     }
 
+  } else {
+    error (tl::to_string (tr ("Expected a VARINT or I64 wire type")));
   }
 }
 
@@ -194,6 +199,10 @@ ProtocolBufferReader::read (bool &b)
 void
 ProtocolBufferReader::open ()
 {
+  if (m_type != PB_LEN) {
+    error (tl::to_string (tr ("Expected a LEN wire type for a submessage")));
+  }
+
   size_t value = 0;
   read (value);
   if (! m_seq_counts.empty ()) {
@@ -259,7 +268,7 @@ ProtocolBufferReader::read_varint ()
     if (! cp) {
       error (tl::to_string (tr ("unexpected end of file")));
     }
-    if ((v & 0xfe00000000000000l) != 0) {
+    if (s + 7 > 64 && pb_varint ((unsigned char) (*cp & 0x7f)) >> (s + 7 - 64) != 0) {
       error (tl::to_string (tr ("64 bit integer overflow")));
     }
     v |= (pb_varint ((unsigned char) (*cp & 0x7f)) << s);

src/tl/tl/tlProtocolBuffer.h

@@ -176,6 +176,11 @@ public:
    *  @brief Returns true if at the end of the file or end of a block
    */
   virtual bool at_end () const = 0;
+
+  /**
+   *  @brief Emits an error at the current position
+   */
+  virtual void error (const std::string &msg) = 0;
 };
 
 /**
@@ -283,6 +288,11 @@ public:
    */
   bool at_end () const;
 
+  /**
+   *  @brief Emits an error at the current position
+   */
+  void error (const std::string &msg);
+
 private:
   tl::InputStream *mp_stream;
   PBWireType m_type;
@@ -292,7 +302,6 @@ private:
 
   pb_varint read_varint ();
   void skip_bytes (size_t n);
-  void error (const std::string &msg);
   const char *get (size_t n);
 };
 

src/tl/tl/tlProtocolBufferStruct.cc

@@ -74,6 +74,21 @@ PBParser::parse_element (const PBElementBase *parent, tl::ProtocolBufferReaderBa
   }
 }
 
+void
+PBParser::expect_header (tl::ProtocolBufferReaderBase &reader, int name_tag, const std::string &name)
+{
+  int tag = reader.read_tag ();
+  if (tag != name_tag) {
+    reader.error (tl::sprintf (tl::to_string (tr ("Expected header field with ID %d (got %d)")), name_tag, tag));
+  }
+
+  std::string n;
+  reader.read (n);
+  if (n != name) {
+    reader.error (tl::sprintf (tl::to_string (tr ("Expected header field with string '%s' (got '%s')")), name, n));
+  }
+}
+
 // --------------------------------------------------------------------
 //  PBElementProxy implementation
 

src/tl/tl/tlProtocolBufferStruct.h

@@ -299,6 +299,8 @@ public:
   void parse (tl::ProtocolBufferReaderBase &reader, const PBElementBase *root, PBReaderState *reader_state);
   void parse_element (const PBElementBase *parent, tl::ProtocolBufferReaderBase &reader);
 
+  void expect_header (tl::ProtocolBufferReaderBase &reader, int name_tag, const std::string &name);
+
   PBReaderState &reader_state ()
   {
     return *mp_state;
@@ -949,9 +951,16 @@ private:
 /**
  *  @brief The root element of the PB structure
  *
- *  The root element is also the handler implementation.
- *  It must be supplied a pointer to an actual root object,
- *  a name and a list of children.
+ *  The root object is also a element node. It belongs to an object
+ *  representing the root of the object hierarchy. Correspondingly,
+ *  child elements (members, elements - aka submessages) need to be
+ *  specified.
+ *
+ *  In addition, the root element has a name and a name tag.
+ *  The name / name tag are emitted as the first element in the
+ *  message, providing a file header with fixed content. This
+ *  allows identifying the file as a encoded protocol buffer
+ *  file for a specific object type.
  */
 
 template <class Obj>
@@ -959,20 +968,20 @@ class TL_PUBLIC_TEMPLATE PBStruct
   : public PBElementBase
 {
 public:
-  PBStruct (const PBElementList *children)
-    : PBElementBase (0, children)
+  PBStruct (const std::string &name, int name_tag, const PBElementList *children)
+    : PBElementBase (0, children), m_name (name), m_name_tag (name_tag)
   {
     // .. nothing yet ..
   }
 
-  PBStruct (const PBElementList &children)
-    : PBElementBase (0, children)
+  PBStruct (const std::string &name, int name_tag, const PBElementList &children)
+    : PBElementBase (0, children), m_name (name), m_name_tag (name_tag)
   {
     // .. nothing yet ..
   }
 
   PBStruct (const PBStruct<Obj> &d)
-    : PBElementBase (d)
+    : PBElementBase (d), m_name (d.name ()), m_name_tag (d.name_tag ())
   {
     // .. nothing yet ..
   }
@@ -982,12 +991,22 @@ public:
     return new PBStruct<Obj> (*this);
   }
 
+  const std::string &name () const
+  {
+    return m_name;
+  }
+
+  int name_tag () const
+  {
+    return m_name_tag;
+  }
+
   void write (tl::ProtocolBufferWriterBase &writer, const Obj &root) const
   {
     PBWriterState writer_state;
     writer_state.push (& root);
 
-    // @@@ writer.write (0, name ());
+    writer.write (name_tag (), name ());
 
     for (PBElementBase::iterator c = this->begin (); c != this->end (); ++c) {
       c->get ()->write (this, writer, writer_state);
@@ -1000,6 +1019,7 @@ public:
     PBReaderState rs;
     rs.push (&root);
     PBParser h;
+    h.expect_header (reader, m_name_tag, m_name);
     h.parse (reader, this, &rs);
     rs.pop (tag);
     tl_assert (rs.empty ());
@@ -1025,6 +1045,9 @@ private:
   {
     // disable base class implementation
   }
+
+  std::string m_name;
+  int m_name_tag;
 };
 
 /**

src/tl/unit_tests/tlProtocolBufferTests.cc

@@ -214,7 +214,7 @@ TEST (100_BasicStruct)
       tl::pb_make_member (&Child::d, 2) +
       tl::pb_make_element (&Child::begin_children, &Child::end_children, &Child::add_child, 3, &child_struct);
 
-  tl::PBStruct<Root> structure (
+  tl::PBStruct<Root> structure ("pbtest-struct", 88888888,
     tl::pb_make_member (&Root::begin_subs, &Root::end_subs, &Root::add_sub, 1) +
     tl::pb_make_member (&Root::begin_isubs, &Root::end_isubs, &Root::add_isub, 2) +
     tl::pb_make_element (&Root::begin_children, &Root::end_children, &Root::add_child, 3, &child_struct) +
@@ -270,8 +270,12 @@ TEST (100_BasicStruct)
   {
     tl::OutputStream os (fn);
     tl::ProtocolBufferWriter writer (os);
-    //  For development: writer.set_debug (true);
     structure.write (writer, root);
+    /*
+    for debugging:
+    tl::ProtocolBufferDumper dumper;
+    structure.write (dumper, root);
+    */
   }
 
   root = Root ();