From f27532d0bdb5479e2c4b583bd5961d40a92c8e0a Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Thu, 31 Oct 2024 10:07:37 -0700 Subject: [PATCH] Check that there are unknown fields before calling MutableUnknownFields to clear it. Otherwise, we end up creating the unknown field set just to call Clear on it, which wastes memory. PiperOrigin-RevId: 691844650 --- src/google/protobuf/reflection_ops.cc | 5 ++++- src/google/protobuf/reflection_ops_unittest.cc | 9 +++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/google/protobuf/reflection_ops.cc b/src/google/protobuf/reflection_ops.cc index 6d6811becf5c..84247c958b8e 100644 --- a/src/google/protobuf/reflection_ops.cc +++ b/src/google/protobuf/reflection_ops.cc @@ -20,6 +20,7 @@ #include "google/protobuf/descriptor.pb.h" #include "google/protobuf/map_field.h" #include "google/protobuf/map_field_inl.h" +#include "google/protobuf/port.h" #include "google/protobuf/unknown_field_set.h" // Must be included last. @@ -320,7 +321,9 @@ static bool IsMapValueMessageTyped(const FieldDescriptor* map_field) { void ReflectionOps::DiscardUnknownFields(Message* message) { const Reflection* reflection = GetReflectionOrDie(*message); - reflection->MutableUnknownFields(message)->Clear(); + if (reflection->GetUnknownFields(*message).field_count() != 0) { + reflection->MutableUnknownFields(message)->Clear(); + } // Walk through the fields of this message and DiscardUnknownFields on any // messages present. diff --git a/src/google/protobuf/reflection_ops_unittest.cc b/src/google/protobuf/reflection_ops_unittest.cc index 8f14edc778a0..b481fa56f728 100644 --- a/src/google/protobuf/reflection_ops_unittest.cc +++ b/src/google/protobuf/reflection_ops_unittest.cc @@ -274,6 +274,15 @@ TEST(ReflectionOpsTest, DiscardUnknownFields) { message.repeated_nested_message(0).unknown_fields().field_count()); } +TEST(ReflectionOpsTest, DiscardUnknownFieldsIsANoopIfNotPresent) { + unittest::TestAllTypes message; + EXPECT_EQ(&message.unknown_fields(), + &google::protobuf::UnknownFieldSet::default_instance()); + ReflectionOps::DiscardUnknownFields(&message); + EXPECT_EQ(&message.unknown_fields(), + &google::protobuf::UnknownFieldSet::default_instance()); +} + TEST(ReflectionOpsTest, DiscardUnknownExtensions) { unittest::TestAllExtensions message; TestUtil::SetAllExtensions(&message);