Skip to content

Commit

Permalink
Check that there are unknown fields before calling MutableUnknownFiel…
Browse files Browse the repository at this point in the history
…ds to

clear it. Otherwise, we end up creating the unknown field set just to call
Clear on it, which wastes memory.

PiperOrigin-RevId: 691844650
  • Loading branch information
protobuf-github-bot authored and copybara-github committed Oct 31, 2024
1 parent e406a1a commit f27532d
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 1 deletion.
5 changes: 4 additions & 1 deletion src/google/protobuf/reflection_ops.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
9 changes: 9 additions & 0 deletions src/google/protobuf/reflection_ops_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit f27532d

Please sign in to comment.