Address review comments

project-chip · Jan 6, 2022 · 92081f8 · 92081f8
1 parent f219eee
commit 92081f8
Show file tree

Hide file tree

Showing 24 changed files with 69 additions and 24 deletions.
diff --git a/src/app/tests/TestEventLogging.cpp b/src/app/tests/TestEventLogging.cpp
@@ -32,6 +32,7 @@
 #include <lib/core/CHIPTLV.h>
 #include <lib/core/CHIPTLVDebug.hpp>
 #include <lib/core/CHIPTLVUtilities.hpp>
+#include <lib/support/EnforceFormat.h>
 #include <lib/support/ErrorStr.h>
 #include <lib/support/UnitTestRegistration.h>
 #include <lib/support/logging/Constants.h>

diff --git a/src/app/tests/TestMessageDef.cpp b/src/app/tests/TestMessageDef.cpp
@@ -37,6 +37,7 @@
 #include <lib/core/CHIPError.h>
 #include <lib/core/CHIPTLVDebug.hpp>
 #include <lib/support/CHIPMem.h>
+#include <lib/support/EnforceFormat.h>
 #include <lib/support/UnitTestRegistration.h>
 #include <lib/support/logging/Constants.h>
 #include <system/TLVPacketBufferBackingStore.h>

diff --git a/src/app/tests/integration/common.cpp b/src/app/tests/integration/common.cpp
@@ -27,6 +27,7 @@
 #include <app/tests/integration/common.h>
 #include <lib/core/CHIPCore.h>
 #include <lib/core/CHIPTLVDebug.hpp>
+#include <lib/support/EnforceFormat.h>
 #include <lib/support/ErrorStr.h>
 #include <lib/support/logging/Constants.h>
 #include <platform/CHIPDeviceLayer.h>

diff --git a/src/lib/core/CHIPTLV.h b/src/lib/core/CHIPTLV.h
@@ -33,6 +33,7 @@
 
 #include <lib/support/BitFlags.h>
 #include <lib/support/DLLUtil.h>
+#include <lib/support/EnforceFormat.h>
 #include <lib/support/ScopedBuffer.h>
 #include <lib/support/Span.h>
 #include <lib/support/TypeTraits.h>

diff --git a/src/lib/shell/streamer.cpp b/src/lib/shell/streamer.cpp
@@ -23,6 +23,7 @@
 
 #include "streamer.h"
 
+#include <lib/support/EnforceFormat.h>
 #include <lib/support/logging/Constants.h>
 #include <limits.h>
 #include <stdio.h>

diff --git a/src/lib/support/BUILD.gn b/src/lib/support/BUILD.gn
@@ -40,6 +40,10 @@ source_set("logging_constants") {
   sources = [ "logging/Constants.h" ]
 }
 
+source_set("enforce_format") {
+  sources = [ "EnforceFormat.h" ]
+}
+
 source_set("chip_version_header") {
   sources = get_target_outputs(":gen_chip_version")
 
@@ -131,6 +135,7 @@ static_library("support") {
 
   public_deps = [
     ":chip_version_header",
+    ":enforce_format",
     ":logging_constants",
     "${chip_root}/src/lib/core:chip_config_header",
     "${chip_root}/src/platform:platform_buildconfig",

diff --git a/src/lib/support/CHIPArgParser.cpp b/src/lib/support/CHIPArgParser.cpp
@@ -48,6 +48,7 @@
 
 #include <lib/support/CHIPMem.h>
 #include <lib/support/CHIPMemString.h>
+#include <lib/support/EnforceFormat.h>
 #include <lib/support/logging/Constants.h>
 
 /*

diff --git a/src/lib/support/DefaultStorageKeyAllocator.h b/src/lib/support/DefaultStorageKeyAllocator.h
@@ -17,6 +17,7 @@
 #pragma once
 
 #include <app/util/basic-types.h>
+#include <lib/support/EnforceFormat.h>
 #include <lib/support/logging/Constants.h>
 #include <string.h>
 

diff --git a/src/lib/support/EnforceFormat.h b/src/lib/support/EnforceFormat.h
@@ -0,0 +1,34 @@
+/*
+ *    Copyright (c) 2022 Project CHIP Authors
+ *    All rights reserved.
+ *
+ *    Licensed under the Apache License, Version 2.0 (the "License");
+ *    you may not use this file except in compliance with the License.
+ *    You may obtain a copy of the License at
+ *
+ *        http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *    Unless required by applicable law or agreed to in writing, software
+ *    distributed under the License is distributed on an "AS IS" BASIS,
+ *    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *    See the License for the specific language governing permissions and
+ *    limitations under the License.
+ */
+
+#pragma once
+
+/**
+ * gcc and clang provide a way to warn for a custom formatter when formats don't
+ * match arguments.  Use that so we catch mistakes.  The "format"
+ * attribute takes the type of format, which arg is the format string, and which
+ * arg is the first variadic arg, with both arg numbers 1-based.
+ *
+ * The second arg should be set to 0 if the function takes a va_list instead of
+ * varargs.
+ */
+
+#if defined(__GNUC__)
+#define ENFORCE_FORMAT(n, m) __attribute__((format(printf, n, m)))
+#else                        // __GNUC__
+#define ENFORCE_FORMAT(n, m) /* How to do with MSVC? */
+#endif                       // __GNUC__
diff --git a/src/lib/support/logging/CHIPLogging.h b/src/lib/support/logging/CHIPLogging.h
@@ -39,6 +39,7 @@
 
 #include <platform/logging/LogV.h>
 
+#include <lib/support/EnforceFormat.h>
 #include <lib/support/logging/Constants.h>
 
 #include <inttypes.h>

diff --git a/src/lib/support/logging/Constants.h b/src/lib/support/logging/Constants.h
@@ -2,22 +2,6 @@
 
 #pragma once
 
-/**
- * gcc and clang provide a way to warn for a custom formatter when formats don't
- * match arguments.  Use that for Log() so we catch mistakes.  The "format"
- * attribute takes the type of format, which arg is the format string, and which
- * arg is the first variadic arg, with both arg numbers 1-based.
- *
- * The second arg should be set to 0 if the function takes a va_list instead of
- * varargs.
- */
-
-#if defined(__GNUC__)
-#define ENFORCE_FORMAT(n, m) __attribute__((format(printf, n, m)))
-#else                        // __GNUC__
-#define ENFORCE_FORMAT(n, m) /* How to do with MSVC? */
-#endif                       // __GNUC__
-
 namespace chip {
 namespace Logging {
 

diff --git a/src/lib/support/tests/TestCHIPArgParser.cpp b/src/lib/support/tests/TestCHIPArgParser.cpp
@@ -26,6 +26,7 @@
 #include <lib/support/CHIPArgParser.hpp>
 #include <lib/support/CHIPMem.h>
 #include <lib/support/CHIPMemString.h>
+#include <lib/support/EnforceFormat.h>
 #include <lib/support/ScopedBuffer.h>
 #include <lib/support/UnitTestRegistration.h>
 #include <lib/support/logging/Constants.h>

diff --git a/src/platform/Darwin/Logging.cpp b/src/platform/Darwin/Logging.cpp
@@ -1,5 +1,6 @@
 /* See Project chip LICENSE file for licensing information. */
 
+#include <lib/support/EnforceFormat.h>
 #include <lib/support/logging/Constants.h>
 #include <platform/logging/LogV.h>
 

diff --git a/src/platform/ESP32/Logging.cpp b/src/platform/ESP32/Logging.cpp
@@ -3,6 +3,7 @@
 #include <platform/logging/LogV.h>
 
 #include <lib/core/CHIPConfig.h>
+#include <lib/support/EnforceFormat.h>
 #include <lib/support/logging/Constants.h>
 
 #include <stdio.h>
@@ -18,7 +19,7 @@ namespace chip {
 namespace Logging {
 namespace Platform {
 
-void LogV(const char * module, uint8_t category, const char * msg, va_list v)
+void ENFORCE_FORMAT(3, 0) LogV(const char * module, uint8_t category, const char * msg, va_list v)
 {
     char tag[11];
 

diff --git a/src/platform/Linux/Logging.cpp b/src/platform/Linux/Logging.cpp
@@ -1,5 +1,6 @@
 /* See Project CHIP LICENSE file for licensing information. */
 
+#include <lib/support/EnforceFormat.h>
 #include <lib/support/logging/Constants.h>
 #include <platform/logging/LogV.h>
 

diff --git a/src/platform/Tizen/Logging.cpp b/src/platform/Tizen/Logging.cpp
@@ -18,6 +18,7 @@
 #include <platform/logging/LogV.h>
 
 #include <lib/core/CHIPConfig.h>
+#include <lib/support/EnforceFormat.h>
 #include <lib/support/logging/Constants.h>
 
 #include <dlog.h>
@@ -30,7 +31,7 @@ namespace Platform {
 /**
  * CHIP log output functions.
  */
-void LogV(const char * module, uint8_t category, const char * msg, va_list v)
+void ENFORCE_FORMAT(3, 0) LogV(const char * module, uint8_t category, const char * msg, va_list v)
 {
     constexpr const char * kLogTag                = "CHIP";
     char msgBuf[CHIP_CONFIG_LOG_MESSAGE_MAX_SIZE] = {

diff --git a/src/platform/Zephyr/Logging.cpp b/src/platform/Zephyr/Logging.cpp
@@ -3,6 +3,7 @@
 #include <platform/logging/LogV.h>
 
 #include <lib/core/CHIPConfig.h>
+#include <lib/support/EnforceFormat.h>
 #include <lib/support/logging/Constants.h>
 
 #include <kernel.h>
@@ -40,7 +41,7 @@ namespace Platform {
  * CHIP log output function.
  */
 
-void LogV(const char * module, uint8_t category, const char * msg, va_list v)
+void ENFORCE_FORMAT(3, 0) LogV(const char * module, uint8_t category, const char * msg, va_list v)
 {
     char formattedMsg[CHIP_CONFIG_LOG_MESSAGE_MAX_SIZE];
     snprintfcb(formattedMsg, sizeof(formattedMsg), "[%s]", module);

diff --git a/src/platform/logging/BUILD.gn b/src/platform/logging/BUILD.gn
@@ -16,6 +16,7 @@ if (current_os == "android") {
     deps = [
       ":headers",
       "${chip_root}/src/lib/core:chip_config_header",
+      "${chip_root}/src/lib/support:enforce_format",
       "${chip_root}/src/lib/support:logging_constants",
       "${chip_root}/src/platform:platform_buildconfig",
     ]
@@ -30,6 +31,7 @@ static_library("stdio") {
   deps = [
     ":headers",
     "${chip_root}/src/lib/core:chip_config_header",
+    "${chip_root}/src/lib/support:enforce_format",
     "${chip_root}/src/lib/support:logging_constants",
     "${chip_root}/src/platform:platform_buildconfig",
   ]

diff --git a/src/platform/logging/LogV.h b/src/platform/logging/LogV.h
@@ -2,6 +2,7 @@
 
 #pragma once
 
+#include <lib/support/EnforceFormat.h>
 #include <stdarg.h>
 #include <stdint.h>
 
@@ -27,7 +28,7 @@ namespace Platform {
  *                      correspond to the format specifiers in @a msg.
  *
  */
-void LogV(const char * module, uint8_t category, const char * msg, va_list v);
+void ENFORCE_FORMAT(3, 0) LogV(const char * module, uint8_t category, const char * msg, va_list v);
 
 } // namespace Platform
 } // namespace Logging

diff --git a/src/platform/logging/impl/android/Logging.cpp b/src/platform/logging/impl/android/Logging.cpp
@@ -1,5 +1,6 @@
 /* See Project chip LICENSE file for licensing information. */
 
+#include <lib/support/EnforceFormat.h>
 #include <lib/support/logging/Constants.h>
 #include <platform/logging/LogV.h>
 

diff --git a/src/platform/logging/impl/stdio/Logging.cpp b/src/platform/logging/impl/stdio/Logging.cpp
@@ -1,5 +1,6 @@
 /* See Project CHIP LICENSE file for licensing information. */
 
+#include <lib/support/EnforceFormat.h>
 #include <lib/support/logging/Constants.h>
 #include <platform/logging/LogV.h>
 

diff --git a/src/platform/mbed/Logging.cpp b/src/platform/mbed/Logging.cpp
@@ -22,6 +22,7 @@
  */
 
 #include <lib/core/CHIPConfig.h>
+#include <lib/support/EnforceFormat.h>
 #include <lib/support/logging/CHIPLogging.h>
 #include <lib/support/logging/Constants.h>
 #include <platform/logging/LogV.h>
@@ -65,7 +66,7 @@ char logMsgBuffer[CHIP_CONFIG_LOG_MESSAGE_MAX_SIZE];
 /**
  * CHIP log output functions.
  */
-void LogV(const char * module, uint8_t category, const char * msg, va_list v)
+void ENFORCE_FORMAT(3, 0) LogV(const char * module, uint8_t category, const char * msg, va_list v)
 {
     size_t prefixLen = 0;
     snprintf(logMsgBuffer, sizeof(logMsgBuffer), "[%s]", module);

diff --git a/src/platform/nxp/k32w/k32w0/Logging.cpp b/src/platform/nxp/k32w/k32w0/Logging.cpp
@@ -3,6 +3,7 @@
 #include <platform/logging/LogV.h>
 
 #include <lib/core/CHIPConfig.h>
+#include <lib/support/EnforceFormat.h>
 #include <lib/support/logging/Constants.h>
 #include <platform/CHIPDeviceConfig.h>
 #include <src/lib/support/CodeUtils.h>
@@ -96,7 +97,7 @@ void __attribute__((weak)) OnLogOutput(void) {}
 } // namespace DeviceLayer
 } // namespace chip
 
-void GenericLog(const char * format, va_list arg, const char * module, uint8_t category)
+void ENFORCE_FORMAT(1, 0) GenericLog(const char * format, va_list arg, const char * module, uint8_t category)
 {
 
 #if K32W_LOG_ENABLED
@@ -135,7 +136,7 @@ namespace Platform {
 /**
  * CHIP log output function.
  */
-void LogV(const char * module, uint8_t category, const char * msg, va_list v)
+void ENFORCE_FORMAT(3, 0) LogV(const char * module, uint8_t category, const char * msg, va_list v)
 {
     (void) module;
     (void) category;

diff --git a/src/platform/qpg/Logging.cpp b/src/platform/qpg/Logging.cpp
@@ -5,6 +5,7 @@
 
 #include <lib/core/CHIPConfig.h>
 #include <lib/support/CHIPPlatformMemory.h>
+#include <lib/support/EnforceFormat.h>
 #include <lib/support/logging/Constants.h>
 #include <platform/CHIPDeviceConfig.h>
 
@@ -42,7 +43,7 @@ namespace Platform {
  * CHIP log output function.
  */
 
-void LogV(const char * module, uint8_t category, const char * msg, va_list v)
+void ENFORCE_FORMAT(3, 0) LogV(const char * module, uint8_t category, const char * msg, va_list v)
 {
     char formattedMsg[CHIP_CONFIG_LOG_MESSAGE_MAX_SIZE];
     size_t prefixLen;