From review

sonndinh · Feb 6, 2024 · 8291f99 · 8291f99
1 parent 84269c6
commit 8291f99
Show file tree

Hide file tree

Showing 6 changed files with 20 additions and 15 deletions.
diff --git a/dds/DCPS/JsonValueWriter.h b/dds/DCPS/JsonValueWriter.h
@@ -59,7 +59,7 @@ class JsonValueWriter : public ValueWriter {
   void end_array();
   void begin_sequence(XTypes::TypeKind elem_kind = XTypes::TK_NONE, ACE_CDR::ULong length = 0);
   void end_sequence();
-  void begin_element(size_t idx);
+  void begin_element(ACE_CDR::ULong idx);
   void end_element();
 
   void write_boolean(ACE_CDR::Boolean value);
@@ -168,7 +168,7 @@ void JsonValueWriter<Writer>::end_sequence()
 }
 
 template <typename Writer>
-void JsonValueWriter<Writer>::begin_element(size_t /*idx*/)
+void JsonValueWriter<Writer>::begin_element(ACE_CDR::ULong /*idx*/)
 {}
 
 template <typename Writer>

diff --git a/dds/DCPS/PrinterValueWriter.h b/dds/DCPS/PrinterValueWriter.h
@@ -52,7 +52,7 @@ class PrinterValueWriter : public ValueWriter {
   void end_array();
   void begin_sequence(XTypes::TypeKind elem_tk = XTypes::TK_NONE, ACE_CDR::ULong length = 0);
   void end_sequence();
-  void begin_element(size_t idx);
+  void begin_element(ACE_CDR::ULong idx);
   void end_element();
 
   void write_boolean(ACE_CDR::Boolean value);
@@ -167,7 +167,7 @@ void PrinterValueWriter::end_sequence()
   current_indent_ -= indent_;
 }
 
-void PrinterValueWriter::begin_element(size_t idx)
+void PrinterValueWriter::begin_element(ACE_CDR::ULong idx)
 {
   stream_ << newline() << std::string(current_indent_, ' ') << '[' << idx << "]: ";
   at_newline_ = false;

diff --git a/dds/DCPS/ValueWriter.h b/dds/DCPS/ValueWriter.h
@@ -29,41 +29,41 @@ struct VWriterMemberParam {
   VWriterMemberParam()
     : id()
     , must_understand(0)
-    , name(0)
     , optional(false)
     , present(true)
+    , name(0)
   {}
 
   VWriterMemberParam(unsigned m_id, bool m_must_understand)
     : id(m_id)
     , must_understand(m_must_understand)
-    , name(0)
     , optional(false)
     , present(true)
+    , name(0)
   {}
 
   VWriterMemberParam(const char* m_name)
     : id(0)
     , must_understand(false)
-    , name(m_name)
     , optional(false)
     , present(true)
+    , name(m_name)
   {}
 
   VWriterMemberParam(unsigned m_id, bool m_must_understand,
                      const char* m_name, bool m_optional, bool m_present)
     : id(m_id)
     , must_understand(m_must_understand)
-    , name(m_name)
     , optional(m_optional)
     , present(m_present)
+    , name(m_name)
   {}
 
   unsigned id;
   bool must_understand;
-  const char* name;
   bool optional;
   bool present;
+  const char* name;
 };
 
 /// A ValueWriter receives events and values from the recitation of a
@@ -98,7 +98,7 @@ class OpenDDS_Dcps_Export ValueWriter {
   virtual void end_array() {}
   virtual void begin_sequence(XTypes::TypeKind /*elem_kind*/, ACE_CDR::ULong /*length*/) {}
   virtual void end_sequence() {}
-  virtual void begin_element(size_t /*idx*/) {}
+  virtual void begin_element(ACE_CDR::ULong /*idx*/) {}
   virtual void end_element() {}
 
   virtual void write_boolean(ACE_CDR::Boolean /*value*/) = 0;
@@ -145,7 +145,7 @@ class OpenDDS_Dcps_Export ValueWriter {
 #endif
   }
 
-  virtual void write_enum(const char* /*name*/, ACE_CDR::Long /*value*/, XTypes::TypeKind /*as_int*/) = 0;
+  virtual void write_enum(const char* name, ACE_CDR::Long value, XTypes::TypeKind as_int) = 0;
   template <typename T>
   void write_enum(const char* name, const T& value, XTypes::TypeKind as_int = XTypes::TK_INT32)
   {

diff --git a/dds/DCPS/Xcdr2ValueWriter.cpp b/dds/DCPS/Xcdr2ValueWriter.cpp
@@ -334,7 +334,7 @@ void Xcdr2ValueWriter::end_sequence()
   end_complex();
 }
 
-void Xcdr2ValueWriter::begin_element(size_t /*idx*/)
+void Xcdr2ValueWriter::begin_element(ACE_CDR::ULong /*idx*/)
 {
   return;
 }

diff --git a/dds/DCPS/Xcdr2ValueWriter.h b/dds/DCPS/Xcdr2ValueWriter.h
@@ -5,6 +5,7 @@
 
 #include <vector>
 #include <stack>
+#include <stdexcept>
 
 OPENDDS_BEGIN_VERSIONED_NAMESPACE_DECL
 
@@ -18,7 +19,11 @@ class OpenDDS_Dcps_Export Xcdr2ValueWriter : public ValueWriter {
     , encoding_(encoding)
     , pos_(0)
     , ser_(0)
-  {}
+  {
+    if (encoding.xcdr_version() != Encoding::XCDR_VERSION_2) {
+      throw std::runtime_error("Xcdr2ValueWriter only supports XCDR2");
+    }
+  }
 
   void begin_struct(Extensibility extensibility);
   void end_struct();
@@ -36,7 +41,7 @@ class OpenDDS_Dcps_Export Xcdr2ValueWriter : public ValueWriter {
   void end_array();
   void begin_sequence(XTypes::TypeKind elem_kind, ACE_CDR::ULong length);
   void end_sequence();
-  void begin_element(size_t idx);
+  void begin_element(ACE_CDR::ULong idx);
   void end_element();
 
   void write_boolean(ACE_CDR::Boolean value);

diff --git a/tests/unit-tests/dds/DCPS/Xcdr2ValueWriter.cpp b/tests/unit-tests/dds/DCPS/Xcdr2ValueWriter.cpp
@@ -81,7 +81,7 @@ void check_component_sizes(DCPS::Xcdr2ValueWriter& value_writer, const size_t* a
 {
   std::vector<size_t> expected_sizes(arr, arr + length);
   const std::vector<size_t>& sizes = value_writer.get_serialized_sizes();
-  EXPECT_TRUE(expected_sizes == sizes);
+  EXPECT_EQ(expected_sizes, sizes);
 }
 
 template <typename Xtag, typename Type>
-Original file line number
+Diff line change
@@ Expand Up / @@ -334,7 +334,7 @@ void Xcdr2ValueWriter::end_sequence() @@
       end_complex();
     }
-    void Xcdr2ValueWriter::begin_element(size_t /*idx*/)
+    void Xcdr2ValueWriter::begin_element(ACE_CDR::ULong /*idx*/)
     {
       return;
     }
@@ Expand Down @@