Restructure fatal error formatting.

Split printFatalError() in errorformat.h to separate functions that
print the error header, (part of) the actual message, and the footer.
This makes it possible to print the message in calling code
incrementally instead of formatting it into a string buffer.
It is easier to print the message in parts this way, and there is also
less complications in handling possible std::bad_alloc errors from
manipulating a std::string.

Also added line wrapping functionality to the message writing function,
and improved the appearance of the header.

Part of #838 and #985.

Change-Id: Ief7399d8c7c2c0529e0bc8492726bff827e18c4c

diff --git a/src/gromacs/utility/errorcodes.cpp b/src/gromacs/utility/errorcodes.cpp
index 4d2a207ba9d174c3ba64fa53ce7ce2aacce2c280..da02091926eac63832968cd1beae02957c73f0f6 100644 (file)
--- a/src/gromacs/utility/errorcodes.cpp
+++ b/src/gromacs/utility/errorcodes.cpp
@@ -82,7 +82,9 @@ void standardErrorHandler(int retcode, const char *msg,
                           const char *file, int line)
 {
     const char *title = getErrorCodeString(retcode);
-    internal::printFatalError(stderr, title, msg, NULL, file, line);
+    internal::printFatalErrorHeader(stderr, title, NULL, file, line);
+    internal::printFatalErrorMessageLine(stderr, msg, 0);
+    internal::printFatalErrorFooter(stderr);
     std::exit(1);
 }
 

diff --git a/src/gromacs/utility/errorformat.cpp b/src/gromacs/utility/errorformat.cpp
index ad9643b08ac533a51b43983cee49f8e168d5c437..546145fc1bdeb94de8052e66f35da0d1311efb01 100644 (file)
--- a/src/gromacs/utility/errorformat.cpp
+++ b/src/gromacs/utility/errorformat.cpp
@@ -35,9 +35,15 @@
  * \author Teemu Murtola <teemu.murtola@cbr.su.se>
  * \ingroup module_utility
  */
-#include "gromacs/utility/errorformat.h"
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
 
+#include "errorformat.h"
+
+#include <cctype>
 #include <cstdio>
+#include <cstring>
 
 #include "gromacs/legacyheaders/copyrite.h"
 
