Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix deregistering error formatter #37254

Merged
merged 2 commits into from
Feb 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions src/lib/core/CHIPError.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,24 @@

namespace chip {

static ErrorFormatter sCHIPErrorFormatter = { FormatCHIPError, nullptr };

/**
* Register a text error formatter for CHIP core errors.
*/
void RegisterCHIPLayerErrorFormatter()
{
static ErrorFormatter sCHIPErrorFormatter = { FormatCHIPError, nullptr };

RegisterErrorFormatter(&sCHIPErrorFormatter);
}

/**
* Deregister a text error formatter for CHIP core errors.
*/
void DeregisterCHIPLayerErrorFormatter()
{
DeregisterErrorFormatter(&sCHIPErrorFormatter);
}

/**
* Given a CHIP error, returns a human-readable NULL-terminated C string
* describing the error.
Expand Down
1 change: 1 addition & 0 deletions src/lib/core/CHIPError.h
Original file line number Diff line number Diff line change
Expand Up @@ -1821,6 +1821,7 @@ using CHIP_ERROR = ::chip::ChipError;
namespace chip {

extern void RegisterCHIPLayerErrorFormatter();
extern void DeregisterCHIPLayerErrorFormatter();
extern bool FormatCHIPError(char * buf, uint16_t bufSize, CHIP_ERROR err);

} // namespace chip
6 changes: 5 additions & 1 deletion src/lib/core/ErrorStr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -142,13 +142,17 @@ DLL_EXPORT void RegisterErrorFormatter(ErrorFormatter * errFormatter)
DLL_EXPORT void DeregisterErrorFormatter(ErrorFormatter * errFormatter)
{
// Remove the formatter if present
for (ErrorFormatter ** lfp = &sErrorFormatterList; *lfp != nullptr; lfp = &(*lfp)->Next)
for (ErrorFormatter ** lfp = &sErrorFormatterList; *lfp != nullptr;)
{
// Remove the formatter from the global list, if found.
if (*lfp == errFormatter)
{
*lfp = errFormatter->Next;
}
else
{
lfp = &(*lfp)->Next;
}
}
}

Expand Down
12 changes: 12 additions & 0 deletions src/lib/core/tests/TestCHIPErrorStr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,9 @@ TEST(TestCHIPErrorStr, CheckCoreErrorStr)
// ErrorStr with static char array.
CheckCoreErrorStrHelper(ErrorStr(err, /*withSourceLocation=*/true), err);
}

// Deregister the layer error formatter
DeregisterCHIPLayerErrorFormatter();
}

TEST(TestCHIPErrorStr, CheckCoreErrorStrStorage)
Expand All @@ -222,6 +225,9 @@ TEST(TestCHIPErrorStr, CheckCoreErrorStrStorage)
ErrorStrStorage storage;
CheckCoreErrorStrHelper(ErrorStr(err, /*withSourceLocation=*/true, storage), err);
}

// Deregister the layer error formatter
DeregisterCHIPLayerErrorFormatter();
}

void CheckCoreErrorStrWithoutSourceLocationHelper(const char * errStr, CHIP_ERROR err)
Expand Down Expand Up @@ -258,6 +264,9 @@ TEST(TestCHIPErrorStr, CheckCoreErrorStrWithoutSourceLocation)
// ErrorStr with static char array.
CheckCoreErrorStrWithoutSourceLocationHelper(ErrorStr(err, /*withSourceLocation=*/false), err);
}

// Deregister the layer error formatter
DeregisterCHIPLayerErrorFormatter();
}

TEST(TestCHIPErrorStr, CheckCoreErrorStrStorageWithoutSourceLocation)
Expand All @@ -273,4 +282,7 @@ TEST(TestCHIPErrorStr, CheckCoreErrorStrStorageWithoutSourceLocation)
ErrorStrStorage storage;
CheckCoreErrorStrWithoutSourceLocationHelper(ErrorStr(err, /*withSourceLocation=*/false, storage), err);
}

// Deregister the layer error formatter
DeregisterCHIPLayerErrorFormatter();
}
10 changes: 10 additions & 0 deletions src/lib/support/tests/TestErrorStr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,14 @@ static bool trueFormat(char * buf, uint16_t bufSize, CHIP_ERROR err)
return true; // means I handled it
}

TEST(TestErrorStr, CheckRegisterDeregisterSingleErrorFormatter)
{
static ErrorFormatter falseFormatter = { falseFormat, nullptr };

RegisterErrorFormatter(&falseFormatter);
DeregisterErrorFormatter(&falseFormatter);
}

TEST(TestErrorStr, CheckRegisterDeregisterErrorFormatter)
{
static ErrorFormatter falseFormatter = { falseFormat, nullptr };
Expand Down Expand Up @@ -114,6 +122,8 @@ TEST(TestErrorStr, CheckRegisterDeregisterErrorFormatter)

// verify this doesn't crash
DeregisterErrorFormatter(&trueFormatter);
DeregisterErrorFormatter(&falseFormatter);
DeregisterErrorFormatter(&falseFormatter2);
}

TEST(TestErrorStr, CheckNoError)
Expand Down
3 changes: 3 additions & 0 deletions src/system/tests/TestSystemErrorStr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,4 +69,7 @@ TEST(TestSystemErrorStr, CheckSystemErrorStr)
EXPECT_NE(strchr(errStr, ':'), nullptr);
#endif // !CHIP_CONFIG_SHORT_ERROR_STR
}

// Deregister the layer error formatter
DeregisterCHIPLayerErrorFormatter();
}
3 changes: 3 additions & 0 deletions src/system/tests/TestSystemPacketBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,9 @@ class TestSystemPacketBuffer : public ::testing::Test
{
chip::DeviceLayer::PlatformMgr().Shutdown();
chip::Platform::MemoryShutdown();

// Deregister the layer error formatter
DeregisterCHIPLayerErrorFormatter();
}

void SetUp()
Expand Down
Loading