From 9d0fbebf3ae5598d9a3c9a3c2fac45f0b5145fce Mon Sep 17 00:00:00 2001 From: dsavvinov Date: Wed, 10 Aug 2016 18:40:27 +0300 Subject: [PATCH] Protobuf: fixed bug in desrialization of arrays --- .../google/protobuf/compiler/kotlin/Makefile | 21 ++++--- .../kotlin/src/kotlin_class_generator.cc | 4 +- .../kotlin/src/kotlin_class_generator.h | 8 +-- .../kotlin/src/kotlin_field_generator.cc | 56 ++++++++----------- .../kotlin/src/kotlin_file_generator.cc | 3 +- .../kotlin/src/kotlin_file_generator.h | 5 +- .../protobuf/compiler/kotlin/src/main.cc | 4 +- proto/pre-build.sh | 11 ++-- 8 files changed, 57 insertions(+), 55 deletions(-) diff --git a/proto/compiler/google/src/google/protobuf/compiler/kotlin/Makefile b/proto/compiler/google/src/google/protobuf/compiler/kotlin/Makefile index 273b3c1e303..f1b52e3440d 100644 --- a/proto/compiler/google/src/google/protobuf/compiler/kotlin/Makefile +++ b/proto/compiler/google/src/google/protobuf/compiler/kotlin/Makefile @@ -6,14 +6,17 @@ EXE = protoc SRCDIR = src BINDIR = bin TESTDIR = test +KTPROJECT = protoc-test-project +KTOUT = $(KTPROJECT)/src OBJECTS = $(patsubst $(SRCDIR)/%.cc,$(BINDIR)/%.o,$(wildcard $(SRCDIR)/*.cc)) +KTARTIFACTS = $(patsubst $(TESTDIR)/%.proto,$(KTOUT)/%.kt,$(wildcard $(TESTDIR)/*.proto)) all: $(EXE) $(EXE): $(BINDIR) $(OBJECTS) $(CXX) $(OBJECTS) -o $(EXE) $(LDFLAGS) - + $(BINDIR)/%.o: $(SRCDIR)/%.cc $(CXX) $(CXXFLAGS) -c -MMD -o $@ $< @@ -22,16 +25,18 @@ include $(wildcard $(BINDIR)/*.d) $(BINDIR): mkdir -p $(BINDIR) +$(KTOUT): + mkdir -p $(KTOUT) + clean: rm -rf $(BINDIR) rm $(EXE) + rm -rf $(KTPROJECT) + +$(KTOUT)/%.kt: $(TESTDIR)/%.proto $(EXE) + ./protoc --kotlin_out=$(KTOUT) $< + +generate: $(KTOUT) $(KTARTIFACTS) -generate: - ./protoc --kotlin_out=$$HOME/Downloads/carkot/proto/compiler/protoc-artifacts ./test/connect.proto - ./protoc --kotlin_out=$$HOME/Downloads/carkot/proto/compiler/protoc-artifacts ./test/direction.proto - ./protoc --kotlin_out=$$HOME/Downloads/carkot/proto/compiler/protoc-artifacts ./test/location.proto - ./protoc --kotlin_out=$$HOME/Downloads/carkot/proto/compiler/protoc-artifacts ./test/route_done.proto - ./protoc --kotlin_out=$$HOME/Downloads/carkot/proto/compiler/protoc-artifacts ./test/route.proto - ./protoc --kotlin_out=$$HOME/Downloads/carkot/proto/compiler/protoc-artifacts ./test/rc_session.proto .PHONY: clean all generate diff --git a/proto/compiler/google/src/google/protobuf/compiler/kotlin/src/kotlin_class_generator.cc b/proto/compiler/google/src/google/protobuf/compiler/kotlin/src/kotlin_class_generator.cc index 40996a01d87..a1d09b3093c 100644 --- a/proto/compiler/google/src/google/protobuf/compiler/kotlin/src/kotlin_class_generator.cc +++ b/proto/compiler/google/src/google/protobuf/compiler/kotlin/src/kotlin_class_generator.cc @@ -224,7 +224,7 @@ void ClassGenerator::generateHeader(io::Printer * printer, bool isBuilder) const // build list of arguments like 'field1: Type1, field2: Type2, ... ' string argumentList = ""; for (int i = 0; i < properties.size(); ++i) { - argumentList += properties[i]->simpleName + ": " + properties[i]->getFullType(); + argumentList += "var " + properties[i]->simpleName + ": " + properties[i]->getFullType(); if (i + 1 != properties.size()) { argumentList += ", "; } @@ -315,7 +315,7 @@ void ClassGenerator::generateParseMethods(io::Printer *printer) const { // check that wire type of that field is equal to expected printer->Print(vars, "if (wireType != $kotlinWireType$) { errorCode = 1; return false } \n"); - properties[i]->generateSerializationCode(printer, /* isRead = */ true, /* noTag = */ true); + properties[i]->generateSerializationCode(printer, /* isRead = */ true, /* noTag = */ true, /* isField = */ false); printer->Outdent(); printer->Print("}\n"); diff --git a/proto/compiler/google/src/google/protobuf/compiler/kotlin/src/kotlin_class_generator.h b/proto/compiler/google/src/google/protobuf/compiler/kotlin/src/kotlin_class_generator.h index 4d3d87756d5..8f4253d60f3 100644 --- a/proto/compiler/google/src/google/protobuf/compiler/kotlin/src/kotlin_class_generator.h +++ b/proto/compiler/google/src/google/protobuf/compiler/kotlin/src/kotlin_class_generator.h @@ -21,7 +21,7 @@ class FieldGenerator; // declared in "kotlin_file_generator.h" class NameResolver; // declared in "kotlin_name_resolver.h" class EnumGenerator; // declared in "kotlin_enum_generator.h" -class ClassGenerator { +class ClassGenerator { // TODO ProtoClass public: string getSimpleType() const; string getFullType() const; @@ -30,12 +30,12 @@ public: string getBuilderInitValue() const; vector properties; - vector classesDeclarations; - vector enumsDeclaraions; + vector classesDeclarations; // TODO nestedClasses + vector enumsDeclaraions; // TODO ProtoEnum ClassGenerator (Descriptor const * descriptor, NameResolver * nameResolver); ~ClassGenerator (); - + // KtClassGenerator(ProtoClass)::generate() void generateCode (io::Printer * printer, bool isBuilder = false) const; private: /** diff --git a/proto/compiler/google/src/google/protobuf/compiler/kotlin/src/kotlin_field_generator.cc b/proto/compiler/google/src/google/protobuf/compiler/kotlin/src/kotlin_field_generator.cc index 4bb3bd8be6c..be459e068bb 100644 --- a/proto/compiler/google/src/google/protobuf/compiler/kotlin/src/kotlin_field_generator.cc +++ b/proto/compiler/google/src/google/protobuf/compiler/kotlin/src/kotlin_field_generator.cc @@ -50,7 +50,7 @@ void FieldGenerator::generateCode(io::Printer *printer, bool isBuilder) const { generateComment(printer); - printer->Print(vars, "var $name$ : $field$\n"); +// printer->Print(vars, "var $name$ : $field$\n"); // generate setter for builder if (isBuilder) { @@ -81,20 +81,13 @@ void FieldGenerator::generateSerializationForPacked(io::Printer *printer, bool i bool isPrimitive = descriptor->type() != FieldDescriptor::TYPE_BYTES && descriptor->type() != FieldDescriptor::TYPE_MESSAGE && - descriptor->type() != FieldDescriptor::TYPE_STRING && - descriptor->type() != FieldDescriptor::TYPE_ENUM; - + descriptor->type() != FieldDescriptor::TYPE_STRING; if (isRead) { if (!noTag) { printer->Print(vars, "val tag = input.readTag($fieldNumber$, WireType.LENGTH_DELIMITED)\n"); } printer->Print(vars, "val expectedByteSize = input.readInt32NoTag()\n"); - /* Now we want to compute element size of array. For this, we are estimating byte size of a single element, - * and then divide byte size of whole array by byte size of single element. - */ - printer->Print(vars, "var singleElemSize = 0\n"); - /* hack: copy current FieldGenerator and change label to OPTIONAL. Also change name to name of iterator in for-loop. This will allow to re-use this function for generating serialization code for elements of array. @@ -105,38 +98,36 @@ void FieldGenerator::generateSerializationForPacked(io::Printer *printer, bool i (then writing CodedOutputStream.writeMessage will be possible). */ FieldGenerator singleFieldGen = getUnderlyingTypeGenerator(); - singleFieldGen.simpleName = "singleElemSize"; - printer->Print("\n"); - singleFieldGen.generateSerializationCode(printer, isRead, /* noTag = */ true, /* isField = */ false); - printer->Print("\n"); - - printer->Print(vars, "val arraySize = expectedByteSize / singleElemSize\n"); - - // Allocate new array of estimated size - printer->Print(vars, "val newArray = $arrayType$(arraySize)\n"); + // Allocate new array and current size + printer->Print(vars, "var newArray = $arrayType$(0)\n"); + printer->Print(vars, "var readSize = 0\n"); // place declaration of new variable in anonymous scope for hygiene - printer->Print("run {\n"); + printer->Print("do {\n"); printer->Indent(); printer->Print("var i = 0\n"); - printer->Print(vars, "while(i < arraySize) {\n"); + printer->Print(vars, "while(readSize < expectedByteSize) {\n"); printer->Indent(); - printer->Print(vars, "var tmp: $underlyingType$ = $initValue$\n"); - singleFieldGen.simpleName = "tmp"; + printer->Print(vars, "var tmp = $arrayType$(1)\n"); + singleFieldGen.simpleName = "tmp[0]"; singleFieldGen.protoLabel = FieldDescriptor::LABEL_OPTIONAL; singleFieldGen.generateSerializationCode(printer, isRead, /* noTag = */ true, /* isField = */ false); - printer->Print(vars, "$fieldName$[i] = tmp\n"); - printer->Print("i += 1\n"); + printer->Print(vars, "newArray = newArray.plus(tmp)\n"); + // add size of read element to readSize + singleFieldGen.generateSizeEstimationCode(printer, "readSize", /* noTag = */ true, /* isField = */ false); printer->Outdent(); // while-loop printer->Print("}\n"); + // finaly, assign new array to our field + printer->Print(vars, "$fieldName$ = newArray\n"); + printer->Outdent(); // anonymous scope - printer->Print("}\n"); + printer->Print("} while (false)\n"); } else { /** @@ -159,7 +150,7 @@ void FieldGenerator::generateSerializationForPacked(io::Printer *printer, bool i // all elements // place declaration of new variable in anonymous scope for hygiene - printer->Print("run {\n"); + printer->Print("do {\n"); printer->Indent(); printer->Print("var i = 0\n"); @@ -179,7 +170,7 @@ void FieldGenerator::generateSerializationForPacked(io::Printer *printer, bool i printer->Print("}\n"); printer->Outdent(); // anonymous scope - printer->Print("}\n"); + printer->Print("} while(false)\n"); } } @@ -219,7 +210,7 @@ void FieldGenerator::generateSerializationForMessages(io::Printer * printer, boo if (isRead) { // We will create some temporary variables // So we place following code into separate block for the sake of hygiene - printer->Print("run {\n"); + printer->Print("do {\n"); printer->Indent(); // read tag @@ -239,8 +230,9 @@ void FieldGenerator::generateSerializationForMessages(io::Printer * printer, boo // check that actual size equal to expected size printer->Print(vars, "if (expectedSize != $fieldName$.getSizeNoTag()) { errorCode = 3; return false }\n"); + printer->Outdent(); - printer->Print("}\n"); + printer->Print("} while(false) \n"); } else { // write tag @@ -394,7 +386,7 @@ void FieldGenerator::generateSizeEstimationCode(io::Printer *printer, string var // We will need total byte size of array, because that size is itself a part of the message and // adds to total message size. // For the sake of hygiene, temporary variables are created in anonymous scope - printer->Print("run {\n"); + printer->Print("do {\n"); printer->Indent(); // Create a temporary variable that will collect array byte size @@ -414,14 +406,14 @@ void FieldGenerator::generateSizeEstimationCode(io::Printer *printer, string var printer->Print(vars, "i += 1\n"); printer->Outdent(); // for-loop - printer->Print("}\n"); + printer->Print("} \n"); // now add size of array to total message size: printer->Print(vars, "$varName$ += arraySize"); // actual array size printer->Print("\n"); printer->Outdent(); // anonymous scope - printer->Print("}\n"); + printer->Print("} while(false)\n"); printer->Outdent(); // if-clause printer->Print("}\n"); diff --git a/proto/compiler/google/src/google/protobuf/compiler/kotlin/src/kotlin_file_generator.cc b/proto/compiler/google/src/google/protobuf/compiler/kotlin/src/kotlin_file_generator.cc index 9dcf0b15228..c5e3a8c6587 100644 --- a/proto/compiler/google/src/google/protobuf/compiler/kotlin/src/kotlin_file_generator.cc +++ b/proto/compiler/google/src/google/protobuf/compiler/kotlin/src/kotlin_file_generator.cc @@ -17,13 +17,14 @@ namespace protobuf { namespace compiler { namespace kotlin { -void FileGenerator::generateCode(io::Printer *printer, std::vector & classes) const { +void FileGenerator::generateCode(io::Printer *printer, std::vector & classes) { for (int i = 0; i < classes.size(); ++i) { classes[i]->generateCode(printer); printer->Print("\n\n"); } } +// Extract more methods or inline generateCode() to have common style in this method bool FileGenerator::Generate(const FileDescriptor *file, const string ¶meter, GeneratorContext *context, string *error) const { std::vector classes; diff --git a/proto/compiler/google/src/google/protobuf/compiler/kotlin/src/kotlin_file_generator.h b/proto/compiler/google/src/google/protobuf/compiler/kotlin/src/kotlin_file_generator.h index 37d3ab21110..e2cc1f039d8 100644 --- a/proto/compiler/google/src/google/protobuf/compiler/kotlin/src/kotlin_file_generator.h +++ b/proto/compiler/google/src/google/protobuf/compiler/kotlin/src/kotlin_file_generator.h @@ -21,13 +21,14 @@ public: FileGenerator(); ~FileGenerator(); - void generateCode(io::Printer *, std::vector &) const; - // implements CodeGenerator ---------------------------------------- bool Generate(const FileDescriptor* file, const string& parameter, GeneratorContext* context, string* error) const; +private: + // TODO check code style + static void generateCode(io::Printer *, std::vector &); }; } // namespace kotlin diff --git a/proto/compiler/google/src/google/protobuf/compiler/kotlin/src/main.cc b/proto/compiler/google/src/google/protobuf/compiler/kotlin/src/main.cc index 54686c7f2c9..b78d9771bc6 100644 --- a/proto/compiler/google/src/google/protobuf/compiler/kotlin/src/main.cc +++ b/proto/compiler/google/src/google/protobuf/compiler/kotlin/src/main.cc @@ -10,6 +10,8 @@ #include "kotlin_file_generator.h" #include +// TODO attacch Kotlin generator to protoc as plugin. +// Don't rewrite protoc main. int main(int argc, const char* const * argv) { google::protobuf::compiler::CommandLineInterface cli; cli.AllowPlugins("protoc-"); @@ -65,7 +67,7 @@ int main(int argc, const char* const * argv) { google::protobuf::compiler::kotlin::FileGenerator kotlinGenerator; cli.RegisterGenerator("--kotlin_out", &kotlinGenerator, "Generate Kotlin source file."); - + return cli.Run(argc, argv); } \ No newline at end of file diff --git a/proto/pre-build.sh b/proto/pre-build.sh index ce910301ab7..0b0c4026a28 100755 --- a/proto/pre-build.sh +++ b/proto/pre-build.sh @@ -16,10 +16,10 @@ if [ ! -e "$lprotobuf" ]; then fi if [ "$ok" == 0 ]; then - echo "$missing, required for building, is not found" - echo "You have to build and install latest Protobuf version from https://github.com/google/protobuf" - echo "Do you want to do it automatically? [y/n]" - echo "(Caution: this will take decent amount of time and internet traffic)" + echo "$missing, required for building, is not found" \ +"You have to build and install latest Protobuf version from https://github.com/google/protobuf" \ +"Do you want to do it automatically? [y/n]" \ +"(Caution: this will take decent amount of time and internet traffic)" response="n" read response if [ $response = "y" ]; then @@ -42,9 +42,10 @@ if [ "$ok" == 0 ]; then make echo "Installing" - sudo make install + #sudo make install echo "Cleaning" + cd ../.. rm -rf protobuf_sources fi else