Darwin: Improve os_log integration (#23528)

* Prefer locally installed restyle-path script if it exists

* Simplify GetModuleName

* Remove unused / unimplemented GetMessageWithPrefix and related unused constants

* Remove duplicate defaults for CHIP_*_LOGGING and move CHIP_LOG_FILTERING to CHIPConfig.h

* Remove duplicate CHIP_SYSTEM_CONFIG_NO_LOCKING section

* Rename support:enforce_format to attributes and include DLLUtil.h in it

* Darwin: Integrate CHIP logging with os_log at the log macro level

Introduce CHIP_SYSTEM_CONFIG_PLATFORM_LOG that enables integrating with CHIP logging at the log macro level.

Note: ChipLog* macros are now statements (not expressions), so a few of usages of VerifyOr... macros required adjustments.

* Fix (unrelated?) clang-tidy error

* Add logging:force_stdio target that takes preference over the platform logging backend and use it for unit tests and cli tools (chip-tool etc)

* Darwin: Implement MTR_LOG_* via ChipLog* and rename MTRLogging.h to MTRLogging_Internal.h. Also compile all ObjC files as ObjC++.

* Darwin: Replace NSLog usages in Matter.framework with MTR_LOG_*

* Darwin: Re-add MTRLogging.h as a public header and expose MTRSetLogCallback()

* Darwin: Use log redirection in darwin-framework-tool to log to stdout.

* Review: Document `...` parameters that take statements in CodeUtils.h

* Review: some comments and tweaks

* Darwin: Add logging preferences plist

build/chip/chip_test_suite.gni

@@ -94,15 +94,9 @@ template("chip_test_suite") {
       public_deps = []
     }
 
-    # TODO: figure out a way to auto-define dependency on stdio logging.
-    #       This dep is required since libSupportLayer now does not include
-    #       a default implementation.
-    #
-    #       Tests such as TestInetEndPoint need to exercise the system event
-    #       loop however they do not seem to include a logging binding so this
-    #       is forcefully added here.
     if (current_os != "zephyr" && current_os != "mbed") {
-      public_deps += [ "${chip_root}/src/platform/logging:stdio" ]
+      # Depend on stdio logging, and have it take precedence over the default platform backend
+      public_deps += [ "${chip_root}/src/platform/logging:force_stdio" ]
     }
   }
 

examples/chip-tool/BUILD.gn

