Skip to content

Commit

Permalink
Clean up defines in src/platform/BUILD.gn (#7575)
Browse files Browse the repository at this point in the history
- Separate true/false defines are no longer required, as the header
  generator maps "true" to 1 and "false" to 0.

- We should be setting false config #defines to 0 so that we
  can build with -Wundef

Move defines into the main list where possible so that they are always
added to the generated header.
  • Loading branch information
mspang authored and pull[bot] committed Sep 1, 2021
1 parent d76d6fa commit 56343b8
Show file tree
Hide file tree
Showing 7 changed files with 39 additions and 56 deletions.
2 changes: 1 addition & 1 deletion src/include/platform/LockTracker.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
namespace chip {
namespace Platform {

#if defined(CHIP_STACK_LOCK_TRACKING_ENABLED)
#if CHIP_STACK_LOCK_TRACKING_ENABLED

namespace Internal {

Expand Down
4 changes: 2 additions & 2 deletions src/include/platform/PlatformManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ class PlatformManager
void UnlockChipStack();
CHIP_ERROR Shutdown();

#if defined(CHIP_STACK_LOCK_TRACKING_ENABLED)
#if CHIP_STACK_LOCK_TRACKING_ENABLED
bool IsChipStackLockedByCurrentThread() const;
#endif

Expand Down Expand Up @@ -187,7 +187,7 @@ extern PlatformManagerImpl & PlatformMgrImpl();
namespace chip {
namespace DeviceLayer {

#if defined(CHIP_STACK_LOCK_TRACKING_ENABLED)
#if CHIP_STACK_LOCK_TRACKING_ENABLED
inline bool PlatformManager::IsChipStackLockedByCurrentThread() const
{
return static_cast<const ImplClass *>(this)->_IsChipStackLockedByCurrentThread();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ void GenericPlatformManagerImpl_POSIX<ImplClass>::_LockChipStack()
int err = pthread_mutex_lock(&mChipStackLock);
assert(err == 0);

#if defined(CHIP_STACK_LOCK_TRACKING_ENABLED)
#if CHIP_STACK_LOCK_TRACKING_ENABLED
mChipStackIsLocked = true;
mChipStackLockOwnerThread = pthread_self();
#endif
Expand All @@ -93,7 +93,7 @@ template <class ImplClass>
bool GenericPlatformManagerImpl_POSIX<ImplClass>::_TryLockChipStack()
{
bool locked = (pthread_mutex_trylock(&mChipStackLock) == 0);
#if defined(CHIP_STACK_LOCK_TRACKING_ENABLED)
#if CHIP_STACK_LOCK_TRACKING_ENABLED
if (locked)
{
mChipStackIsLocked = true;
Expand All @@ -106,15 +106,15 @@ bool GenericPlatformManagerImpl_POSIX<ImplClass>::_TryLockChipStack()
template <class ImplClass>
void GenericPlatformManagerImpl_POSIX<ImplClass>::_UnlockChipStack()
{
#if defined(CHIP_STACK_LOCK_TRACKING_ENABLED)
#if CHIP_STACK_LOCK_TRACKING_ENABLED
mChipStackIsLocked = false;
#endif

int err = pthread_mutex_unlock(&mChipStackLock);
assert(err == 0);
}

#if defined(CHIP_STACK_LOCK_TRACKING_ENABLED)
#if CHIP_STACK_LOCK_TRACKING_ENABLED
template <class ImplClass>
bool GenericPlatformManagerImpl_POSIX<ImplClass>::_IsChipStackLockedByCurrentThread() const
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ class GenericPlatformManagerImpl_POSIX : public GenericPlatformManagerImpl<ImplC
pthread_attr_t mChipTaskAttr;
struct sched_param mChipTaskSchedParam;

#if defined(CHIP_STACK_LOCK_TRACKING_ENABLED)
#if CHIP_STACK_LOCK_TRACKING_ENABLED
bool mMainLoopStarted = false;
bool mChipStackIsLocked = false;
pthread_t mChipStackLockOwnerThread;
Expand All @@ -86,7 +86,7 @@ class GenericPlatformManagerImpl_POSIX : public GenericPlatformManagerImpl<ImplC
CHIP_ERROR _StartChipTimer(int64_t durationMS);
CHIP_ERROR _Shutdown();

#if defined(CHIP_STACK_LOCK_TRACKING_ENABLED)
#if CHIP_STACK_LOCK_TRACKING_ENABLED
bool _IsChipStackLockedByCurrentThread() const;
#endif

Expand Down
71 changes: 27 additions & 44 deletions src/platform/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -60,57 +60,56 @@ if (chip_device_platform != "none") {
chip_stack_lock_tracking = "auto"
}

if (chip_stack_lock_tracking == "auto") {
if (chip_device_platform == "linux" || chip_device_platform == "darwin") {
# TODO: should be fatal for development. Change once bugs are fixed
chip_stack_lock_tracking = "log"
} else {
# TODO: may want to enable at least logging for embedded to find bugs
# this needs tuning depending on how many resources various platforms have
# available (mainly flash size)
chip_stack_lock_tracking = "none"
}
} else {
assert(
chip_stack_lock_tracking == "none" ||
chip_stack_lock_tracking == "log" ||
chip_stack_lock_tracking == "fatal",
"Please select a valid value for chip_stack_lock_tracking: auto, none, log, fatal")
}

buildconfig_header("platform_buildconfig") {
header = "CHIPDeviceBuildConfig.h"
header_dir = "platform"

chip_with_gio = chip_enable_wifi
chip_device_config_enable_wpa = chip_enable_wifi
chip_device_config_enable_mdns = chip_mdns != "none"
chip_stack_lock_tracking_log = chip_stack_lock_tracking != "none"
chip_stack_lock_tracking_fatal = chip_stack_lock_tracking == "fatal"

defines = [
"CHIP_DEVICE_CONFIG_ENABLE_WPA=${chip_device_config_enable_wpa}",
"CHIP_ENABLE_OPENTHREAD=${chip_enable_openthread}",
"CHIP_WITH_GIO=${chip_with_gio}",
"OPENTHREAD_CONFIG_ENABLE_TOBLE=false",
"CONFIG_USE_CLUSTERS_FOR_IP_COMMISSIONING=${chip_use_clusters_for_ip_commissioning}",
"CHIP_DEVICE_CONFIG_ENABLE_MDNS=${chip_device_config_enable_mdns}",
"CHIP_BYPASS_RENDEZVOUS=${chip_bypass_rendezvous}",
"CHIP_STACK_LOCK_TRACKING_ENABLED=${chip_stack_lock_tracking_log}",
"CHIP_STACK_LOCK_TRACKING_ERROR_FATAL=${chip_stack_lock_tracking_fatal}",
"CHIP_ENABLE_ADDITIONAL_DATA_ADVERTISING=${chip_enable_additional_data_advertising}",
"CHIP_ENABLE_ROTATING_DEVICE_ID=${chip_enable_rotating_device_id}",
]

if (chip_device_platform == "linux" || chip_device_platform == "darwin") {
defines += [ "CHIP_DEVICE_CONFIG_ENABLE_CHIPOBLE=${chip_enable_ble}" ]
}

stack_log_tracking = chip_stack_lock_tracking
if (stack_log_tracking == "auto") {
if (chip_device_platform == "linux") {
# TODO: should be fatal for development. Change once bugs are fixed
stack_log_tracking = "log"
} else {
# TODO: may want to enable at least logging for embedded to find bugs
# this needs tuning depending on how many resources various platforms have
# available (mainly flash size)
stack_log_tracking = "none"
}
}

if (stack_log_tracking == "fatal") {
defines += [
"CHIP_STACK_LOCK_TRACKING_ENABLED=1",
"CHIP_STACK_LOCK_TRACKING_ERROR_FATAL=1",
]
} else if (stack_log_tracking == "log") {
defines += [ "CHIP_STACK_LOCK_TRACKING_ENABLED=1" ]
} else {
assert(stack_log_tracking == "none")
}

if (chip_enable_nfc) {
defines += [ "CHIP_DEVICE_CONFIG_ENABLE_NFC=1" ]
}

if (chip_mdns != "none") {
defines += [ "CHIP_DEVICE_CONFIG_ENABLE_MDNS=1" ]
}

if (chip_device_project_config_include != "") {
defines += [ "CHIP_DEVICE_PROJECT_CONFIG_INCLUDE=${chip_device_project_config_include}" ]
}
Expand All @@ -125,22 +124,6 @@ if (chip_device_platform != "none") {
defines += [ "CHIP_DEVICE_CONFIG_FIRMWARE_BUILD_TIME=\"${chip_device_config_firmware_build_time}\"" ]
}

if (chip_bypass_rendezvous) {
defines += [ "CHIP_BYPASS_RENDEZVOUS=1" ]
}

if (chip_enable_additional_data_advertising) {
defines += [ "CHIP_ENABLE_ADDITIONAL_DATA_ADVERTISING=1" ]
} else {
defines += [ "CHIP_ENABLE_ADDITIONAL_DATA_ADVERTISING=0" ]
}

if (chip_enable_rotating_device_id) {
defines += [ "CHIP_ENABLE_ROTATING_DEVICE_ID=1" ]
} else {
defines += [ "CHIP_ENABLE_ROTATING_DEVICE_ID=0" ]
}

if (chip_device_platform == "cc13x2_26x2") {
defines += [
"CHIP_DEVICE_LAYER_TARGET_CC13X2_26X2=1",
Expand Down
2 changes: 1 addition & 1 deletion src/platform/Darwin/PlatformManagerImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class PlatformManagerImpl final : public PlatformManager, public Internal::Gener
void _UnlockChipStack(){};
void _PostEvent(const ChipDeviceEvent * event);

#if defined(CHIP_STACK_LOCK_TRACKING_ENABLED)
#if CHIP_STACK_LOCK_TRACKING_ENABLED
bool _IsChipStackLockedByCurrentThread() const { return false; };
#endif

Expand Down
4 changes: 2 additions & 2 deletions src/platform/LockTracker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

#include <platform/LockTracker.h>

#if defined(CHIP_STACK_LOCK_TRACKING_ENABLED)
#if CHIP_STACK_LOCK_TRACKING_ENABLED

#include <platform/CHIPDeviceLayer.h>
#include <platform/PlatformManager.h>
Expand All @@ -32,7 +32,7 @@ void AssertChipStackLockedByCurrentThread(const char * file, int line)
if (!chip::DeviceLayer::PlatformMgr().IsChipStackLockedByCurrentThread())
{
ChipLogError(DeviceLayer, "Chip stack locking error at '%s:%d'. Code is unsafe/racy", file, line);
#if defined(CHIP_STACK_LOCK_TRACKING_ERROR_FATAL)
#if CHIP_STACK_LOCK_TRACKING_ERROR_FATAL
chipDie();
#endif
}
Expand Down

0 comments on commit 56343b8

Please sign in to comment.