From 3b3c8abb9635eb3ea078a821a99c9ef29d66dff7 Mon Sep 17 00:00:00 2001 From: Jisi Liu Date: Wed, 30 Mar 2016 11:39:59 -0700 Subject: Integrate google internal changes. --- src/google/protobuf/io/coded_stream.cc | 10 +- src/google/protobuf/io/coded_stream_unittest.cc | 1 + src/google/protobuf/io/gzip_stream.h | 3 +- src/google/protobuf/io/printer.cc | 76 ++++++- src/google/protobuf/io/printer.h | 164 ++++++++++++++ src/google/protobuf/io/printer_unittest.cc | 237 +++++++++++++++++++++ src/google/protobuf/io/tokenizer.cc | 8 +- src/google/protobuf/io/tokenizer.h | 20 +- src/google/protobuf/io/tokenizer_unittest.cc | 18 +- .../protobuf/io/zero_copy_stream_impl_lite.cc | 20 +- .../protobuf/io/zero_copy_stream_impl_lite.h | 1 + 11 files changed, 513 insertions(+), 45 deletions(-) (limited to 'src/google/protobuf/io') diff --git a/src/google/protobuf/io/coded_stream.cc b/src/google/protobuf/io/coded_stream.cc index e3a34d0a..d8354c1f 100644 --- a/src/google/protobuf/io/coded_stream.cc +++ b/src/google/protobuf/io/coded_stream.cc @@ -105,7 +105,7 @@ void CodedInputStream::BackUpInputToCurrentPosition() { inline void CodedInputStream::RecomputeBufferLimits() { buffer_end_ += buffer_size_after_limit_; - int closest_limit = min(current_limit_, total_bytes_limit_); + int closest_limit = std::min(current_limit_, total_bytes_limit_); if (closest_limit < total_bytes_read_) { // The limit position is in the current buffer. We must adjust // the buffer size accordingly. @@ -135,7 +135,7 @@ CodedInputStream::Limit CodedInputStream::PushLimit(int byte_limit) { // We need to enforce all limits, not just the new one, so if the previous // limit was before the new requested limit, we continue to enforce the // previous limit. - current_limit_ = min(current_limit_, old_limit); + current_limit_ = std::min(current_limit_, old_limit); RecomputeBufferLimits(); return old_limit; @@ -188,7 +188,7 @@ void CodedInputStream::SetTotalBytesLimit( // Make sure the limit isn't already past, since this could confuse other // code. int current_position = CurrentPosition(); - total_bytes_limit_ = max(current_position, total_bytes_limit); + total_bytes_limit_ = std::max(current_position, total_bytes_limit); if (warning_threshold >= 0) { total_bytes_warning_threshold_ = warning_threshold; } else { @@ -233,7 +233,7 @@ bool CodedInputStream::Skip(int count) { buffer_end_ = buffer_; // Make sure this skip doesn't try to skip past the current limit. - int closest_limit = min(current_limit_, total_bytes_limit_); + int closest_limit = std::min(current_limit_, total_bytes_limit_); int bytes_until_limit = closest_limit - total_bytes_read_; if (bytes_until_limit < count) { // We hit the limit. Skip up to it then fail. @@ -270,7 +270,7 @@ bool CodedInputStream::ReadStringFallback(string* buffer, int size) { buffer->clear(); } - int closest_limit = min(current_limit_, total_bytes_limit_); + int closest_limit = std::min(current_limit_, total_bytes_limit_); if (closest_limit != INT_MAX) { int bytes_to_limit = closest_limit - CurrentPosition(); if (bytes_to_limit > 0 && size > 0 && size <= bytes_to_limit) { diff --git a/src/google/protobuf/io/coded_stream_unittest.cc b/src/google/protobuf/io/coded_stream_unittest.cc index d1782e39..f10d4670 100644 --- a/src/google/protobuf/io/coded_stream_unittest.cc +++ b/src/google/protobuf/io/coded_stream_unittest.cc @@ -46,6 +46,7 @@ #include #include +#include #include #include #include diff --git a/src/google/protobuf/io/gzip_stream.h b/src/google/protobuf/io/gzip_stream.h index 82445000..15b02fe3 100644 --- a/src/google/protobuf/io/gzip_stream.h +++ b/src/google/protobuf/io/gzip_stream.h @@ -43,10 +43,9 @@ #ifndef GOOGLE_PROTOBUF_IO_GZIP_STREAM_H__ #define GOOGLE_PROTOBUF_IO_GZIP_STREAM_H__ -#include - #include #include +#include namespace google { namespace protobuf { diff --git a/src/google/protobuf/io/printer.cc b/src/google/protobuf/io/printer.cc index 7d886506..7532b098 100644 --- a/src/google/protobuf/io/printer.cc +++ b/src/google/protobuf/io/printer.cc @@ -42,13 +42,25 @@ namespace protobuf { namespace io { Printer::Printer(ZeroCopyOutputStream* output, char variable_delimiter) - : variable_delimiter_(variable_delimiter), - output_(output), - buffer_(NULL), - buffer_size_(0), - at_start_of_line_(true), - failed_(false) { -} + : variable_delimiter_(variable_delimiter), + output_(output), + buffer_(NULL), + buffer_size_(0), + offset_(0), + at_start_of_line_(true), + failed_(false), + annotation_collector_(NULL) {} + +Printer::Printer(ZeroCopyOutputStream* output, char variable_delimiter, + AnnotationCollector* annotation_collector) + : variable_delimiter_(variable_delimiter), + output_(output), + buffer_(NULL), + buffer_size_(0), + offset_(0), + at_start_of_line_(true), + failed_(false), + annotation_collector_(annotation_collector) {} Printer::~Printer() { // Only BackUp() if we have called Next() at least once and never failed. @@ -57,9 +69,47 @@ Printer::~Printer() { } } +bool Printer::GetSubstitutionRange(const char* varname, + pair* range) { + map >::const_iterator iter = + substitutions_.find(varname); + if (iter == substitutions_.end()) { + GOOGLE_LOG(DFATAL) << " Undefined variable in annotation: " << varname; + return false; + } + if (iter->second.first > iter->second.second) { + GOOGLE_LOG(DFATAL) << " Variable used for annotation used multiple times: " + << varname; + return false; + } + *range = iter->second; + return true; +} + +void Printer::Annotate(const char* begin_varname, const char* end_varname, + const string& file_path, const vector& path) { + if (annotation_collector_ == NULL) { + // Can't generate signatures with this Printer. + return; + } + pair begin, end; + if (!GetSubstitutionRange(begin_varname, &begin) || + !GetSubstitutionRange(end_varname, &end)) { + return; + } + if (begin.first > end.second) { + GOOGLE_LOG(DFATAL) << " Annotation has negative length from " << begin_varname + << " to " << end_varname; + } else { + annotation_collector_->AddAnnotation(begin.first, end.second, file_path, + path); + } +} + void Printer::Print(const map& variables, const char* text) { int size = strlen(text); int pos = 0; // The number of bytes we've written so far. + substitutions_.clear(); for (int i = 0; i < size; i++) { if (text[i] == '\n') { @@ -97,7 +147,17 @@ void Printer::Print(const map& variables, const char* text) { if (iter == variables.end()) { GOOGLE_LOG(DFATAL) << " Undefined variable: " << varname; } else { + size_t begin = offset_; WriteRaw(iter->second.data(), iter->second.size()); + pair >::iterator, bool> inserted = + substitutions_.insert( + std::make_pair(varname, std::make_pair(begin, offset_))); + if (!inserted.second) { + // This variable was used multiple times. Make its span have + // negative length so we can detect it if it gets used in an + // annotation. + inserted.first->second = std::make_pair(1, 0); + } } } @@ -265,6 +325,7 @@ void Printer::WriteRaw(const char* data, int size) { // Data exceeds space in the buffer. Copy what we can and request a // new buffer. memcpy(buffer_, data, buffer_size_); + offset_ += buffer_size_; data += buffer_size_; size -= buffer_size_; void* void_buffer; @@ -277,6 +338,7 @@ void Printer::WriteRaw(const char* data, int size) { memcpy(buffer_, data, size); buffer_ += size; buffer_size_ -= size; + offset_ += size; } } // namespace io diff --git a/src/google/protobuf/io/printer.h b/src/google/protobuf/io/printer.h index f1490bbe..2ba84559 100644 --- a/src/google/protobuf/io/printer.h +++ b/src/google/protobuf/io/printer.h @@ -39,6 +39,7 @@ #include #include +#include #include namespace google { @@ -47,6 +48,47 @@ namespace io { class ZeroCopyOutputStream; // zero_copy_stream.h +// Records annotations about a Printer's output. +class LIBPROTOBUF_EXPORT AnnotationCollector { + public: + // Records that the bytes in file_path beginning with begin_offset and ending + // before end_offset are associated with the SourceCodeInfo-style path. + virtual void AddAnnotation(size_t begin_offset, size_t end_offset, + const string& file_path, + const vector& path) = 0; + + virtual ~AnnotationCollector() {} +}; + +// Records annotations about a Printer's output to the given protocol buffer, +// assuming that the buffer has an ::Annotation message exposing path, +// source_file, begin and end fields. +template +class AnnotationProtoCollector : public AnnotationCollector { + public: + // annotation_proto is the protocol buffer to which new Annotations should be + // added. It is not owned by the AnnotationProtoCollector. + explicit AnnotationProtoCollector(AnnotationProto* annotation_proto) + : annotation_proto_(annotation_proto) {} + + // Override for AnnotationCollector::AddAnnotation. + virtual void AddAnnotation(size_t begin_offset, size_t end_offset, + const string& file_path, const vector& path) { + typename AnnotationProto::Annotation* annotation = + annotation_proto_->add_annotation(); + for (int i = 0; i < path.size(); ++i) { + annotation->add_path(path[i]); + } + annotation->set_source_file(file_path); + annotation->set_begin(begin_offset); + annotation->set_end(end_offset); + } + + private: + // The protocol buffer to which new annotations should be added. + AnnotationProto* const annotation_proto_; +}; + // This simple utility class assists in code generation. It basically // allows the caller to define a set of variables and then output some // text with variable substitutions. Example usage: @@ -61,13 +103,103 @@ class ZeroCopyOutputStream; // zero_copy_stream.h // Printer aggressively enforces correct usage, crashing (with assert failures) // in the case of undefined variables in debug builds. This helps greatly in // debugging code which uses it. +// +// If a Printer is constructed with an AnnotationCollector, it will provide it +// with annotations that connect the Printer's output to paths that can identify +// various descriptors. In the above example, if person_ is a descriptor that +// identifies Bob, we can associate the output string "My name is Bob." with +// a source path pointing to that descriptor with: +// +// printer.Annotate("name", person_); +// +// The AnnotationCollector will be sent an annotation linking the output range +// covering "Bob" to the logical path provided by person_. Tools may use +// this association to (for example) link "Bob" in the output back to the +// source file that defined the person_ descriptor identifying Bob. +// +// Annotate can only examine variables substituted during the last call to +// Print. It is invalid to refer to a variable that was used multiple times +// in a single Print call. +// +// In full generality, one may specify a range of output text using a beginning +// substitution variable and an ending variable. The resulting annotation will +// span from the first character of the substituted value for the beginning +// variable to the last character of the substituted value for the ending +// variable. For example, the Annotate call above is equivalent to this one: +// +// printer.Annotate("name", "name", person_); +// +// This is useful if multiple variables combine to form a single span of output +// that should be annotated with the same source path. For example: +// +// Printer printer(output, '$'); +// map vars; +// vars["first"] = "Alice"; +// vars["last"] = "Smith"; +// printer.Print(vars, "My name is $first$ $last$."); +// printer.Annotate("first", "last", person_); +// +// This code would associate the span covering "Alice Smith" in the output with +// the person_ descriptor. +// +// Note that the beginning variable must come before (or overlap with, in the +// case of zero-sized substitution values) the ending variable. +// +// It is also sometimes useful to use variables with zero-sized values as +// markers. This avoids issues with multiple references to the same variable +// and also allows annotation ranges to span literal text from the Print +// templates: +// +// Printer printer(output, '$'); +// map vars; +// vars["foo"] = "bar"; +// vars["function"] = "call"; +// vars["mark"] = ""; +// printer.Print(vars, "$function$($foo$,$foo$)$mark$"); +// printer.Annotate("function", "rmark", call_); +// +// This code associates the span covering "call(bar,bar)" in the output with the +// call_ descriptor. + class LIBPROTOBUF_EXPORT Printer { public: // Create a printer that writes text to the given output stream. Use the // given character as the delimiter for variables. Printer(ZeroCopyOutputStream* output, char variable_delimiter); + + // Create a printer that writes text to the given output stream. Use the + // given character as the delimiter for variables. If annotation_collector + // is not null, Printer will provide it with annotations about code written + // to the stream. annotation_collector is not owned by Printer. + Printer(ZeroCopyOutputStream* output, char variable_delimiter, + AnnotationCollector* annotation_collector); + ~Printer(); + // Link a subsitution variable emitted by the last call to Print to the object + // described by descriptor. + template + void Annotate(const char* varname, const SomeDescriptor* descriptor) { + Annotate(varname, varname, descriptor); + } + + // Link the output range defined by the substitution variables as emitted by + // the last call to Print to the object described by descriptor. The range + // begins at begin_varname's value and ends after the last character of the + // value substituted for end_varname. + template + void Annotate(const char* begin_varname, const char* end_varname, + const SomeDescriptor* descriptor) { + if (annotation_collector_ == NULL) { + // Annotations aren't turned on for this Printer, so don't pay the cost + // of building the location path. + return; + } + vector path; + descriptor->GetLocationPath(&path); + Annotate(begin_varname, end_varname, descriptor->file()->name(), path); + } + // Print some text after applying variable substitutions. If a particular // variable in the text is not defined, this will crash. Variables to be // substituted are identified by their names surrounded by delimiter @@ -149,16 +281,48 @@ class LIBPROTOBUF_EXPORT Printer { bool failed() const { return failed_; } private: + // Link the output range defined by the substitution variables as emitted by + // the last call to Print to the object found at the SourceCodeInfo-style path + // in a file with path file_path. The range begins at the start of + // begin_varname's value and ends after the last character of the value + // substituted for end_varname. Note that begin_varname and end_varname + // may refer to the same variable. + void Annotate(const char* begin_varname, const char* end_varname, + const string& file_path, const vector& path); + const char variable_delimiter_; ZeroCopyOutputStream* const output_; char* buffer_; int buffer_size_; + // The current position, in bytes, in the output stream. This is equivalent + // to the total number of bytes that have been written so far. This value is + // used to calculate annotation ranges in the substitutions_ map below. + size_t offset_; string indent_; bool at_start_of_line_; bool failed_; + // A map from variable name to [start, end) offsets in the output buffer. + // These refer to the offsets used for a variable after the last call to + // Print. If a variable was used more than once, the entry used in + // this map is set to a negative-length span. For singly-used variables, the + // start offset is the beginning of the substitution; the end offset is the + // last byte of the substitution plus one (such that (end - start) is the + // length of the substituted string). + map > substitutions_; + + // Returns true and sets range to the substitution range in the output for + // varname if varname was used once in the last call to Print. If varname + // was not used, or if it was used multiple times, returns false (and + // fails a debug assertion). + bool GetSubstitutionRange(const char* varname, pair* range); + + // If non-null, annotation_collector_ is used to store annotations about + // generated code. + AnnotationCollector* const annotation_collector_; + GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(Printer); }; diff --git a/src/google/protobuf/io/printer_unittest.cc b/src/google/protobuf/io/printer_unittest.cc index 258dd986..95f3afa2 100644 --- a/src/google/protobuf/io/printer_unittest.cc +++ b/src/google/protobuf/io/printer_unittest.cc @@ -36,6 +36,7 @@ #include #include +#include #include #include @@ -169,6 +170,196 @@ TEST(Printer, InlineVariableSubstitution) { buffer); } +// MockDescriptorFile defines only those members that Printer uses to write out +// annotations. +class MockDescriptorFile { + public: + explicit MockDescriptorFile(const string& file) : file_(file) {} + + // The mock filename for this file. + const string& name() const { return file_; } + + private: + string file_; +}; + +// MockDescriptor defines only those members that Printer uses to write out +// annotations. +class MockDescriptor { + public: + MockDescriptor(const string& file, const vector& path) + : file_(file), path_(path) {} + + // The mock file in which this descriptor was defined. + const MockDescriptorFile* file() const { return &file_; } + + private: + // Allows access to GetLocationPath. + friend class ::google::protobuf::io::Printer; + + // Copies the pre-stored path to output. + void GetLocationPath(std::vector* output) const { *output = path_; } + + MockDescriptorFile file_; + vector path_; +}; + +TEST(Printer, AnnotateMap) { + char buffer[8192]; + ArrayOutputStream output(buffer, sizeof(buffer)); + GeneratedCodeInfo info; + AnnotationProtoCollector info_collector(&info); + { + Printer printer(&output, '$', &info_collector); + map vars; + vars["foo"] = "3"; + vars["bar"] = "5"; + printer.Print(vars, "012$foo$4$bar$\n"); + vector path_1; + path_1.push_back(33); + vector path_2; + path_2.push_back(11); + path_2.push_back(22); + MockDescriptor descriptor_1("path_1", path_1); + MockDescriptor descriptor_2("path_2", path_2); + printer.Annotate("foo", "foo", &descriptor_1); + printer.Annotate("bar", "bar", &descriptor_2); + } + buffer[output.ByteCount()] = '\0'; + EXPECT_STREQ("012345\n", buffer); + ASSERT_EQ(2, info.annotation_size()); + const GeneratedCodeInfo::Annotation* foo = info.annotation(0).path_size() == 1 + ? &info.annotation(0) + : &info.annotation(1); + const GeneratedCodeInfo::Annotation* bar = info.annotation(0).path_size() == 1 + ? &info.annotation(1) + : &info.annotation(0); + ASSERT_EQ(1, foo->path_size()); + ASSERT_EQ(2, bar->path_size()); + EXPECT_EQ(33, foo->path(0)); + EXPECT_EQ(11, bar->path(0)); + EXPECT_EQ(22, bar->path(1)); + EXPECT_EQ("path_1", foo->source_file()); + EXPECT_EQ("path_2", bar->source_file()); + EXPECT_EQ(3, foo->begin()); + EXPECT_EQ(4, foo->end()); + EXPECT_EQ(5, bar->begin()); + EXPECT_EQ(6, bar->end()); +} + +TEST(Printer, AnnotateInline) { + char buffer[8192]; + ArrayOutputStream output(buffer, sizeof(buffer)); + GeneratedCodeInfo info; + AnnotationProtoCollector info_collector(&info); + { + Printer printer(&output, '$', &info_collector); + printer.Print("012$foo$4$bar$\n", "foo", "3", "bar", "5"); + vector path_1; + path_1.push_back(33); + vector path_2; + path_2.push_back(11); + path_2.push_back(22); + MockDescriptor descriptor_1("path_1", path_1); + MockDescriptor descriptor_2("path_2", path_2); + printer.Annotate("foo", "foo", &descriptor_1); + printer.Annotate("bar", "bar", &descriptor_2); + } + buffer[output.ByteCount()] = '\0'; + EXPECT_STREQ("012345\n", buffer); + ASSERT_EQ(2, info.annotation_size()); + const GeneratedCodeInfo::Annotation* foo = info.annotation(0).path_size() == 1 + ? &info.annotation(0) + : &info.annotation(1); + const GeneratedCodeInfo::Annotation* bar = info.annotation(0).path_size() == 1 + ? &info.annotation(1) + : &info.annotation(0); + ASSERT_EQ(1, foo->path_size()); + ASSERT_EQ(2, bar->path_size()); + EXPECT_EQ(33, foo->path(0)); + EXPECT_EQ(11, bar->path(0)); + EXPECT_EQ(22, bar->path(1)); + EXPECT_EQ("path_1", foo->source_file()); + EXPECT_EQ("path_2", bar->source_file()); + EXPECT_EQ(3, foo->begin()); + EXPECT_EQ(4, foo->end()); + EXPECT_EQ(5, bar->begin()); + EXPECT_EQ(6, bar->end()); +} + +TEST(Printer, AnnotateRange) { + char buffer[8192]; + ArrayOutputStream output(buffer, sizeof(buffer)); + GeneratedCodeInfo info; + AnnotationProtoCollector info_collector(&info); + { + Printer printer(&output, '$', &info_collector); + printer.Print("012$foo$4$bar$\n", "foo", "3", "bar", "5"); + vector path; + path.push_back(33); + MockDescriptor descriptor("path", path); + printer.Annotate("foo", "bar", &descriptor); + } + buffer[output.ByteCount()] = '\0'; + EXPECT_STREQ("012345\n", buffer); + ASSERT_EQ(1, info.annotation_size()); + const GeneratedCodeInfo::Annotation* foobar = &info.annotation(0); + ASSERT_EQ(1, foobar->path_size()); + EXPECT_EQ(33, foobar->path(0)); + EXPECT_EQ("path", foobar->source_file()); + EXPECT_EQ(3, foobar->begin()); + EXPECT_EQ(6, foobar->end()); +} + +TEST(Printer, AnnotateEmptyRange) { + char buffer[8192]; + ArrayOutputStream output(buffer, sizeof(buffer)); + GeneratedCodeInfo info; + AnnotationProtoCollector info_collector(&info); + { + Printer printer(&output, '$', &info_collector); + printer.Print("012$foo$4$baz$$bam$$bar$\n", "foo", "3", "bar", "5", "baz", + "", "bam", ""); + vector path; + path.push_back(33); + MockDescriptor descriptor("path", path); + printer.Annotate("baz", "bam", &descriptor); + } + buffer[output.ByteCount()] = '\0'; + EXPECT_STREQ("012345\n", buffer); + ASSERT_EQ(1, info.annotation_size()); + const GeneratedCodeInfo::Annotation* bazbam = &info.annotation(0); + ASSERT_EQ(1, bazbam->path_size()); + EXPECT_EQ(33, bazbam->path(0)); + EXPECT_EQ("path", bazbam->source_file()); + EXPECT_EQ(5, bazbam->begin()); + EXPECT_EQ(5, bazbam->end()); +} + +TEST(Printer, AnnotateDespiteUnrelatedMultipleUses) { + char buffer[8192]; + ArrayOutputStream output(buffer, sizeof(buffer)); + GeneratedCodeInfo info; + AnnotationProtoCollector info_collector(&info); + { + Printer printer(&output, '$', &info_collector); + printer.Print("012$foo$4$foo$$bar$\n", "foo", "3", "bar", "5"); + vector path; + path.push_back(33); + MockDescriptor descriptor("path", path); + printer.Annotate("bar", "bar", &descriptor); + } + buffer[output.ByteCount()] = '\0'; + EXPECT_STREQ("0123435\n", buffer); + ASSERT_EQ(1, info.annotation_size()); + const GeneratedCodeInfo::Annotation* bar = &info.annotation(0); + ASSERT_EQ(1, bar->path_size()); + EXPECT_EQ(33, bar->path(0)); + EXPECT_EQ("path", bar->source_file()); + EXPECT_EQ(6, bar->begin()); + EXPECT_EQ(7, bar->end()); +} + TEST(Printer, Indenting) { char buffer[8192]; @@ -232,6 +423,52 @@ TEST(Printer, Death) { EXPECT_DEBUG_DEATH(printer.Print("$unclosed"), "Unclosed variable name"); EXPECT_DEBUG_DEATH(printer.Outdent(), "without matching Indent"); } + +TEST(Printer, AnnotateMultipleUsesDeath) { + char buffer[8192]; + ArrayOutputStream output(buffer, sizeof(buffer)); + GeneratedCodeInfo info; + AnnotationProtoCollector info_collector(&info); + { + Printer printer(&output, '$', &info_collector); + printer.Print("012$foo$4$foo$\n", "foo", "3"); + vector path; + path.push_back(33); + MockDescriptor descriptor("path", path); + EXPECT_DEBUG_DEATH(printer.Annotate("foo", "foo", &descriptor), "multiple"); + } +} + +TEST(Printer, AnnotateNegativeLengthDeath) { + char buffer[8192]; + ArrayOutputStream output(buffer, sizeof(buffer)); + GeneratedCodeInfo info; + AnnotationProtoCollector info_collector(&info); + { + Printer printer(&output, '$', &info_collector); + printer.Print("012$foo$4$bar$\n", "foo", "3", "bar", "5"); + vector path; + path.push_back(33); + MockDescriptor descriptor("path", path); + EXPECT_DEBUG_DEATH(printer.Annotate("bar", "foo", &descriptor), "negative"); + } +} + +TEST(Printer, AnnotateUndefinedDeath) { + char buffer[8192]; + ArrayOutputStream output(buffer, sizeof(buffer)); + GeneratedCodeInfo info; + AnnotationProtoCollector info_collector(&info); + { + Printer printer(&output, '$', &info_collector); + printer.Print("012$foo$4$foo$\n", "foo", "3"); + vector path; + path.push_back(33); + MockDescriptor descriptor("path", path); + EXPECT_DEBUG_DEATH(printer.Annotate("bar", "bar", &descriptor), + "Undefined"); + } +} #endif // PROTOBUF_HAS_DEATH_TEST TEST(Printer, WriteFailurePartial) { diff --git a/src/google/protobuf/io/tokenizer.cc b/src/google/protobuf/io/tokenizer.cc index 3d57707c..b3550dfb 100644 --- a/src/google/protobuf/io/tokenizer.cc +++ b/src/google/protobuf/io/tokenizer.cc @@ -881,9 +881,11 @@ bool Tokenizer::ParseInteger(const string& text, uint64 max_value, uint64 result = 0; for (; *ptr != '\0'; ptr++) { int digit = DigitValue(*ptr); - GOOGLE_LOG_IF(DFATAL, digit < 0 || digit >= base) - << " Tokenizer::ParseInteger() passed text that could not have been" - " tokenized as an integer: " << CEscape(text); + if (digit < 0 || digit >= base) { + // The token provided by Tokenizer is invalid. i.e., 099 is an invalid + // token, but Tokenizer still think it's integer. + return false; + } if (digit > max_value || result > (max_value - digit) / base) { // Overflow. return false; diff --git a/src/google/protobuf/io/tokenizer.h b/src/google/protobuf/io/tokenizer.h index 49885eda..64ee7d84 100644 --- a/src/google/protobuf/io/tokenizer.h +++ b/src/google/protobuf/io/tokenizer.h @@ -52,6 +52,12 @@ class ZeroCopyInputStream; // zero_copy_stream.h class ErrorCollector; class Tokenizer; +// By "column number", the proto compiler refers to a count of the number +// of bytes before a given byte, except that a tab character advances to +// the next multiple of 8 bytes. Note in particular that column numbers +// are zero-based, while many user interfaces use one-based column numbers. +typedef int ColumnNumber; + // Abstract interface for an object which collects the errors that occur // during parsing. A typical implementation might simply print the errors // to stdout. @@ -63,13 +69,14 @@ class LIBPROTOBUF_EXPORT ErrorCollector { // Indicates that there was an error in the input at the given line and // column numbers. The numbers are zero-based, so you may want to add // 1 to each before printing them. - virtual void AddError(int line, int column, const string& message) = 0; + virtual void AddError(int line, ColumnNumber column, + const string& message) = 0; // Indicates that there was a warning in the input at the given line and // column numbers. The numbers are zero-based, so you may want to add // 1 to each before printing them. - virtual void AddWarning(int /* line */, int /* column */, - const string& /* message */) { } + virtual void AddWarning(int line, ColumnNumber column, + const string& message) { } private: GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(ErrorCollector); @@ -124,8 +131,8 @@ class LIBPROTOBUF_EXPORT Tokenizer { // "line" and "column" specify the position of the first character of // the token within the input stream. They are zero-based. int line; - int column; - int end_column; + ColumnNumber column; + ColumnNumber end_column; }; // Get the current token. This is updated when Next() is called. Before @@ -263,7 +270,7 @@ class LIBPROTOBUF_EXPORT Tokenizer { // Line and column number of current_char_ within the whole input stream. int line_; - int column_; + ColumnNumber column_; // String to which text should be appended as we advance through it. // Call RecordTo(&str) to start recording and StopRecording() to stop. @@ -280,6 +287,7 @@ class LIBPROTOBUF_EXPORT Tokenizer { // Since we count columns we need to interpret tabs somehow. We'll take // the standard 8-character definition for lack of any way to do better. + // This must match the documentation of ColumnNumber. static const int kTabWidth = 8; // ----------------------------------------------------------------- diff --git a/src/google/protobuf/io/tokenizer_unittest.cc b/src/google/protobuf/io/tokenizer_unittest.cc index 20d50a2c..ae0811f8 100644 --- a/src/google/protobuf/io/tokenizer_unittest.cc +++ b/src/google/protobuf/io/tokenizer_unittest.cc @@ -736,19 +736,13 @@ TEST_F(TokenizerTest, ParseInteger) { EXPECT_EQ(0, ParseInteger("0x")); uint64 i; -#ifdef PROTOBUF_HAS_DEATH_TEST // death tests do not work on Windows yet + // Test invalid integers that will never be tokenized as integers. - EXPECT_DEBUG_DEATH(Tokenizer::ParseInteger("zxy", kuint64max, &i), - "passed text that could not have been tokenized as an integer"); - EXPECT_DEBUG_DEATH(Tokenizer::ParseInteger("1.2", kuint64max, &i), - "passed text that could not have been tokenized as an integer"); - EXPECT_DEBUG_DEATH(Tokenizer::ParseInteger("08", kuint64max, &i), - "passed text that could not have been tokenized as an integer"); - EXPECT_DEBUG_DEATH(Tokenizer::ParseInteger("0xg", kuint64max, &i), - "passed text that could not have been tokenized as an integer"); - EXPECT_DEBUG_DEATH(Tokenizer::ParseInteger("-1", kuint64max, &i), - "passed text that could not have been tokenized as an integer"); -#endif // PROTOBUF_HAS_DEATH_TEST + EXPECT_FALSE(Tokenizer::ParseInteger("zxy", kuint64max, &i)); + EXPECT_FALSE(Tokenizer::ParseInteger("1.2", kuint64max, &i)); + EXPECT_FALSE(Tokenizer::ParseInteger("08", kuint64max, &i)); + EXPECT_FALSE(Tokenizer::ParseInteger("0xg", kuint64max, &i)); + EXPECT_FALSE(Tokenizer::ParseInteger("-1", kuint64max, &i)); // Test overflows. EXPECT_TRUE (Tokenizer::ParseInteger("0", 0, &i)); diff --git a/src/google/protobuf/io/zero_copy_stream_impl_lite.cc b/src/google/protobuf/io/zero_copy_stream_impl_lite.cc index 083beca4..e6ca88c2 100644 --- a/src/google/protobuf/io/zero_copy_stream_impl_lite.cc +++ b/src/google/protobuf/io/zero_copy_stream_impl_lite.cc @@ -69,7 +69,7 @@ ArrayInputStream::~ArrayInputStream() { bool ArrayInputStream::Next(const void** data, int* size) { if (position_ < size_) { - last_returned_size_ = min(block_size_, size_ - position_); + last_returned_size_ = std::min(block_size_, size_ - position_); *data = data_ + position_; *size = last_returned_size_; position_ += last_returned_size_; @@ -122,7 +122,7 @@ ArrayOutputStream::~ArrayOutputStream() { bool ArrayOutputStream::Next(void** data, int* size) { if (position_ < size_) { - last_returned_size_ = min(block_size_, size_ - position_); + last_returned_size_ = std::min(block_size_, size_ - position_); *data = data_ + position_; *size = last_returned_size_; position_ += last_returned_size_; @@ -157,7 +157,7 @@ StringOutputStream::~StringOutputStream() { } bool StringOutputStream::Next(void** data, int* size) { - GOOGLE_CHECK_NE(NULL, target_); + GOOGLE_CHECK(target_ != NULL); int old_size = target_->size(); // Grow the string. @@ -177,9 +177,9 @@ bool StringOutputStream::Next(void** data, int* size) { // Double the size, also make sure that the new size is at least // kMinimumSize. STLStringResizeUninitialized( - target_, - max(old_size * 2, - kMinimumSize + 0)); // "+ 0" works around GCC4 weirdness. + target_, + std::max(old_size * 2, + kMinimumSize + 0)); // "+ 0" works around GCC4 weirdness. } *data = mutable_string_data(target_) + old_size; @@ -189,13 +189,13 @@ bool StringOutputStream::Next(void** data, int* size) { void StringOutputStream::BackUp(int count) { GOOGLE_CHECK_GE(count, 0); - GOOGLE_CHECK_NE(NULL, target_); + GOOGLE_CHECK(target_ != NULL); GOOGLE_CHECK_LE(count, target_->size()); target_->resize(target_->size() - count); } int64 StringOutputStream::ByteCount() const { - GOOGLE_CHECK_NE(NULL, target_); + GOOGLE_CHECK(target_ != NULL); return target_->size(); } @@ -235,8 +235,8 @@ int CopyingInputStream::Skip(int count) { char junk[4096]; int skipped = 0; while (skipped < count) { - int bytes = Read(junk, min(count - skipped, - implicit_cast(sizeof(junk)))); + int bytes = + Read(junk, std::min(count - skipped, implicit_cast(sizeof(junk)))); if (bytes <= 0) { // EOF or read error. return skipped; diff --git a/src/google/protobuf/io/zero_copy_stream_impl_lite.h b/src/google/protobuf/io/zero_copy_stream_impl_lite.h index 1c397dea..cc7430ec 100644 --- a/src/google/protobuf/io/zero_copy_stream_impl_lite.h +++ b/src/google/protobuf/io/zero_copy_stream_impl_lite.h @@ -51,6 +51,7 @@ #include #include #include +#include #include #include #include -- cgit v1.2.3