Add a helper function to allow ErrorStr not using static char array (#36391)
* Add a helper function to allow ErrorStr does not rely on static char
array
* Use dedicated storage for ErrorStr and add unit tests
* Restyled by whitespace
* Restyled by clang-format
* remove size()
---------
Co-authored-by: Restyled.io <commits@restyled.io>
diff --git a/src/lib/core/ErrorStr.cpp b/src/lib/core/ErrorStr.cpp
index 6d83bc0..b3cf968 100644
--- a/src/lib/core/ErrorStr.cpp
+++ b/src/lib/core/ErrorStr.cpp
@@ -29,7 +29,7 @@
/**
* 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 @@
/**
* 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 @@
*/
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 @@
formattedError += n;
formattedSpace = static_cast<uint16_t>(formattedSpace - n);
}
+#endif // CHIP_CONFIG_ERROR_SOURCE && !CHIP_CONFIG_SHORT_ERROR_STR
+
if (err == CHIP_NO_ERROR)
{
(void) snprintf(formattedError, formattedSpace, CHIP_NO_ERROR_STRING);
- return sErrorStr;
+ return storage.buff;
}
-#else // CHIP_CONFIG_ERROR_SOURCE && !CHIP_CONFIG_SHORT_ERROR_STR
-
- if (err == CHIP_NO_ERROR)
- {
- return CHIP_NO_ERROR_STRING;
- }
-
-#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 d67680c..f1365a6 100644
--- a/src/lib/core/ErrorStr.h
+++ b/src/lib/core/ErrorStr.h
@@ -48,7 +48,14 @@
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 01130ac..1ce158c 100644
--- a/src/lib/core/tests/TestCHIPErrorStr.cpp
+++ b/src/lib/core/tests/TestCHIPErrorStr.cpp
@@ -170,6 +170,31 @@
};
// 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<uint32_t>(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 @@
// 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);
+ }
+}
- // Assert that the error string contains the error number in hex.
- snprintf(expectedText, sizeof(expectedText), "%08" PRIX32, static_cast<uint32_t>(err.AsInteger()));
- EXPECT_TRUE((strstr(errStr, expectedText) != nullptr));
+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<uint32_t>(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 @@
// 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<uint32_t>(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 809d69b..ac8a9f5 100644
--- a/src/lib/support/tests/TestErrorStr.cpp
+++ b/src/lib/support/tests/TestErrorStr.cpp
@@ -121,6 +121,13 @@
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