@@ -120,7 +120,10 @@ static_library("chip-tool-utils") {
 executable("chip-tool") {
   sources = [ "main.cpp" ]
 
-  deps = [ ":chip-tool-utils" ]
+  deps = [
+    ":chip-tool-utils",
+    "${chip_root}/src/platform/logging:force_stdio",
+  ]
 
   output_dir = root_out_dir
 }

examples/chip-tool/commands/interactive/InteractiveCommands.cpp

@@ -18,6 +18,8 @@
 
 #include "InteractiveCommands.h"
 
+#include <platform/logging/LogV.h>
+
 #include <editline.h>
 #include <iomanip>
 #include <sstream>

examples/darwin-framework-tool/commands/interactive/InteractiveCommands.mm

@@ -17,7 +17,8 @@
  */
 
 #include "InteractiveCommands.h"
-#import <Matter/Matter.h>
+
+#include <platform/logging/LogV.h>
 
 #include <editline.h>
 #include <iomanip>

examples/darwin-framework-tool/main.mm

@@ -28,9 +28,15 @@
 #include <zap-generated/cluster/Commands.h>
 #include <zap-generated/test/Commands.h>
 
+#include <cstdio>
+
 int main(int argc, const char * argv[])
 {
     @autoreleasepool {
+        MTRSetLogCallback(MTRLogTypeDetail, ^(MTRLogType type, NSString * component, NSString * message) {
+            fprintf(stdout, "CHIP:%s: %s\n", component.UTF8String, message.UTF8String);
+        });
+
         Commands commands;
         registerCommandsPairing(commands);
         registerCommandsInteractive(commands);

examples/platform/linux/BUILD.gn

@@ -70,6 +70,7 @@ source_set("app-main") {
     "${chip_root}/src/lib",
     "${chip_root}/src/lib/shell",
     "${chip_root}/src/lib/shell:shell_core",
+    "${chip_root}/src/platform/logging:force_stdio",
   ]
 
   if (chip_enable_transport_trace) {

scripts/helpers/restyle-diff.sh

@@ -33,9 +33,12 @@ set -e
 CHIP_ROOT=$(cd "$here/../.." && pwd)
 
 restyle-paths() {
-    url=https://github.com/restyled-io/restyler/raw/main/bin/restyle-path
-
-    sh <(curl --location --proto "=https" --tlsv1.2 "$url" -sSf) "$@"
+    if hash restyle-path 2>/dev/null; then
+        command restyle-path "$@"
+    else
+        url=https://github.com/restyled-io/restyler/raw/main/bin/restyle-path
+        sh <(curl --location --proto "=https" --tlsv1.2 "$url" -sSf) "$@"
+    fi
 }
 
 cd "$CHIP_ROOT"

src/access/AccessControl.cpp

@@ -549,11 +549,11 @@ bool AccessControl::IsValid(const Entry & entry)
     const char * log = "unexpected error";
     IgnoreUnusedVariable(log); // logging may be disabled
 
-    AuthMode authMode;
-    FabricIndex fabricIndex;
-    Privilege privilege;
-    size_t subjectCount = 0;
-    size_t targetCount  = 0;
+    AuthMode authMode       = AuthMode::kNone;
+    FabricIndex fabricIndex = kUndefinedFabricIndex;
+    Privilege privilege     = static_cast<Privilege>(0);
+    size_t subjectCount     = 0;
+    size_t targetCount      = 0;
 
     SuccessOrExit(entry.GetAuthMode(authMode));
     SuccessOrExit(entry.GetFabricIndex(fabricIndex));

src/darwin/Framework/CHIP/MTRAsyncCallbackWorkQueue.mm

@@ -19,7 +19,7 @@
 #import <os/lock.h>
 
 #import "MTRAsyncCallbackWorkQueue.h"
-#import "MTRLogging.h"
+#import "MTRLogging_Internal.h"
 
 #pragma mark - Class extensions
 

src/darwin/Framework/CHIP/MTRBaseDevice.mm

@@ -24,7 +24,7 @@
 #import "MTRCluster_internal.h"
 #import "MTRError_Internal.h"
 #import "MTREventTLVValueDecoder_Internal.h"
-#import "MTRLogging.h"
+#import "MTRLogging_Internal.h"
 #import "MTRSetupPayload_Internal.h"
 
 #include "app/ConcreteAttributePath.h"

src/darwin/Framework/CHIP/MTRCertificates.mm

@@ -16,8 +16,8 @@
 
 #import "MTRCertificates.h"
 #import "MTRError_Internal.h"
-#import "MTRLogging.h"
-#import "MTRMemory.h"
+#import "MTRFramework.h"
+#import "MTRLogging_Internal.h"
 #import "MTROperationalCredentialsDelegate.h"
 #import "MTRP256KeypairBridge.h"
 #import "NSDataSpanConversion.h"
@@ -31,23 +31,25 @@
 
 @implementation MTRCertificates
 
++ (void)initialize
+{
+    MTRFrameworkInit();
+}
+
 + (MTRCertificateDERBytes _Nullable)createRootCertificate:(id<MTRKeypair>)keypair
                                                  issuerID:(NSNumber * _Nullable)issuerID
                                                  fabricID:(NSNumber * _Nullable)fabricID
                                                     error:(NSError * __autoreleasing *)error
 {
-    NSLog(@"Generating root certificate");
-
-    [MTRMemory ensureInit];
-
+    MTR_LOG_DEFAULT("Generating root certificate");
     NSData * rootCert = nil;
     CHIP_ERROR err = MTROperationalCredentialsDelegate::GenerateRootCertificate(keypair, issuerID, fabricID, &rootCert);
     if (error) {
         *error = [MTRError errorForCHIPErrorCode:err];
     }
 
     if (err != CHIP_NO_ERROR) {
-        NSLog(@"Generating root certificate failed: %s", ErrorStr(err));
+        MTR_LOG_ERROR("Generating root certificate failed: %s", ErrorStr(err));
     }
 
     return rootCert;
@@ -60,10 +62,7 @@ + (MTRCertificateDERBytes _Nullable)createIntermediateCertificate:(id<MTRKeypair
                                                          fabricID:(NSNumber * _Nullable)fabricID
                                                             error:(NSError * __autoreleasing *)error
 {
-    NSLog(@"Generating intermediate certificate");
-
-    [MTRMemory ensureInit];
-
+    MTR_LOG_DEFAULT("Generating intermediate certificate");
     NSData * intermediate = nil;
     CHIP_ERROR err = MTROperationalCredentialsDelegate::GenerateIntermediateCertificate(
         rootKeypair, rootCertificate, intermediatePublicKey, issuerID, fabricID, &intermediate);
@@ -72,7 +71,7 @@ + (MTRCertificateDERBytes _Nullable)createIntermediateCertificate:(id<MTRKeypair
     }
 
     if (err != CHIP_NO_ERROR) {
-        NSLog(@"Generating intermediate certificate failed: %s", ErrorStr(err));
+        MTR_LOG_ERROR("Generating intermediate certificate failed: %s", ErrorStr(err));
     }
 
     return intermediate;
@@ -86,10 +85,7 @@ + (MTRCertificateDERBytes _Nullable)createOperationalCertificate:(id<MTRKeypair>
                                            caseAuthenticatedTags:(NSArray<NSNumber *> * _Nullable)caseAuthenticatedTags
                                                            error:(NSError * __autoreleasing _Nullable * _Nullable)error
 {
-    NSLog(@"Generating operational certificate");
-
-    [MTRMemory ensureInit];
-
+    MTR_LOG_DEFAULT("Generating operational certificate");
     NSData * opcert = nil;
     CHIP_ERROR err = MTROperationalCredentialsDelegate::GenerateOperationalCertificate(
         signingKeypair, signingCertificate, operationalPublicKey, fabricID, nodeID, caseAuthenticatedTags, &opcert);
@@ -98,28 +94,26 @@ + (MTRCertificateDERBytes _Nullable)createOperationalCertificate:(id<MTRKeypair>
     }
 
     if (err != CHIP_NO_ERROR) {
-        NSLog(@"Generating operational certificate failed: %s", ErrorStr(err));
+        MTR_LOG_ERROR("Generating operational certificate failed: %s", ErrorStr(err));
     }
 
     return opcert;
 }
 
 + (BOOL)keypair:(id<MTRKeypair>)keypair matchesCertificate:(NSData *)certificate
 {
-    [MTRMemory ensureInit];
-
     P256PublicKey keypairPubKey;
     CHIP_ERROR err = MTRP256KeypairBridge::MatterPubKeyFromSecKeyRef(keypair.publicKey, &keypairPubKey);
     if (err != CHIP_NO_ERROR) {
-        NSLog(@"Can't extract public key from keypair: %s", ErrorStr(err));
+        MTR_LOG_ERROR("Can't extract public key from keypair: %s", ErrorStr(err));
         return NO;
     }
     P256PublicKeySpan keypairKeySpan(keypairPubKey.ConstBytes());
 
     P256PublicKey certPubKey;
     err = ExtractPubkeyFromX509Cert(AsByteSpan(certificate), certPubKey);
     if (err != CHIP_NO_ERROR) {
-        NSLog(@"Can't extract public key from certificate: %s", ErrorStr(err));
+        MTR_LOG_ERROR("Can't extract public key from certificate: %s", ErrorStr(err));
         return NO;
     }
     P256PublicKeySpan certKeySpan(certPubKey.ConstBytes());
@@ -129,20 +123,18 @@ + (BOOL)keypair:(id<MTRKeypair>)keypair matchesCertificate:(NSData *)certificate
 
 + (BOOL)isCertificate:(MTRCertificateDERBytes)certificate1 equalTo:(MTRCertificateDERBytes)certificate2
 {
-    [MTRMemory ensureInit];
-
     P256PublicKey pubKey1;
     CHIP_ERROR err = ExtractPubkeyFromX509Cert(AsByteSpan(certificate1), pubKey1);
     if (err != CHIP_NO_ERROR) {
-        NSLog(@"Can't extract public key from first certificate: %s", ErrorStr(err));
+        MTR_LOG_ERROR("Can't extract public key from first certificate: %s", ErrorStr(err));
         return NO;
     }
     P256PublicKeySpan keySpan1(pubKey1.ConstBytes());
 
     P256PublicKey pubKey2;
     err = ExtractPubkeyFromX509Cert(AsByteSpan(certificate2), pubKey2);
     if (err != CHIP_NO_ERROR) {
-        NSLog(@"Can't extract public key from second certificate: %s", ErrorStr(err));
+        MTR_LOG_ERROR("Can't extract public key from second certificate: %s", ErrorStr(err));
         return NO;
     }
     P256PublicKeySpan keySpan2(pubKey1.ConstBytes());
@@ -154,14 +146,14 @@ + (BOOL)isCertificate:(MTRCertificateDERBytes)certificate1 equalTo:(MTRCertifica
     ChipDN subject1;
     err = ExtractSubjectDNFromX509Cert(AsByteSpan(certificate1), subject1);
     if (err != CHIP_NO_ERROR) {
-        NSLog(@"Can't extract subject DN from first certificate: %s", ErrorStr(err));
+        MTR_LOG_ERROR("Can't extract subject DN from first certificate: %s", ErrorStr(err));
         return NO;
     }
 
     ChipDN subject2;
     err = ExtractSubjectDNFromX509Cert(AsByteSpan(certificate2), subject2);
     if (err != CHIP_NO_ERROR) {
-        NSLog(@"Can't extract subject DN from second certificate: %s", ErrorStr(err));
+        MTR_LOG_ERROR("Can't extract subject DN from second certificate: %s", ErrorStr(err));
         return NO;
     }
 
@@ -171,8 +163,6 @@ + (BOOL)isCertificate:(MTRCertificateDERBytes)certificate1 equalTo:(MTRCertifica
 + (NSData * _Nullable)createCertificateSigningRequest:(id<MTRKeypair>)keypair
                                                 error:(NSError * __autoreleasing _Nullable * _Nullable)error
 {
-    [MTRMemory ensureInit];
-
     MTRP256KeypairBridge keypairBridge;
     CHIP_ERROR err = CHIP_NO_ERROR;
     do {
@@ -206,7 +196,7 @@ + (MTRCertificateTLVBytes _Nullable)convertX509Certificate:(MTRCertificateDERByt
     chip::MutableByteSpan chipCertBytes(chipCertBuffer);
 
     CHIP_ERROR errorCode = chip::Credentials::ConvertX509CertToChipCert(x509CertBytes, chipCertBytes);
-    MTR_LOG_ERROR("ConvertX509CertToChipCert: %{public}s", chip::ErrorStr(errorCode));
+    MTR_LOG_ERROR("ConvertX509CertToChipCert: %s", chip::ErrorStr(errorCode));
 
     if (errorCode != CHIP_NO_ERROR)
         return nil;
@@ -224,7 +214,7 @@ + (MTRCertificateDERBytes _Nullable)convertMatterCertificate:(MTRCertificateTLVB
     CHIP_ERROR errorCode = chip::Credentials::ConvertChipCertToX509Cert(tlvCertBytes, derCertBytes);
 
     if (errorCode != CHIP_NO_ERROR) {
-        MTR_LOG_ERROR("ConvertChipCertToX509Cert: %{public}s", chip::ErrorStr(errorCode));
+        MTR_LOG_ERROR("ConvertChipCertToX509Cert: %s", chip::ErrorStr(errorCode));
         return nil;
     }
 

src/darwin/Framework/CHIP/MTRClusterStateCacheContainer.mm

@@ -23,7 +23,7 @@
 #import "MTRDeviceControllerXPCConnection.h"
 #import "MTRError.h"
 #import "MTRError_Internal.h"
-#import "MTRLogging.h"
+#import "MTRLogging_Internal.h"
 
 #include <app/InteractionModelEngine.h>
 #include <lib/support/ErrorStr.h>

src/darwin/Framework/CHIP/MTRMemory.mm → src/darwin/Framework/CHIP/MTRDefines.h

@@ -1,4 +1,5 @@
 /**
+ *
  *    Copyright (c) 2022 Project CHIP Authors
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
@@ -14,19 +15,12 @@
  *    limitations under the License.
  */
 
-#import "MTRMemory.h"
-
-#include <lib/support/CHIPMem.h>
-
-@implementation MTRMemory
+#import <Foundation/Foundation.h>
 
-+ (void)ensureInit
-{
-    static dispatch_once_t onceToken;
-    dispatch_once(&onceToken, ^{
-        // The malloc version of MemoryInit never fails.
-        chip::Platform::MemoryInit();
-    });
-}
+#define MTR_EXPORT __attribute__((visibility("default")))
 
-@end
+#ifdef __cplusplus
+#define MTR_EXTERN extern "C" MTR_EXPORT
+#else
+#define MTR_EXTERN extern MTR_EXPORT
+#endif

src/darwin/Framework/CHIP/MTRDevice.mm

@@ -25,7 +25,7 @@
 #import "MTRDevice_Internal.h"
 #import "MTRError_Internal.h"
 #import "MTREventTLVValueDecoder_Internal.h"
-#import "MTRLogging.h"
+#import "MTRLogging_Internal.h"
 
 #include "lib/core/CHIPError.h"
 #include "lib/core/DataModelTypes.h"

src/darwin/Framework/CHIP/MTRDeviceAttestationDelegateBridge.mm

@@ -18,14 +18,16 @@
 #import "MTRDeviceAttestationDelegateBridge.h"
 #import "MTRDeviceAttestationDelegate_Internal.h"
 #import "MTRError_Internal.h"
+#import "MTRLogging_Internal.h"
 #import "NSDataSpanConversion.h"
 
 void MTRDeviceAttestationDelegateBridge::OnDeviceAttestationCompleted(chip::Controller::DeviceCommissioner * deviceCommissioner,
     chip::DeviceProxy * device, const chip::Credentials::DeviceAttestationVerifier::AttestationDeviceInfo & info,
     chip::Credentials::AttestationVerificationResult attestationResult)
 {
     dispatch_async(mQueue, ^{
-        NSLog(@"MTRDeviceAttestationDelegateBridge::OnDeviceAttestationFailed completed with result: %hu", attestationResult);
+        MTR_LOG_DEFAULT(
+            "MTRDeviceAttestationDelegateBridge::OnDeviceAttestationFailed completed with result: %hu", attestationResult);
 
         mResult = attestationResult;
 

src/darwin/Framework/CHIP/MTRDeviceController.mm

@@ -27,7 +27,7 @@
 #import "MTRDevice_Internal.h"
 #import "MTRError_Internal.h"
 #import "MTRKeypair.h"
-#import "MTRLogging.h"
+#import "MTRLogging_Internal.h"
 #import "MTROperationalCredentialsDelegate.h"
 #import "MTRP256KeypairBridge.h"
 #import "MTRPersistentStorageDelegateBridge.h"