From 7467f752b3e1db66d75be87de4a491343e5e74ed Mon Sep 17 00:00:00 2001 From: Josh Humphries Date: Wed, 14 Sep 2022 15:16:51 -0400 Subject: [PATCH] change format of int range in error message; use macro to make DRY --- src/google/protobuf/descriptor.cc | 35 +++++++++++----------- src/google/protobuf/descriptor_unittest.cc | 16 +++++----- 2 files changed, 25 insertions(+), 26 deletions(-) diff --git a/src/google/protobuf/descriptor.cc b/src/google/protobuf/descriptor.cc index 0e43bc3af6fc..cd40584b374a 100644 --- a/src/google/protobuf/descriptor.cc +++ b/src/google/protobuf/descriptor.cc @@ -58,6 +58,7 @@ #include "absl/strings/ascii.h" #include "absl/strings/escaping.h" #include "absl/strings/str_cat.h" +#include "absl/strings/str_format.h" #include "google/protobuf/stubs/stringprintf.h" #include "absl/strings/str_join.h" #include "absl/strings/str_split.h" @@ -7675,6 +7676,14 @@ bool DescriptorBuilder::OptionInterpreter::ExamineIfOptionIsSet( return true; } +#define VALUE_OUT_OF_RANGE(T, NAME) absl::StrFormat( \ + "Value out of range, %d to %d, for " #T " option \"%s\".", \ + std::numeric_limits::min(), std::numeric_limits::max(), NAME) + +#define VALUE_MUST_BE_INT(T, NAME) absl::StrFormat( \ + "Value must be integer, from %d to %d, for " #T " option \"%s\".", \ + std::numeric_limits::min(), std::numeric_limits::max(), NAME) + bool DescriptorBuilder::OptionInterpreter::SetOptionValue( const FieldDescriptor* option_field, UnknownFieldSet* unknown_fields) { // We switch on the CppType to validate. @@ -7683,8 +7692,7 @@ bool DescriptorBuilder::OptionInterpreter::SetOptionValue( if (uninterpreted_option_->has_positive_int_value()) { if (uninterpreted_option_->positive_int_value() > static_cast(std::numeric_limits::max())) { - return AddValueError("Value out of range, -2,147,483,648 to 2,147,483,647, for int32 option \"" + - option_field->full_name() + "\"."); + return AddValueError(VALUE_OUT_OF_RANGE(int32, option_field->full_name())); } else { SetInt32(option_field->number(), uninterpreted_option_->positive_int_value(), @@ -7693,16 +7701,14 @@ bool DescriptorBuilder::OptionInterpreter::SetOptionValue( } else if (uninterpreted_option_->has_negative_int_value()) { if (uninterpreted_option_->negative_int_value() < static_cast(std::numeric_limits::min())) { - return AddValueError("Value out of range, -2,147,483,648 to 2,147,483,647, for int32 option \"" + - option_field->full_name() + "\"."); + return AddValueError(VALUE_OUT_OF_RANGE(int32, option_field->full_name())); } else { SetInt32(option_field->number(), uninterpreted_option_->negative_int_value(), option_field->type(), unknown_fields); } } else { - return AddValueError("Value must be integer, from -2,147,483,648 to 2,147,483,647, for int32 option \"" + - option_field->full_name() + "\"."); + return AddValueError(VALUE_MUST_BE_INT(int32, option_field->full_name())); } break; @@ -7710,8 +7716,7 @@ bool DescriptorBuilder::OptionInterpreter::SetOptionValue( if (uninterpreted_option_->has_positive_int_value()) { if (uninterpreted_option_->positive_int_value() > static_cast(std::numeric_limits::max())) { - return AddValueError("Value out of range, -9,223,372,036,854,775,808 to 9,223,372,036,854,775,807, for int64 option \"" + - option_field->full_name() + "\"."); + return AddValueError(VALUE_OUT_OF_RANGE(int64, option_field->full_name())); } else { SetInt64(option_field->number(), uninterpreted_option_->positive_int_value(), @@ -7722,8 +7727,7 @@ bool DescriptorBuilder::OptionInterpreter::SetOptionValue( uninterpreted_option_->negative_int_value(), option_field->type(), unknown_fields); } else { - return AddValueError("Value must be integer, from -9,223,372,036,854,775,808 to 9,223,372,036,854,775,807, for int64 option \"" + - option_field->full_name() + "\"."); + return AddValueError(VALUE_MUST_BE_INT(int64, option_field->full_name())); } break; @@ -7731,17 +7735,14 @@ bool DescriptorBuilder::OptionInterpreter::SetOptionValue( if (uninterpreted_option_->has_positive_int_value()) { if (uninterpreted_option_->positive_int_value() > std::numeric_limits::max()) { - return AddValueError("Value out of range, 0 to 4,294,967,295, for uint32 option \"" + - option_field->name() + "\"."); + return AddValueError(VALUE_OUT_OF_RANGE(uint32, option_field->full_name())); } else { SetUInt32(option_field->number(), uninterpreted_option_->positive_int_value(), option_field->type(), unknown_fields); } } else { - return AddValueError( - "Value must be integer, from 0 to 4,294,967,295, for uint32 option \"" + - option_field->full_name() + "\"."); + return AddValueError(VALUE_MUST_BE_INT(uint32, option_field->full_name())); } break; @@ -7751,9 +7752,7 @@ bool DescriptorBuilder::OptionInterpreter::SetOptionValue( uninterpreted_option_->positive_int_value(), option_field->type(), unknown_fields); } else { - return AddValueError( - "Value must be integer, from 0 to 18,446,744,073,709,551,615, for uint64 option \"" + - option_field->full_name() + "\"."); + return AddValueError(VALUE_MUST_BE_INT(uint64, option_field->full_name())); } break; diff --git a/src/google/protobuf/descriptor_unittest.cc b/src/google/protobuf/descriptor_unittest.cc index 33d4299fb647..8079a2689511 100644 --- a/src/google/protobuf/descriptor_unittest.cc +++ b/src/google/protobuf/descriptor_unittest.cc @@ -5557,7 +5557,7 @@ TEST_F(ValidationErrorTest, Int32OptionValueOutOfPositiveRange) { " positive_int_value: 0x80000000 } " "}", - "foo.proto: foo.proto: OPTION_VALUE: Value out of range, -2,147,483,648 to 2,147,483,647, " + "foo.proto: foo.proto: OPTION_VALUE: Value out of range, -2147483648 to 2147483647, " "for int32 option \"foo\".\n"); } @@ -5574,7 +5574,7 @@ TEST_F(ValidationErrorTest, Int32OptionValueOutOfNegativeRange) { " negative_int_value: -0x80000001 } " "}", - "foo.proto: foo.proto: OPTION_VALUE: Value out of range, -2,147,483,648 to 2,147,483,647, " + "foo.proto: foo.proto: OPTION_VALUE: Value out of range, -2147483648 to 2147483647, " "for int32 option \"foo\".\n"); } @@ -5590,7 +5590,7 @@ TEST_F(ValidationErrorTest, Int32OptionValueIsNotPositiveInt) { " is_extension: true } " " string_value: \"5\" } }", - "foo.proto: foo.proto: OPTION_VALUE: Value must be integer, from -2,147,483,648 to 2,147,483,647, " + "foo.proto: foo.proto: OPTION_VALUE: Value must be integer, from -2147483648 to 2147483647, " "for int32 option \"foo\".\n"); } @@ -5608,7 +5608,7 @@ TEST_F(ValidationErrorTest, Int64OptionValueOutOfRange) { "} " "}", - "foo.proto: foo.proto: OPTION_VALUE: Value out of range, -9,223,372,036,854,775,808 to 9,223,372,036,854,775,807, " + "foo.proto: foo.proto: OPTION_VALUE: Value out of range, -9223372036854775808 to 9223372036854775807, " "for int64 option \"foo\".\n"); } @@ -5624,7 +5624,7 @@ TEST_F(ValidationErrorTest, Int64OptionValueIsNotPositiveInt) { " is_extension: true } " " identifier_value: \"5\" } }", - "foo.proto: foo.proto: OPTION_VALUE: Value must be integer, from -9,223,372,036,854,775,808 to 9,223,372,036,854,775,807, " + "foo.proto: foo.proto: OPTION_VALUE: Value must be integer, from -9223372036854775808 to 9223372036854775807, " "for int64 option \"foo\".\n"); } @@ -5640,7 +5640,7 @@ TEST_F(ValidationErrorTest, UInt32OptionValueOutOfRange) { " is_extension: true } " " positive_int_value: 0x100000000 } }", - "foo.proto: foo.proto: OPTION_VALUE: Value out of range, 0 to 4,294,967,295, " + "foo.proto: foo.proto: OPTION_VALUE: Value out of range, 0 to 4294967295, " "for uint32 option \"foo\".\n"); } @@ -5656,7 +5656,7 @@ TEST_F(ValidationErrorTest, UInt32OptionValueIsNotPositiveInt) { " is_extension: true } " " double_value: -5.6 } }", - "foo.proto: foo.proto: OPTION_VALUE: Value must be integer, from 0 to 4,294,967,295, " + "foo.proto: foo.proto: OPTION_VALUE: Value must be integer, from 0 to 4294967295, " "for uint32 option \"foo\".\n"); } @@ -5672,7 +5672,7 @@ TEST_F(ValidationErrorTest, UInt64OptionValueIsNotPositiveInt) { " is_extension: true } " " negative_int_value: -5 } }", - "foo.proto: foo.proto: OPTION_VALUE: Value must be integer, from 0 to 18,446,744,073,709,551,615, " + "foo.proto: foo.proto: OPTION_VALUE: Value must be integer, from 0 to 18446744073709551615, " "for uint64 option \"foo\".\n"); }