diff --git a/src/lib/core/ErrorStr.cpp b/src/lib/core/ErrorStr.cpp index 6d83bc015b21d2..b3cf968c674dd5 100644 --- a/src/lib/core/ErrorStr.cpp +++ b/src/lib/core/ErrorStr.cpp @@ -29,7 +29,7 @@ namespace chip { /** * Static buffer to store the formatted error string. */ -static char sErrorStr[CHIP_CONFIG_ERROR_STR_SIZE]; +static ErrorStrStorage sErrorStr; /** * Linked-list of error formatter functions. @@ -38,7 +38,7 @@ static ErrorFormatter * sErrorFormatterList = nullptr; /** * This routine returns a human-readable NULL-terminated C string - * describing the provided error. + * describing the provided error. This uses the global static storage. * * @param[in] err The error for format and describe. * @param[in] withSourceLocation Whether or not to include the source @@ -50,8 +50,26 @@ static ErrorFormatter * sErrorFormatterList = nullptr; */ DLL_EXPORT const char * ErrorStr(CHIP_ERROR err, bool withSourceLocation) { - char * formattedError = sErrorStr; - uint16_t formattedSpace = sizeof(sErrorStr); + return ErrorStr(err, withSourceLocation, sErrorStr); +} + +/** + * This routine writess a human-readable NULL-terminated C string into the buf + * which describes the provided error. + * + * @param[in] err The error for format and describe. + * @param[in] withSourceLocation Whether or not to include the source + * @param[in] storage ErrorStrStorage to write into + * location in the output string. Only used if CHIP_CONFIG_ERROR_SOURCE && + * !CHIP_CONFIG_SHORT_ERROR_STR. Defaults to true. + * + * @return A pointer to a NULL-terminated C string describing the + * provided error. + */ +DLL_EXPORT const char * ErrorStr(CHIP_ERROR err, bool withSourceLocation, ErrorStrStorage & storage) +{ + char * formattedError = storage.buff; + uint16_t formattedSpace = storage.kBufferSize; #if CHIP_CONFIG_ERROR_SOURCE && !CHIP_CONFIG_SHORT_ERROR_STR @@ -65,34 +83,27 @@ DLL_EXPORT const char * ErrorStr(CHIP_ERROR err, bool withSourceLocation) formattedError += n; formattedSpace = static_cast(formattedSpace - n); } - if (err == CHIP_NO_ERROR) - { - (void) snprintf(formattedError, formattedSpace, CHIP_NO_ERROR_STRING); - return sErrorStr; - } - -#else // CHIP_CONFIG_ERROR_SOURCE && !CHIP_CONFIG_SHORT_ERROR_STR +#endif // CHIP_CONFIG_ERROR_SOURCE && !CHIP_CONFIG_SHORT_ERROR_STR if (err == CHIP_NO_ERROR) { - return CHIP_NO_ERROR_STRING; + (void) snprintf(formattedError, formattedSpace, CHIP_NO_ERROR_STRING); + return storage.buff; } -#endif // CHIP_CONFIG_ERROR_SOURCE && !CHIP_CONFIG_SHORT_ERROR_STR - // Search the registered error formatter for one that will format the given // error code. for (const ErrorFormatter * errFormatter = sErrorFormatterList; errFormatter != nullptr; errFormatter = errFormatter->Next) { if (errFormatter->FormatError(formattedError, formattedSpace, err)) { - return sErrorStr; + return storage.buff; } } // Use a default formatting if no formatter found. FormatError(formattedError, formattedSpace, nullptr, err, nullptr); - return sErrorStr; + return storage.buff; } /** diff --git a/src/lib/core/ErrorStr.h b/src/lib/core/ErrorStr.h index d67680c2151b1e..f1365a6bfb25f3 100644 --- a/src/lib/core/ErrorStr.h +++ b/src/lib/core/ErrorStr.h @@ -48,7 +48,14 @@ struct ErrorFormatter ErrorFormatter * Next; }; +struct ErrorStrStorage +{ + char buff[CHIP_CONFIG_ERROR_STR_SIZE]; + constexpr static uint16_t kBufferSize = sizeof(buff); +}; + extern const char * ErrorStr(CHIP_ERROR err, bool withSourceLocation = true); +extern const char * ErrorStr(CHIP_ERROR err, bool withSourceLocation, ErrorStrStorage & storage); extern void RegisterErrorFormatter(ErrorFormatter * errFormatter); extern void DeregisterErrorFormatter(ErrorFormatter * errFormatter); extern void FormatError(char * buf, uint16_t bufSize, const char * subsys, CHIP_ERROR err, const char * desc); diff --git a/src/lib/core/tests/TestCHIPErrorStr.cpp b/src/lib/core/tests/TestCHIPErrorStr.cpp index 01130aca4261d7..1ce158cc8594ae 100644 --- a/src/lib/core/tests/TestCHIPErrorStr.cpp +++ b/src/lib/core/tests/TestCHIPErrorStr.cpp @@ -170,6 +170,31 @@ static const CHIP_ERROR kTestElements[] = }; // clang-format on +void CheckCoreErrorStrHelper(const char * errStr, CHIP_ERROR err) +{ + char expectedText[9]; + + // Assert that the error string contains the error number in hex. + snprintf(expectedText, sizeof(expectedText), "%08" PRIX32, static_cast(err.AsInteger())); + EXPECT_TRUE((strstr(errStr, expectedText) != nullptr)); + +#if !CHIP_CONFIG_SHORT_ERROR_STR + // Assert that the error string contains a description, which is signaled + // by a presence of a colon proceeding the description. + EXPECT_TRUE((strchr(errStr, ':') != nullptr)); +#endif // !CHIP_CONFIG_SHORT_ERROR_STR + +#if CHIP_CONFIG_ERROR_SOURCE + // GetFile() should be relative to ${chip_root} + char const * const file = err.GetFile(); + ASSERT_NE(file, nullptr); + EXPECT_EQ(strstr(file, "src/lib/core/"), file); + + // File should be included in the error. + EXPECT_NE(strstr(errStr, file), nullptr); +#endif // CHIP_CONFIG_ERROR_SOURCE +} + TEST(TestCHIPErrorStr, CheckCoreErrorStr) { // Register the layer error formatter @@ -179,29 +204,46 @@ TEST(TestCHIPErrorStr, CheckCoreErrorStr) // For each defined error... for (const auto & err : kTestElements) { - const char * errStr = ErrorStr(err); - char expectedText[9]; + // ErrorStr with static char array. + CheckCoreErrorStrHelper(ErrorStr(err, /*withSourceLocation=*/true), err); + } +} + +TEST(TestCHIPErrorStr, CheckCoreErrorStrStorage) +{ + // Register the layer error formatter + + RegisterCHIPLayerErrorFormatter(); + + // For each defined error... + for (const auto & err : kTestElements) + { + // ErrorStr with given storage. + ErrorStrStorage storage; + CheckCoreErrorStrHelper(ErrorStr(err, /*withSourceLocation=*/true, storage), err); + } +} + +void CheckCoreErrorStrWithoutSourceLocationHelper(const char * errStr, CHIP_ERROR err) +{ + char expectedText[9]; - // Assert that the error string contains the error number in hex. - snprintf(expectedText, sizeof(expectedText), "%08" PRIX32, static_cast(err.AsInteger())); - EXPECT_TRUE((strstr(errStr, expectedText) != nullptr)); + // Assert that the error string contains the error number in hex. + snprintf(expectedText, sizeof(expectedText), "%08" PRIX32, static_cast(err.AsInteger())); + EXPECT_TRUE((strstr(errStr, expectedText) != nullptr)); #if !CHIP_CONFIG_SHORT_ERROR_STR - // Assert that the error string contains a description, which is signaled - // by a presence of a colon proceeding the description. - EXPECT_TRUE((strchr(errStr, ':') != nullptr)); + // Assert that the error string contains a description, which is signaled + // by a presence of a colon proceeding the description. + EXPECT_TRUE((strchr(errStr, ':') != nullptr)); #endif // !CHIP_CONFIG_SHORT_ERROR_STR #if CHIP_CONFIG_ERROR_SOURCE - // GetFile() should be relative to ${chip_root} - char const * const file = err.GetFile(); - ASSERT_NE(file, nullptr); - EXPECT_EQ(strstr(file, "src/lib/core/"), file); - - // File should be included in the error. - EXPECT_NE(strstr(errStr, file), nullptr); + char const * const file = err.GetFile(); + ASSERT_NE(file, nullptr); + // File should not be included in the error. + EXPECT_EQ(strstr(errStr, file), nullptr); #endif // CHIP_CONFIG_ERROR_SOURCE - } } TEST(TestCHIPErrorStr, CheckCoreErrorStrWithoutSourceLocation) @@ -213,24 +255,22 @@ TEST(TestCHIPErrorStr, CheckCoreErrorStrWithoutSourceLocation) // For each defined error... for (const auto & err : kTestElements) { - const char * errStr = ErrorStr(err, /*withSourceLocation=*/false); - char expectedText[9]; + // ErrorStr with static char array. + CheckCoreErrorStrWithoutSourceLocationHelper(ErrorStr(err, /*withSourceLocation=*/false), err); + } +} - // Assert that the error string contains the error number in hex. - snprintf(expectedText, sizeof(expectedText), "%08" PRIX32, static_cast(err.AsInteger())); - EXPECT_TRUE((strstr(errStr, expectedText) != nullptr)); +TEST(TestCHIPErrorStr, CheckCoreErrorStrStorageWithoutSourceLocation) +{ + // Register the layer error formatter -#if !CHIP_CONFIG_SHORT_ERROR_STR - // Assert that the error string contains a description, which is signaled - // by a presence of a colon proceeding the description. - EXPECT_TRUE((strchr(errStr, ':') != nullptr)); -#endif // !CHIP_CONFIG_SHORT_ERROR_STR + RegisterCHIPLayerErrorFormatter(); -#if CHIP_CONFIG_ERROR_SOURCE - char const * const file = err.GetFile(); - ASSERT_NE(file, nullptr); - // File should not be included in the error. - EXPECT_EQ(strstr(errStr, file), nullptr); -#endif // CHIP_CONFIG_ERROR_SOURCE + // For each defined error... + for (const auto & err : kTestElements) + { + // ErrorStr with given storage. + ErrorStrStorage storage; + CheckCoreErrorStrWithoutSourceLocationHelper(ErrorStr(err, /*withSourceLocation=*/false, storage), err); } } diff --git a/src/lib/support/tests/TestErrorStr.cpp b/src/lib/support/tests/TestErrorStr.cpp index 809d69b302af0b..ac8a9f5e9e3469 100644 --- a/src/lib/support/tests/TestErrorStr.cpp +++ b/src/lib/support/tests/TestErrorStr.cpp @@ -121,6 +121,13 @@ TEST(TestErrorStr, CheckNoError) EXPECT_STREQ(CHECK_AND_SKIP_SOURCE(ErrorStr(CHIP_NO_ERROR)), CHIP_NO_ERROR_STRING); } +TEST(TestErrorStr, CheckErrorWithProvidedStorage) +{ + ErrorStrStorage storage; + EXPECT_STREQ(CHECK_AND_SKIP_SOURCE(ErrorStr(CHIP_NO_ERROR, true, storage)), CHIP_NO_ERROR_STRING); + EXPECT_STREQ(CHECK_AND_SKIP_SOURCE(ErrorStr(CHIP_ERROR_INTERNAL, true, storage)), "Error 0x000000AC"); +} + TEST(TestErrorStr, CheckFormatErr) { #if CHIP_CONFIG_SHORT_ERROR_STR