Fix closing submessages with known size

include/protozero/basic_pbf_writer.hpp

@@ -166,8 +166,8 @@ class basic_pbf_writer {
     };
 
     // If m_rollpack_pos is set to this special value, it means that when
-    // the submessage is closed, nothing needs to be done, because the length
-    // of the submessage has already been written correctly.
+    // the submessage is closed, only m_pos needs to be reset, because the
+    // length of the submessage has already been written correctly.
     enum : std::size_t {
         size_is_known = std::numeric_limits<std::size_t>::max()
     };
@@ -211,6 +211,7 @@ class basic_pbf_writer {
     void close_submessage() {
         protozero_assert(m_data);
         if (m_pos == 0 || m_rollback_pos == size_is_known) {
+            m_pos = 0;
             return;
         }
         if (buffer_customization<TBuffer>::size(m_data) - m_pos == 0) {

test/t/message/reader_test_cases.cpp

@@ -76,6 +76,22 @@ TEST_CASE("write message field") {
     REQUIRE(buffer == load_data("message/data-message"));
 }
 
+// Closing a submessage with known size must reset the parent writer so it
+// can be written to again.
+TEST_CASE("write message field with reserved size followed by another field") {
+    std::string buffer;
+    protozero::pbf_writer pbf_test{buffer};
+
+    {
+        protozero::pbf_writer pbf_submessage{pbf_test, 1, 2};
+        pbf_submessage.add_uint32(1, 5);
+    }
+    pbf_test.add_uint32(2, 5);
+
+    // field 1, length 2, containing field 1 = 5; then field 2 = 5
+    REQUIRE(buffer == std::string{"\x0a\x02\x08\x05\x10\x05", 6});
+}
+
 TEST_CASE("write message field into non-empty buffer") {
     std::string buffer{"some data already in here"};
     protozero::pbf_writer pbf_test{buffer};

test/t/repeated_packed_fixed32/reader_test_cases.cpp

@@ -1,4 +1,3 @@
-
 #include <test.hpp>
 
 #define PBF_TYPE fixed32
@@ -7,3 +6,63 @@ using cpp_type = uint32_t;
 
 #include <packed_access.hpp>
 
+// Closing a packed field with known size must reset the parent writer so it
+// can be written to again.
+TEST_CASE("write repeated packed fixed32 field using packed field with known size") {
+    std::string buffer;
+    protozero::pbf_writer pw{buffer};
+
+    // field 1, length 8, fixed32 values 17 and 1
+    const std::string packed{"\x0a\x08\x11\x00\x00\x00\x01\x00\x00\x00", 10};
+    // field 2, varint 5
+    const std::string scalar{"\x10\x05", 2};
+
+    SECTION("closed by destructor, then add field to parent") {
+        {
+            protozero::packed_field_fixed32 field{pw, 1, 2};
+            field.add_element(17);
+            field.add_element(1);
+        }
+        pw.add_uint32(2, 5);
+
+        REQUIRE(buffer == packed + scalar);
+    }
+
+    SECTION("closed by commit(), then add field to parent") {
+        protozero::packed_field_fixed32 field{pw, 1, 2};
+        field.add_element(17);
+        field.add_element(1);
+        field.commit();
+        pw.add_uint32(2, 5);
+
+        REQUIRE(buffer == packed + scalar);
+    }
+
+    SECTION("two packed fields in a row") {
+        {
+            protozero::packed_field_fixed32 field{pw, 1, 2};
+            field.add_element(17);
+            field.add_element(1);
+        }
+        {
+            protozero::packed_field_fixed32 field{pw, 1, 2};
+            field.add_element(17);
+            field.add_element(1);
+        }
+
+        REQUIRE(buffer == packed + packed);
+    }
+
+    SECTION("inside a submessage that is committed afterwards") {
+        protozero::pbf_writer sw{pw, 3};
+        {
+            protozero::packed_field_fixed32 field{sw, 1, 2};
+            field.add_element(17);
+            field.add_element(1);
+        }
+        sw.commit();
+
+        // field 3, length 10, containing the packed field
+        REQUIRE(buffer == std::string{"\x1a\x0a", 2} + packed);
+    }
+}