Skip to content

Commit

Permalink
Enable Darwin per-controller storage by default. (#30215)
Browse files Browse the repository at this point in the history
Fixes the parameters bits to allow later adding XPC parameters.
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Mar 20, 2024
1 parent af11a23 commit 7d5a7fe
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 10 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/darwin.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,8 @@ jobs:
# -enableUndefinedBehaviorSanitizer instruments the code in Matter.framework,
# but to instrument the code in the underlying libCHIP we need to pass CHIP_IS_UBSAN=YES
TEST_RUNNER_ASAN_OPTIONS=__CURRENT_VALUE__:detect_stack_use_after_return=1 xcodebuild test -target "Matter" -scheme "Matter Framework Tests" -sdk macosx -enableAddressSanitizer YES -enableUndefinedBehaviorSanitizer YES OTHER_CFLAGS='${inherited} -Werror -Wconversion' CHIP_IS_UBSAN=YES CHIP_IS_BLE=NO GCC_PREPROCESSOR_DEFINITIONS='${inherited} MTR_NO_AVAILABILITY=1'> >(tee /tmp/darwin/framework-tests/darwin-tests-asan.log) 2> >(tee /tmp/darwin/framework-tests/darwin-tests-asan-err.log >&2)
# And the same thing, but with MTR_PER_CONTROLLER_STORAGE_ENABLED turned on.
TEST_RUNNER_ASAN_OPTIONS=__CURRENT_VALUE__:detect_stack_use_after_return=1 xcodebuild test -target "Matter" -scheme "Matter Framework Tests" -sdk macosx -enableAddressSanitizer YES -enableUndefinedBehaviorSanitizer YES OTHER_CFLAGS='${inherited} -Werror -Wconversion' CHIP_IS_UBSAN=YES CHIP_IS_BLE=NO GCC_PREPROCESSOR_DEFINITIONS='${inherited} MTR_NO_AVAILABILITY=1 MTR_PER_CONTROLLER_STORAGE_ENABLED=1' > >(tee /tmp/darwin/framework-tests/darwin-tests-asan-controller-storage.log) 2> >(tee /tmp/darwin/framework-tests/darwin-tests-asan-controller-storage-err.log >&2)
# And the same thing, but with MTR_PER_CONTROLLER_STORAGE_ENABLED turned off, so we test that it does not break for now.
TEST_RUNNER_ASAN_OPTIONS=__CURRENT_VALUE__:detect_stack_use_after_return=1 xcodebuild test -target "Matter" -scheme "Matter Framework Tests" -sdk macosx -enableAddressSanitizer YES -enableUndefinedBehaviorSanitizer YES OTHER_CFLAGS='${inherited} -Werror -Wconversion' CHIP_IS_UBSAN=YES CHIP_IS_BLE=NO GCC_PREPROCESSOR_DEFINITIONS='${inherited} MTR_NO_AVAILABILITY=1 MTR_PER_CONTROLLER_STORAGE_ENABLED=0' > >(tee /tmp/darwin/framework-tests/darwin-tests-asan-controller-storage.log) 2> >(tee /tmp/darwin/framework-tests/darwin-tests-asan-controller-storage-err.log >&2)
# And the same thing, but with MTR_ENABLE_PROVISIONAL also turned on.
TEST_RUNNER_ASAN_OPTIONS=__CURRENT_VALUE__:detect_stack_use_after_return=1 xcodebuild test -target "Matter" -scheme "Matter Framework Tests" -sdk macosx -enableAddressSanitizer YES -enableUndefinedBehaviorSanitizer YES OTHER_CFLAGS='${inherited} -Werror -Wconversion' CHIP_IS_UBSAN=YES CHIP_IS_BLE=NO GCC_PREPROCESSOR_DEFINITIONS='${inherited} MTR_NO_AVAILABILITY=1 MTR_PER_CONTROLLER_STORAGE_ENABLED=1 MTR_ENABLE_PROVISIONAL=1' > >(tee /tmp/darwin/framework-tests/darwin-tests-asan-provisional.log) 2> >(tee /tmp/darwin/framework-tests/darwin-tests-asan-provisional-err.log >&2)
# And the same thing, but with MTR_NO_AVAILABILITY not turned on. This requires -Wno-unguarded-availability-new to avoid availability errors.
Expand Down
2 changes: 1 addition & 1 deletion src/darwin/Framework/CHIP/MTRDefines.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@
#endif

#ifndef MTR_PER_CONTROLLER_STORAGE_ENABLED
#define MTR_PER_CONTROLLER_STORAGE_ENABLED 0
#define MTR_PER_CONTROLLER_STORAGE_ENABLED 1
#endif

#pragma mark - Types
Expand Down
4 changes: 2 additions & 2 deletions src/darwin/Framework/CHIP/MTRDeviceController.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
@class MTRBaseDevice;

#if MTR_PER_CONTROLLER_STORAGE_ENABLED
@class MTRDeviceControllerParameters;
@class MTRDeviceControllerAbstractParameters;
#endif // MTR_PER_CONTROLLER_STORAGE_ENABLED

NS_ASSUME_NONNULL_BEGIN
Expand Down Expand Up @@ -58,7 +58,7 @@ typedef void (^MTRDeviceConnectionCallback)(MTRBaseDevice * _Nullable device, NS
* Once this returns non-nil, it's the caller's resposibility to call shutdown
* on the controller to avoid leaking it.
*/
- (nullable instancetype)initWithParameters:(MTRDeviceControllerParameters *)parameters
- (nullable instancetype)initWithParameters:(MTRDeviceControllerAbstractParameters *)parameters
error:(NSError * __autoreleasing *)error MTR_NEWLY_AVAILABLE;
#endif // MTR_PER_CONTROLLER_STORAGE_ENABLED

Expand Down
15 changes: 13 additions & 2 deletions src/darwin/Framework/CHIP/MTRDeviceController.mm
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,17 @@ @interface MTRDeviceController () {

@implementation MTRDeviceController

- (nullable instancetype)initWithParameters:(MTRDeviceControllerParameters *)parameters error:(NSError * __autoreleasing *)error
- (nullable instancetype)initWithParameters:(MTRDeviceControllerAbstractParameters *)parameters error:(NSError * __autoreleasing *)error
{
if (![parameters isKindOfClass:MTRDeviceControllerParameters.class]) {
MTR_LOG_ERROR("Unsupported type of MTRDeviceControllerAbstractParameters: %@", parameters);

if (error) {
*error = [MTRError errorForCHIPErrorCode:CHIP_ERROR_INVALID_ARGUMENT];
}
return nil;
}

__auto_type * factory = [MTRDeviceControllerFactory sharedInstance];
if (!factory.isRunning) {
auto * params = [[MTRDeviceControllerFactoryParams alloc] initWithoutStorage];
Expand All @@ -138,7 +147,9 @@ - (nullable instancetype)initWithParameters:(MTRDeviceControllerParameters *)par
}
}

return [factory initializeController:self withParameters:parameters error:error];
auto * parametersForFactory = static_cast<MTRDeviceControllerParameters *>(parameters);

return [factory initializeController:self withParameters:parametersForFactory error:error];
}

- (instancetype)initWithFactory:(MTRDeviceControllerFactory *)factory
Expand Down
19 changes: 17 additions & 2 deletions src/darwin/Framework/CHIP/MTRDeviceControllerParameters.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,29 @@

NS_ASSUME_NONNULL_BEGIN

/**
* Parameters that can be used to initialize an MTRDeviceController. Specific
* interfaces inheriting from this one should be used to actually do the
* initialization.
*/
#if !MTR_PER_CONTROLLER_STORAGE_ENABLED
MTR_HIDDEN
#endif
MTR_NEWLY_AVAILABLE
@interface MTRDeviceControllerParameters : NSObject

@interface MTRDeviceControllerAbstractParameters : NSObject
- (instancetype)init NS_UNAVAILABLE;
+ (instancetype)new NS_UNAVAILABLE;
@end

/**
* Parameters that can be used to initialize an MTRDeviceController which
* has a node identity.
*/
#if !MTR_PER_CONTROLLER_STORAGE_ENABLED
MTR_HIDDEN
#endif
MTR_NEWLY_AVAILABLE
@interface MTRDeviceControllerParameters : MTRDeviceControllerAbstractParameters

/**
* The Product Attestation Authority certificates that are trusted to sign
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,13 @@ - (instancetype)initWithOperationalKeypair:(id<MTRKeypair>)operationalKeypair

@end

@implementation MTRDeviceControllerAbstractParameters
- (instancetype)_initInternal
{
return [super init];
}
@end

@implementation MTRDeviceControllerParameters
- (instancetype)initWithStorageDelegate:(id<MTRDeviceControllerStorageDelegate>)storageDelegate
storageDelegateQueue:(dispatch_queue_t)storageDelegateQueue
Expand All @@ -262,7 +269,7 @@ - (instancetype)initWithStorageDelegate:(id<MTRDeviceControllerStorageDelegate>)
intermediateCertificate:(MTRCertificateDERBytes _Nullable)intermediateCertificate
rootCertificate:(MTRCertificateDERBytes)rootCertificate
{
if (!(self = [super init])) {
if (!(self = [super _initInternal])) {
return nil;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,11 @@ NS_ASSUME_NONNULL_BEGIN
- (instancetype)initWithParams:(MTRDeviceControllerStartupParams *)params;
@end

@interface MTRDeviceControllerAbstractParameters ()
// Allow init from our subclasses.
- (instancetype)_initInternal;
@end

@interface MTRDeviceControllerParameters ()

- (instancetype)initWithStorageDelegate:(id<MTRDeviceControllerStorageDelegate>)storageDelegate
Expand Down

0 comments on commit 7d5a7fe

Please sign in to comment.