Simplify the validation code by using VerifyOrReturnValue

setUpPINCode range is checked in IsValidSetupPIN() via CheckPayloadCommonConstraints()
so no separate check is needed.

Author: ksperling-apple

src/setup_payload/SetupPayload.cpp

@@ -39,38 +39,23 @@ namespace chip {
 bool PayloadContents::isValidQRCodePayload(ValidationMode mode) const
 {
     // 3-bit value specifying the QR code payload version.
-    if (version >= 1 << kVersionFieldLengthInBits)
-    {
-        return false;
-    }
+    VerifyOrReturnValue(version < (1 << kVersionFieldLengthInBits), false);
 
-    if (static_cast<uint8_t>(commissioningFlow) > static_cast<uint8_t>((1 << kCommissioningFlowFieldLengthInBits) - 1))
-    {
-        return false;
-    }
+    VerifyOrReturnValue(static_cast<uint8_t>(commissioningFlow) < (1 << kCommissioningFlowFieldLengthInBits), false);
 
     // Device Commissioning Flow
     // Even in ValidatoinMode::kConsume we can only handle modes that we understand.
     // 0: Standard commissioning flow: such a device, when uncommissioned, always enters commissioning mode upon power-up, subject
     // to the rules in [ref_Announcement_Commencement]. 1: User-intent commissioning flow: user action required to enter
     // commissioning mode. 2: Custom commissioning flow: interaction with a vendor-specified means is needed before commissioning.
     // 3: Reserved
-    if (commissioningFlow != CommissioningFlow::kStandard && commissioningFlow != CommissioningFlow::kUserActionRequired &&
-        commissioningFlow != CommissioningFlow::kCustom)
-    {
-        return false;
-    }
+    VerifyOrReturnValue(commissioningFlow == CommissioningFlow::kStandard ||
+                            commissioningFlow == CommissioningFlow::kUserActionRequired ||
+                            commissioningFlow == CommissioningFlow::kCustom,
+                        false);
 
     // General discriminator validity is enforced by the SetupDiscriminator class, but it can't be short for QR a code.
-    if (discriminator.IsShortDiscriminator())
-    {
-        return false;
-    }
-
-    if (setUpPINCode >= 1 << kSetupPINCodeFieldLengthInBits)
-    {
-        return false;
-    }
+    VerifyOrReturnValue(!discriminator.IsShortDiscriminator(), false);
 
     // RendevouzInformation must be present for a QR code.
     VerifyOrReturnValue(rendezvousInformation.HasValue(), false);
@@ -86,12 +71,8 @@ bool PayloadContents::isValidQRCodePayload(ValidationMode mode) const
 
 bool PayloadContents::isValidManualCode(ValidationMode mode) const
 {
-    // Discriminator validity is enforced by the SetupDiscriminator class.
-    if (setUpPINCode >= 1 << kSetupPINCodeFieldLengthInBits)
-    {
-        return false;
-    }
-
+    // No additional constraints apply to Manual Pairing Codes.
+    // (If the payload has a long discriminator it will be converted automatically.)
     return CheckPayloadCommonConstraints();
 }
 
@@ -114,30 +95,19 @@ bool PayloadContents::CheckPayloadCommonConstraints() const
     // Validation rules in this method apply to all validation modes.
 
     // Even in ValidationMode::kConsume we don't understand how to handle any payload version other than 0.
-    if (version != 0)
-    {
-        return false;
-    }
+    VerifyOrReturnValue(version == 0, false);
 
-    if (!IsValidSetupPIN(setUpPINCode))
-    {
-        return false;
-    }
+    VerifyOrReturnValue(IsValidSetupPIN(setUpPINCode), false);
 
     // VendorID must be unspecified (0) or in valid range expected.
-    if (!IsVendorIdValidOperationally(vendorID) && (vendorID != VendorId::Unspecified))
-    {
-        return false;
-    }
+    VerifyOrReturnValue((vendorID == VendorId::Unspecified) || IsVendorIdValidOperationally(vendorID), false);
 
     // A value of 0x0000 SHALL NOT be assigned to a product since Product ID = 0x0000 is used for these specific cases:
     //  * To announce an anonymized Product ID as part of device discovery
     //  * To indicate an OTA software update file applies to multiple Product IDs equally.
     //  * To avoid confusion when presenting the Onboarding Payload for ECM with multiple nodes
-    if (productID == 0 && vendorID != VendorId::Unspecified)
-    {
-        return false;
-    }
+    // In these special cases the vendorID must be 0 (Unspecified)
+    VerifyOrReturnValue(productID != 0 || vendorID == VendorId::Unspecified, false);
 
     return true;
 }

src/setup_payload/SetupPayload.h

@@ -76,6 +76,7 @@ inline constexpr uint8_t kCommissioningTimeoutTag = 0x04;
 
 inline constexpr uint32_t kSetupPINCodeMaximumValue   = 99999998;
 inline constexpr uint32_t kSetupPINCodeUndefinedValue = 0;
+static_assert(kSetupPINCodeMaximumValue < (1 << kSetupPINCodeFieldLengthInBits));
 
 // clang-format off
 const int kTotalPayloadDataSizeInBits =