From 1c277f9c68b81b594701d0c5cd90e74170bbab21 Mon Sep 17 00:00:00 2001
From: Ulas Kirazci <ulas@google.com>
Date: Mon, 29 Jul 2013 11:08:34 -0700
Subject: [PATCH] Fixed packed repeated serialization.

Remove buggy memoization. Memoization also is too fragile for the api
because the repeated field is public.

Change-Id: I538b8426d274b22df2eeea5935023abbe7df49fe
---
 .../java/com/google/protobuf/NanoTest.java    | 19 +++++++
 .../javanano/javanano_primitive_field.cc      | 57 +++++++++----------
 .../javanano/javanano_primitive_field.h       |  2 +
 3 files changed, 48 insertions(+), 30 deletions(-)

diff --git a/java/src/test/java/com/google/protobuf/NanoTest.java b/java/src/test/java/com/google/protobuf/NanoTest.java
index 0ea80d400f..edb0a208cb 100644
--- a/java/src/test/java/com/google/protobuf/NanoTest.java
+++ b/java/src/test/java/com/google/protobuf/NanoTest.java
@@ -46,6 +46,7 @@ import com.google.protobuf.nano.UnittestImportNano;
 import junit.framework.TestCase;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 
 /**
@@ -1955,6 +1956,24 @@ public class NanoTest extends TestCase {
     assertEquals(TestAllTypesNano.BAR, msg.repeatedPackedNestedEnum[1]);
   }
 
+  public void testNanoRepeatedPackedSerializedSize() throws Exception {
+    TestAllTypesNano msg = new TestAllTypesNano();
+    msg.repeatedPackedInt32 = new int[] { 123, 789, 456 };
+    int msgSerializedSize = msg.getSerializedSize();
+    byte [] result = MessageNano.toByteArray(msg);
+    //System.out.printf("mss=%d result.length=%d\n", msgSerializedSize, result.length);
+    assertTrue(msgSerializedSize == 11);
+    assertEquals(result.length, msgSerializedSize);
+    TestAllTypesNano msg2 = new TestAllTypesNano();
+    msg2.repeatedPackedInt32 = new int[] { 123, 789, 456 };
+    byte [] result2 = new byte[msgSerializedSize];
+    MessageNano.toByteArray(msg2, result2, 0, msgSerializedSize);
+
+    // Check equal size and content.
+    assertEquals(msgSerializedSize, msg2.getSerializedSize());
+    assertTrue(Arrays.equals(result, result2));
+  }
+
   public void testNanoRepeatedInt32ReMerge() throws Exception {
     TestAllTypesNano msg = new TestAllTypesNano();
     msg.repeatedInt32 = new int[] { 234 };
diff --git a/src/google/protobuf/compiler/javanano/javanano_primitive_field.cc b/src/google/protobuf/compiler/javanano/javanano_primitive_field.cc
index 987a1037f4..4aba262d1a 100644
--- a/src/google/protobuf/compiler/javanano/javanano_primitive_field.cc
+++ b/src/google/protobuf/compiler/javanano/javanano_primitive_field.cc
@@ -401,10 +401,6 @@ void RepeatedPrimitiveFieldGenerator::
 GenerateMembers(io::Printer* printer) const {
   printer->Print(variables_,
     "public $type$[] $name$ = $default$;\n");
-  if (descriptor_->options().packed()) {
-    printer->Print(variables_,
-      "private int $name$MemoizedSerializedSize;\n");
-  }
 }
 
 void RepeatedPrimitiveFieldGenerator::
@@ -453,13 +449,34 @@ GenerateParsingCode(io::Printer* printer) const {
   }
 }
 
+void RepeatedPrimitiveFieldGenerator::
+GenerateRepeatedDataSizeCode(io::Printer* printer) const {
+  // Creates a variable dataSize and puts the serialized size in
+  // there.
+  if (FixedSize(descriptor_->type()) == -1) {
+    printer->Print(variables_,
+      "int dataSize = 0;\n"
+      "for ($type$ element : this.$name$) {\n"
+      "  dataSize += com.google.protobuf.nano.CodedOutputByteBufferNano\n"
+      "    .compute$capitalized_type$SizeNoTag(element);\n"
+      "}\n");
+  } else {
+    printer->Print(variables_,
+      "int dataSize = $fixed_size$ * this.$name$.length;\n");
+  }
+}
+
 void RepeatedPrimitiveFieldGenerator::
 GenerateSerializationCode(io::Printer* printer) const {
   if (descriptor_->options().packed()) {
     printer->Print(variables_,
-      "if (this.$name$.length > 0) {\n"
+      "if (this.$name$.length > 0) {\n");
+    printer->Indent();
+    GenerateRepeatedDataSizeCode(printer);
+    printer->Outdent();
+    printer->Print(variables_,
       "  output.writeRawVarint32($tag$);\n"
-      "  output.writeRawVarint32($name$MemoizedSerializedSize);\n"
+      "  output.writeRawVarint32(dataSize);\n"
       "}\n");
     printer->Print(variables_,
       "for ($type$ element : this.$name$) {\n"
@@ -479,27 +496,15 @@ GenerateSerializedSizeCode(io::Printer* printer) const {
     "if (this.$name$.length > 0) {\n");
   printer->Indent();
 
-  if (FixedSize(descriptor_->type()) == -1) {
-    printer->Print(variables_,
-      "int dataSize = 0;\n"
-      "for ($type$ element : this.$name$) {\n"
-      "  dataSize += com.google.protobuf.nano.CodedOutputByteBufferNano\n"
-      "    .compute$capitalized_type$SizeNoTag(element);\n"
-      "}\n");
-  } else {
-    printer->Print(variables_,
-      "int dataSize = $fixed_size$ * this.$name$.length;\n");
-  }
+  GenerateRepeatedDataSizeCode(printer);
 
   printer->Print(
     "size += dataSize;\n");
   if (descriptor_->options().packed()) {
-    // cache the data size for packed fields.
     printer->Print(variables_,
       "size += $tag_size$;\n"
       "size += com.google.protobuf.nano.CodedOutputByteBufferNano\n"
-      "  .computeRawVarint32Size(dataSize);\n"
-      "$name$MemoizedSerializedSize = dataSize;\n");
+      "  .computeRawVarint32Size(dataSize);\n");
   } else {
     printer->Print(variables_,
         "size += $tag_size$ * this.$name$.length;\n");
@@ -507,16 +512,8 @@ GenerateSerializedSizeCode(io::Printer* printer) const {
 
   printer->Outdent();
 
-  // set cached size to 0 for empty packed fields.
-  if (descriptor_->options().packed()) {
-    printer->Print(variables_,
-      "} else {\n"
-      "  $name$MemoizedSerializedSize = 0;\n"
-      "}\n");
-  } else {
-    printer->Print(
-      "}\n");
-  }
+  printer->Print(
+    "}\n");
 }
 
 string RepeatedPrimitiveFieldGenerator::GetBoxedType() const {
diff --git a/src/google/protobuf/compiler/javanano/javanano_primitive_field.h b/src/google/protobuf/compiler/javanano/javanano_primitive_field.h
index 9b1bb46533..39245393ec 100644
--- a/src/google/protobuf/compiler/javanano/javanano_primitive_field.h
+++ b/src/google/protobuf/compiler/javanano/javanano_primitive_field.h
@@ -78,6 +78,8 @@ class RepeatedPrimitiveFieldGenerator : public FieldGenerator {
   string GetBoxedType() const;
 
  private:
+  void GenerateRepeatedDataSizeCode(io::Printer* printer) const;
+
   const FieldDescriptor* descriptor_;
   map<string, string> variables_;