@@ -51,8 +57,8 @@ namespace gmx
 namespace internal
 {
 
-void printFatalError(FILE *fp, const char *title, const char *details,
-                     const char *func, const char *file, int line)
+void printFatalErrorHeader(FILE *fp, const char *title,
+                           const char *func, const char *file, int line)
 {
     // In case ProgramInfo is not initialized and there is an issue with the
     // initialization, fall back to "GROMACS".
@@ -66,20 +72,51 @@ void printFatalError(FILE *fp, const char *title, const char *details,
     }
 
     std::fprintf(fp, "\n-------------------------------------------------------\n");
-    std::fprintf(fp, "Program %s, %s\n", programName, GromacsVersion());
-    if (func != NULL)
+    std::fprintf(fp, "Program:     %s, %s\n", programName, GromacsVersion());
+    if (file != NULL)
     {
-        std::fprintf(fp, "In function: %s\n", func);
+        // TODO: Check whether this works on Windows. If it doesn't, perhaps
+        // add Path::startsWith().
+        if (startsWith(file, CMAKE_SOURCE_DIR))
+        {
+            file += std::strlen(CMAKE_SOURCE_DIR);
+            if (file[0] == '/' || file[0] == '\\')
+            {
+                ++file;
+            }
+        }
+        std::fprintf(fp, "Source file: %s (line %d)\n", file, line);
     }
-    // TODO: Strip away absolute paths from file names (CMake seems to generate those)
-    if (file != NULL)
+    if (func != NULL)
     {
-        std::fprintf(fp, "Source file %s, line %d\n", file, line);
+        std::fprintf(fp, "Function:    %s\n", func);
     }
     std::fprintf(fp, "\n");
     std::fprintf(fp, "%s:\n", title);
-    // TODO: Line wrapping
-    std::fprintf(fp, "%s\n", details);
+}
+
+void printFatalErrorMessageLine(FILE *fp, const char *text, int indent)
+{
+    gmx::TextLineWrapper wrapper;
+    wrapper.settings().setLineLength(78 - indent);
+    size_t lineStart = 0;
+    size_t length = std::strlen(text);
+    while (lineStart < length)
+    {
+        size_t nextLineStart = wrapper.findNextLine(text, lineStart);
+        int lineLength = static_cast<int>(nextLineStart - lineStart);
+        while (lineLength > 0 && std::isspace(text[lineStart + lineLength - 1]))
+        {
+            --lineLength;
+        }
+        std::fprintf(fp, "%*s%.*s\n", indent, "", lineLength, text + lineStart);
+        lineStart = nextLineStart;
+    }
+}
+
+void printFatalErrorFooter(FILE *fp)
+{
+    std::fprintf(fp, "\n");
     std::fprintf(fp, "For more information and tips for troubleshooting, please check the GROMACS\n"
                      "website at http://www.gromacs.org/Documentation/Errors");
     std::fprintf(fp, "\n-------------------------------------------------------\n");

diff --git a/src/gromacs/utility/errorformat.h b/src/gromacs/utility/errorformat.h
index 30da26002163157b07d5c320aeccbb13b95660a3..e4b2bab47a3ca5dc78701787d7f698fd8a960f3f 100644 (file)
--- a/src/gromacs/utility/errorformat.h
+++ b/src/gromacs/utility/errorformat.h
@@ -48,14 +48,30 @@ namespace internal
 {
 
 /*! \internal \brief
- * Formats common headers and footers for error messages.
+ * Formats a common header for fatal error messages.
  *
  * Does not throw.
  *
  * \ingroup module_utility
  */
-void printFatalError(FILE *fp, const char *title, const char *details,
-                     const char *func, const char *file, int line);
+void printFatalErrorHeader(FILE *fp, const char *title,
+                           const char *func, const char *file, int line);
+/*! \internal \brief
+ * Formats a line of fatal error message text.
+ *
+ * Does not throw.
+ *
+ * \ingroup module_utility
+ */
+void printFatalErrorMessageLine(FILE *fp, const char *text, int indent);
+/*! \internal \brief
+ * Formats a common footer for fatal error messages.
+ *
+ * Does not throw.
+ *
+ * \ingroup module_utility
+ */
+void printFatalErrorFooter(FILE *fp);
 
 } // namespace internal
 //! \endcond

diff --git a/src/gromacs/utility/exceptions.cpp b/src/gromacs/utility/exceptions.cpp
index 3244e7d33bed420f44062312abe5e3321f800613..88172fcaea8a5447e019b64893876914152120a7 100644 (file)
--- a/src/gromacs/utility/exceptions.cpp
+++ b/src/gromacs/utility/exceptions.cpp
@@ -131,11 +131,13 @@ void printFatalErrorMessage(FILE *fp, const std::exception &ex)
         filePtr = boost::get_error_info<boost::throw_file>(*boostEx);
         linePtr = boost::get_error_info<boost::throw_line>(*boostEx);
     }
+    internal::printFatalErrorHeader(fp, title,
+                                    funcPtr != NULL ? *funcPtr : NULL,
+                                    filePtr != NULL ? *filePtr : NULL,
+                                    linePtr != NULL ? *linePtr : 0);
+    internal::printFatalErrorMessageLine(fp, ex.what(), 0);
     // TODO: Treat errno information in boost exceptions
-    internal::printFatalError(fp, title, ex.what(),
-                              funcPtr != NULL ? *funcPtr : NULL,
-                              filePtr != NULL ? *filePtr : NULL,
-                              linePtr != NULL ? *linePtr : 0);
+    internal::printFatalErrorFooter(fp);
 }
 
 } // namespace gmx

