Skip to content

Commit

Permalink
Fix asan when using ExtendendedParamDefinition::operator==.
Browse files Browse the repository at this point in the history
  - IAMF defines the extended param definition to just carry a raw payload. It makes no promise that it will extend from `ParamDefinition`.
  - Therfore the equality operator should not check these fields. Simply checking the derived payload is enough.
  - Test the equality operator, which would have flagged the asan issue earlier.

PiperOrigin-RevId: 703477465
  • Loading branch information
jwcullen committed Dec 6, 2024
1 parent 10b1fda commit 7ca9f3c
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 5 deletions.
25 changes: 20 additions & 5 deletions iamf/obu/param_definitions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,29 @@ absl::Status ValidateSpecificParamDefinition(
} // namespace

bool operator==(const ParamDefinition& lhs, const ParamDefinition& rhs) {
// First check always-present fields.
if (lhs.param_definition_mode_ != rhs.param_definition_mode_) {
return false;
}
if (lhs.type_ != rhs.type_) {
return false;
}
if (!lhs.EquivalentDerived(rhs)) {
if (!lhs.type_.has_value()) {
return true;
}
switch (*lhs.type_) {
using enum ParamDefinition::ParameterDefinitionType;
case kParameterDefinitionMixGain:
case kParameterDefinitionDemixing:
case kParameterDefinitionReconGain:
if (!lhs.EquivalentDerived(rhs)) {
return false;
}
break;
case kParameterDefinitionReservedStart:
case kParameterDefinitionReservedEnd:
// Other fields are virtual. Only compare the derived "extended" bytes.
return lhs.EquivalentDerived(rhs);
}

// First check always-present fields.
if (lhs.param_definition_mode_ != rhs.param_definition_mode_) {
return false;
}

Expand Down
35 changes: 35 additions & 0 deletions iamf/obu/tests/param_definitions_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -864,5 +864,40 @@ TEST(ExtendedParamDefinition, ReadAndValidateWithNonZeroSize) {
kExpectedParamDefinitionBytes);
}

TEST(ExtendedParamDefinitionEqualityOperator, Equals) {
ExtendedParamDefinition lhs(
ParamDefinition::kParameterDefinitionReservedStart);
lhs.param_definition_size_ = 5;
lhs.param_definition_bytes_ = {'e', 'x', 't', 'r', 'a'};
ExtendedParamDefinition rhs(
ParamDefinition::kParameterDefinitionReservedStart);
rhs.param_definition_size_ = 5;
rhs.param_definition_bytes_ = {'e', 'x', 't', 'r', 'a'};

EXPECT_TRUE(lhs == rhs);
}

TEST(ExtendedParamDefinitionEqualityOperator, NotEqualsWhenTypeIsDifferent) {
const ExtendedParamDefinition lhs(
ParamDefinition::kParameterDefinitionReservedStart);
const ExtendedParamDefinition rhs(
ParamDefinition::kParameterDefinitionReservedEnd);

EXPECT_NE(lhs, rhs);
}

TEST(ExtendedParamDefinitionEqualityOperator, NotEqualsWhenPayloadIsDifferent) {
ExtendedParamDefinition lhs(
ParamDefinition::kParameterDefinitionReservedStart);
lhs.param_definition_size_ = 3;
lhs.param_definition_bytes_ = {'e', 'x', 't'};
ExtendedParamDefinition rhs(
ParamDefinition::kParameterDefinitionReservedStart);
rhs.param_definition_size_ = 5;
rhs.param_definition_bytes_ = {'e', 'x', 't', 'r', 'a'};

EXPECT_NE(lhs, rhs);
}

} // namespace
} // namespace iamf_tools

0 comments on commit 7ca9f3c

Please sign in to comment.