Add withSourceLocation param to ErrorStr (#33369)

* Add withSourceLocation param to ErrorStr This allows callers to choose at runtime whether or not to include source locations in error strings. * Add tests * Restyled by whitespace * Restyled by clang-format * Remove iostream import This got forgotten from debugging. * Restyled by whitespace --------- Co-authored-by: Restyled.io <[email protected]>
project-chip · May 15, 2024 · 83d35a3 · 83d35a3
1 parent 8c9ef98
commit 83d35a3
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 9 deletions.
diff --git a/src/lib/core/CHIPError.h b/src/lib/core/CHIPError.h
@@ -35,9 +35,9 @@
 #include <limits>
 #include <type_traits>
 
-#if __cplusplus >= 202002L
+#if CHIP_CONFIG_ERROR_SOURCE && __cplusplus >= 202002L
 #include <source_location>
-#endif // __cplusplus >= 202002L
+#endif // CHIP_CONFIG_ERROR_SOURCE && __cplusplus >= 202002L
 
 namespace chip {
 
@@ -239,10 +239,10 @@ class ChipError
      * @note
      *  Normally, prefer to use Format()
      */
-    const char * AsString() const
+    const char * AsString(bool withSourceLocation = true) const
     {
-        extern const char * ErrorStr(ChipError);
-        return ErrorStr(*this);
+        extern const char * ErrorStr(ChipError, bool);
+        return ErrorStr(*this, withSourceLocation);
     }
 
     /**

diff --git a/src/lib/core/ErrorStr.cpp b/src/lib/core/ErrorStr.cpp
@@ -41,19 +41,21 @@ static ErrorFormatter * sErrorFormatterList = nullptr;
  * describing the provided error.
  *
  * @param[in] err                      The error for format and describe.
+ * @param[in] withSourceLocation       Whether or not to include the source
+ * 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)
+DLL_EXPORT const char * ErrorStr(CHIP_ERROR err, bool withSourceLocation)
 {
     char * formattedError   = sErrorStr;
     uint16_t formattedSpace = sizeof(sErrorStr);
 
 #if CHIP_CONFIG_ERROR_SOURCE && !CHIP_CONFIG_SHORT_ERROR_STR
 
-    const char * const file = err.GetFile();
-    if (file != nullptr)
+    if (const char * const file = err.GetFile(); withSourceLocation && file != nullptr)
     {
         int n = snprintf(formattedError, formattedSpace, "%s:%u: ", file, err.GetLine());
         if (n > formattedSpace)

diff --git a/src/lib/core/ErrorStr.h b/src/lib/core/ErrorStr.h
@@ -48,7 +48,7 @@ struct ErrorFormatter
     ErrorFormatter * Next;
 };
 
-extern const char * ErrorStr(CHIP_ERROR err);
+extern const char * ErrorStr(CHIP_ERROR err, bool withSourceLocation = true);
 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
@@ -195,6 +195,40 @@ TEST(TestCHIPErrorStr, CheckCoreErrorStr)
         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, CheckCoreErrorStrWithoutSourceLocation)
+{
+    // Register the layer error formatter
+
+    RegisterCHIPLayerErrorFormatter();
+
+    // For each defined error...
+    for (const auto & err : kTestElements)
+    {
+        const char * errStr = ErrorStr(err, /*withSourceLocation=*/false);
+        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
+        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
     }
 }