diff --git a/src/gromacs/utility/gmxassert.cpp b/src/gromacs/utility/gmxassert.cpp
index 2f4f2ed6602ef246c64441a6f6aa843c80b41058..018dcd110b6eb91dda3588ab6651c33086ab0cb2 100644 (file)
--- a/src/gromacs/utility/gmxassert.cpp
+++ b/src/gromacs/utility/gmxassert.cpp
@@ -35,15 +35,11 @@
  * \author Teemu Murtola <teemu.murtola@cbr.su.se>
  * \ingroup module_utility
  */
-#include "gromacs/utility/gmxassert.h"
+#include "gmxassert.h"
 
 #include <cstdio>
 #include <cstdlib>
 
-#include <string>
-
-#include "gromacs/utility/stringutil.h"
-
 #include "errorformat.h"
 
 namespace gmx
@@ -56,18 +52,10 @@ namespace internal
 void assertHandler(const char *condition, const char *msg,
                    const char *func, const char *file, int line)
 {
-    try
-    {
-        std::string title = formatString("Condition: %s\n%s", condition, msg);
-        printFatalError(stderr, "Assertion failed", title.c_str(),
-                        func, file, line);
-    }
-    catch (const std::bad_alloc &)
-    {
-        printFatalError(stderr, "Assertion failed",
-                "(memory allocation failed while formatting the error message)",
-                func, file, line);
-    }
+    printFatalErrorHeader(stderr, "Assertion failed", func, file, line);
+    std::fprintf(stderr, "Condition: %s\n", condition);
+    printFatalErrorMessageLine(stderr, msg, 0);
+    printFatalErrorFooter(stderr);
     std::abort();
 }
 

diff --git a/src/gromacs/utility/stringutil.h b/src/gromacs/utility/stringutil.h
index dc6b7a8c4a593679832e0ca9456c3a1560fff49c..0dd731771be673aa23b736acc37919f5f60c1cda 100644 (file)
--- a/src/gromacs/utility/stringutil.h
+++ b/src/gromacs/utility/stringutil.h
@@ -39,6 +39,8 @@
 #ifndef GMX_UTILITY_STRINGUTIL_H
 #define GMX_UTILITY_STRINGUTIL_H
 
+#include <cstring>
+
 #include <string>
 #include <vector>
 
@@ -61,6 +63,11 @@ bool inline startsWith(const std::string &str, const std::string &prefix)
 {
     return str.compare(0, prefix.length(), prefix) == 0;
 }
+//! \copydoc startsWith(const std::string &, const std::string &)
+bool inline startsWith(const char *str, const char *prefix)
+{
+    return std::strncmp(str, prefix, std::strlen(prefix)) == 0;
+}
 
 /*! \brief
  * Tests whether a string ends with another string.

diff --git a/src/gromacs/utility/tests/stringutil.cpp b/src/gromacs/utility/tests/stringutil.cpp
index 39c60da1f1e5d2f356d78ca8aeb44fc93d0f0b82..b1c317076440385f2c26eeea479193309cfc19d7 100644 (file)
--- a/src/gromacs/utility/tests/stringutil.cpp
+++ b/src/gromacs/utility/tests/stringutil.cpp
@@ -64,6 +64,12 @@ TEST(StringUtilityTest, StartsWithWorks)
     EXPECT_FALSE(gmx::startsWith("", "foobar"));
     EXPECT_FALSE(gmx::startsWith("foo", "foobar"));
     EXPECT_FALSE(gmx::startsWith("foobar", "oob"));
+    EXPECT_TRUE(gmx::startsWith(std::string("foobar"), "foo"));
+    EXPECT_TRUE(gmx::startsWith(std::string("foobar"), ""));
+    EXPECT_TRUE(gmx::startsWith(std::string(""), ""));
+    EXPECT_FALSE(gmx::startsWith(std::string(""), "foobar"));
+    EXPECT_FALSE(gmx::startsWith(std::string("foo"), "foobar"));
+    EXPECT_FALSE(gmx::startsWith(std::string("foobar"), "oob"));
 }
 
 TEST(StringUtilityTest, EndsWithWorks)