Fix deparsing of VL field with length 0 (#283)

This edge case had not been tested previously.
This fixes issue #282.

src/bm_sim/extract.h

@@ -68,6 +68,8 @@ static inline void generic_extract(const char *data, int bit_offset,
 
 static inline void generic_deparse(const char *data, int bitwidth,
                                    char *dst, int hdr_offset) {
+  if (bitwidth == 0) return;
+
   int nbytes = (bitwidth + 7) / 8;
 
   if (hdr_offset == 0 && bitwidth % 8 == 0) {

src/bm_sim/fields.cpp

@@ -41,7 +41,11 @@ int Field::extract_VL(const char *data, int hdr_offset, int computed_nbits) {
 }
 
 int Field::deparse(char *data, int hdr_offset) const {
-  extract::generic_deparse(&bytes[0], nbits, data, hdr_offset);
+  // this does not work for empty variable-length fields, as we assert in the
+  // ByteContainer's [] operator. The right thing to do would probably be to add
+  // a at() method to ByteContainer and not perform any check in [].
+  // extract::generic_deparse(&bytes[0], nbits, data, hdr_offset);
+  extract::generic_deparse(bytes.data(), nbits, data, hdr_offset);
   return nbits;
 }
 

tests/test_parser.cpp

@@ -1096,9 +1096,12 @@ TEST_F(IPv4TLVParsingTest, BothOptions) {
 }
 
 
+using ::testing::TestWithParam;
+using ::testing::Range;
+
 // Google Test fixture for IPv4 Variable Length parsing test
 // This test parses the options as one VL field
-class IPv4VLParsingTest : public ::testing::Test {
+class IPv4VLParsingTest : public TestWithParam<int> {
  protected:
   PHVFactory phv_factory;
 
@@ -1218,53 +1221,32 @@ class IPv4VLParsingTest : public ::testing::Test {
     ASSERT_EQ(v, f_options.get_bytes());
   }
 
-  template<size_t OptionWords>
-  void test() {
-    const ByteContainer buf = get_ipv4_bytes(OptionWords);
-    Packet packet = get_pkt(buf);
-    const PHV &phv = *packet.get_phv();
-
-    parser.parse(&packet);
-
-    check_base(phv, OptionWords);
-    ByteContainer expected_value = option_value(OptionWords);
-    check_option(phv, OptionWords, expected_value);
-  }
-
   // virtual void TearDown() { }
 };
 
-TEST_F(IPv4VLParsingTest, NoOption) {
-  test<0u>();
-}
-
-TEST_F(IPv4VLParsingTest, SmallOption) {
-  test<3u>();
-}
+TEST_P(IPv4VLParsingTest, ParseAndDeparse) {
+  const auto OptionWords = GetParam();
 
-TEST_F(IPv4VLParsingTest, BigOption) {
-  test<9u>();  // max value
-}
-
-TEST_F(IPv4VLParsingTest, Deparser) {
-  const size_t option_words = 4;
-  const ByteContainer buf = get_ipv4_bytes(option_words);
+  const ByteContainer buf = get_ipv4_bytes(OptionWords);
   const ByteContainer buf_save = buf;
   Packet packet = get_pkt(buf);
   const PHV &phv = *packet.get_phv();
 
   parser.parse(&packet);
 
-  check_base(phv, option_words);
-  ByteContainer expected_value = option_value(option_words);
-  check_option(phv, option_words, expected_value);
+  check_base(phv, OptionWords);
+  ByteContainer expected_value = option_value(OptionWords);
+  check_option(phv, OptionWords, expected_value);
 
   deparser.deparse(&packet);
 
   ASSERT_EQ(buf_save.size(), packet.get_data_size());
   ASSERT_EQ(0, memcmp(buf_save.data(), packet.data(), buf_save.size()));
 }
 
+INSTANTIATE_TEST_CASE_P(IPv4VLOptionWords, IPv4VLParsingTest,
+                        Range(0, 10));
+
 
 class ParseVSetTest : public ::testing::Test {
  protected: