Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
bzbarsky-apple committed Jan 6, 2022
1 parent d3b975b commit 5ee66b3
Show file tree
Hide file tree
Showing 24 changed files with 69 additions and 24 deletions.
1 change: 1 addition & 0 deletions src/app/tests/TestEventLogging.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
1 change: 1 addition & 0 deletions src/app/tests/TestMessageDef.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
1 change: 1 addition & 0 deletions src/app/tests/integration/common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
1 change: 1 addition & 0 deletions src/lib/core/CHIPTLV.h
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
1 change: 1 addition & 0 deletions src/lib/shell/streamer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

#include "streamer.h"

#include <lib/support/EnforceFormat.h>
#include <lib/support/logging/Constants.h>
#include <limits.h>
#include <stdio.h>
Expand Down
5 changes: 5 additions & 0 deletions src/lib/support/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down Expand Up @@ -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",
Expand Down
1 change: 1 addition & 0 deletions src/lib/support/CHIPArgParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>

/*
Expand Down
1 change: 1 addition & 0 deletions src/lib/support/DefaultStorageKeyAllocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand Down
34 changes: 34 additions & 0 deletions src/lib/support/EnforceFormat.h
Original file line number Diff line number Diff line change
@@ -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__
1 change: 1 addition & 0 deletions src/lib/support/logging/CHIPLogging.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@

#include <platform/logging/LogV.h>

#include <lib/support/EnforceFormat.h>
#include <lib/support/logging/Constants.h>

#include <inttypes.h>
Expand Down
16 changes: 0 additions & 16 deletions src/lib/support/logging/Constants.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down
1 change: 1 addition & 0 deletions src/lib/support/tests/TestCHIPArgParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
1 change: 1 addition & 0 deletions src/platform/Darwin/Logging.cpp
Original file line number Diff line number Diff line change
@@ -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>

Expand Down
3 changes: 2 additions & 1 deletion src/platform/ESP32/Logging.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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];

Expand Down
1 change: 1 addition & 0 deletions src/platform/Linux/Logging.cpp
Original file line number Diff line number Diff line change
@@ -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>

Expand Down
3 changes: 2 additions & 1 deletion src/platform/Tizen/Logging.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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] = {
Expand Down
3 changes: 2 additions & 1 deletion src/platform/Zephyr/Logging.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 2 additions & 0 deletions src/platform/logging/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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",
]
Expand All @@ -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",
]
Expand Down
3 changes: 2 additions & 1 deletion src/platform/logging/LogV.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#pragma once

#include <lib/support/EnforceFormat.h>
#include <stdarg.h>
#include <stdint.h>

Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions src/platform/logging/impl/android/Logging.cpp
Original file line number Diff line number Diff line change
@@ -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>

Expand Down
1 change: 1 addition & 0 deletions src/platform/logging/impl/stdio/Logging.cpp
Original file line number Diff line number Diff line change
@@ -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>

Expand Down
3 changes: 2 additions & 1 deletion src/platform/mbed/Logging.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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);
Expand Down
5 changes: 3 additions & 2 deletions src/platform/nxp/k32w/k32w0/Logging.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion src/platform/qpg/Logging.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 5ee66b3

Please sign in to comment.