[Lldb-commits] [lldb] r294210 - Get rid of Error::PutToLog().

Zachary Turner via lldb-commits lldb-commits at lists.llvm.org
Mon Feb 6 10:31:44 PST 2017


Author: zturner
Date: Mon Feb  6 12:31:44 2017
New Revision: 294210

URL: http://llvm.org/viewvc/llvm-project?rev=294210&view=rev
Log:
Get rid of Error::PutToLog().

Instead just rely on LLDB_LOG().

This is part of an effort to sort out dependency hell in LLDB.
Error is in Utility, but Log is in Core.  Core can depend on
Utility, but not vice versa.  So this patch moves the knowledge
about how to log Errors from the Error class to the Log file.

Differential Revision: https://reviews.llvm.org/D29514

Modified:
    lldb/trunk/include/lldb/Utility/Error.h
    lldb/trunk/source/Core/Communication.cpp
    lldb/trunk/source/Host/common/Host.cpp
    lldb/trunk/source/Plugins/OperatingSystem/Go/OperatingSystemGo.cpp
    lldb/trunk/source/Utility/Error.cpp

Modified: lldb/trunk/include/lldb/Utility/Error.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/Error.h?rev=294210&r1=294209&r2=294210&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Utility/Error.h (original)
+++ lldb/trunk/include/lldb/Utility/Error.h Mon Feb  6 12:31:44 2017
@@ -12,6 +12,7 @@
 #if defined(__cplusplus)
 
 #include "llvm/Support/DataTypes.h"
+#include "llvm/Support/FormatAdapters.h"
 #include "llvm/Support/FormatVariadic.h"
 
 #include <cstdarg>
@@ -24,8 +25,6 @@
 
 namespace lldb_private {
 
-class Log;
-
 //----------------------------------------------------------------------
 /// @class Error Error.h "lldb/Utility/Error.h"
 /// @brief An error handling class.
@@ -148,48 +147,6 @@ public:
   lldb::ErrorType GetType() const;
 
   //------------------------------------------------------------------
-  /// Log an error to Log().
-  ///
-  /// Log the error given a formatted string \a format. If the this
-  /// object contains an error code, update the error string to
-  /// contain the prefix "error: ", followed by the formatted string,
-  /// followed by the error value and any string that describes the
-  /// error value. This allows more context to be given to an error
-  /// string that remains cached in this object. Logging always occurs
-  /// even when the error code contains a non-error value.
-  ///
-  /// @param[in] format
-  ///     A printf style format string.
-  ///
-  /// @param[in] ...
-  ///     Variable arguments that are needed for the printf style
-  ///     format string \a format.
-  //------------------------------------------------------------------
-  void PutToLog(Log *log, const char *format, ...)
-      __attribute__((format(printf, 3, 4)));
-
-  //------------------------------------------------------------------
-  /// Log an error to Log() if the error value is an error.
-  ///
-  /// Log the error given a formatted string \a format only if the
-  /// error value in this object describes an error condition. If the
-  /// this object contains an error, update the error string to
-  /// contain the prefix "error: " followed by the formatted string,
-  /// followed by the error value and any string that describes the
-  /// error value. This allows more context to be given to an error
-  /// string that remains cached in this object.
-  ///
-  /// @param[in] format
-  ///     A printf style format string.
-  ///
-  /// @param[in] ...
-  ///     Variable arguments that are needed for the printf style
-  ///     format string \a format.
-  //------------------------------------------------------------------
-  void LogIfError(Log *log, const char *format, ...)
-      __attribute__((format(printf, 3, 4)));
-
-  //------------------------------------------------------------------
   /// Set accessor from a kern_return_t.
   ///
   /// Set accesssor for the error value to \a err and the error type
@@ -304,10 +261,7 @@ protected:
 namespace llvm {
 template <> struct format_provider<lldb_private::Error> {
   static void format(const lldb_private::Error &error, llvm::raw_ostream &OS,
-                     llvm::StringRef Options) {
-    llvm::format_provider<llvm::StringRef>::format(error.AsCString(), OS,
-                                                   Options);
-  }
+                     llvm::StringRef Options);
 };
 }
 

Modified: lldb/trunk/source/Core/Communication.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Communication.cpp?rev=294210&r1=294209&r2=294210&view=diff
==============================================================================
--- lldb/trunk/source/Core/Communication.cpp (original)
+++ lldb/trunk/source/Core/Communication.cpp Mon Feb  6 12:31:44 2017
@@ -322,10 +322,9 @@ lldb::thread_result_t Communication::Rea
         comm->Disconnect();
         done = true;
       }
-      if (log)
-        error.LogIfError(
-            log, "%p Communication::ReadFromConnection () => status = %s", p,
-            Communication::ConnectionStatusAsCString(status));
+      if (error.Fail())
+        LLDB_LOG(log, "error: {0}, status = {1}", error,
+                 Communication::ConnectionStatusAsCString(status));
       break;
     case eConnectionStatusInterrupted: // Synchronization signal from
                                        // SynchronizeWithReadThread()
@@ -340,10 +339,9 @@ lldb::thread_result_t Communication::Rea
       done = true;
       LLVM_FALLTHROUGH;
     case eConnectionStatusTimedOut: // Request timed out
-      if (log)
-        error.LogIfError(
-            log, "%p Communication::ReadFromConnection () => status = %s", p,
-            Communication::ConnectionStatusAsCString(status));
+      if (error.Fail())
+        LLDB_LOG(log, "error: {0}, status = {1}", error,
+                 Communication::ConnectionStatusAsCString(status));
       break;
     }
   }

Modified: lldb/trunk/source/Host/common/Host.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/Host.cpp?rev=294210&r1=294209&r2=294210&view=diff
==============================================================================
--- lldb/trunk/source/Host/common/Host.cpp (original)
+++ lldb/trunk/source/Host/common/Host.cpp Mon Feb  6 12:31:44 2017
@@ -685,10 +685,10 @@ Error Host::LaunchProcessPosixSpawn(cons
   posix_spawnattr_t attr;
   error.SetError(::posix_spawnattr_init(&attr), eErrorTypePOSIX);
 
-  if (error.Fail() || log)
-    error.PutToLog(log, "::posix_spawnattr_init ( &attr )");
-  if (error.Fail())
+  if (error.Fail()) {
+    LLDB_LOG(log, "error: {0}, ::posix_spawnattr_init ( &attr )", error);
     return error;
+  }
 
   // Make a quick class that will cleanup the posix spawn attributes in case
   // we return in the middle of this function.
@@ -709,11 +709,12 @@ Error Host::LaunchProcessPosixSpawn(cons
   short flags = GetPosixspawnFlags(launch_info);
 
   error.SetError(::posix_spawnattr_setflags(&attr, flags), eErrorTypePOSIX);
-  if (error.Fail() || log)
-    error.PutToLog(log, "::posix_spawnattr_setflags ( &attr, flags=0x%8.8x )",
-                   flags);
-  if (error.Fail())
+  if (error.Fail()) {
+    LLDB_LOG(log,
+             "error: {0}, ::posix_spawnattr_setflags ( &attr, flags={1:x} )",
+             error, flags);
     return error;
+  }
 
 // posix_spawnattr_setbinpref_np appears to be an Apple extension per:
 // http://www.unix.com/man-page/OSX/3/posix_spawnattr_setbinpref_np/
@@ -740,10 +741,10 @@ Error Host::LaunchProcessPosixSpawn(cons
       size_t ocount = 0;
       error.SetError(::posix_spawnattr_setbinpref_np(&attr, 1, &cpu, &ocount),
                      eErrorTypePOSIX);
-      if (error.Fail() || log)
-        error.PutToLog(log, "::posix_spawnattr_setbinpref_np ( &attr, 1, "
-                            "cpu_type = 0x%8.8x, count => %llu )",
-                       cpu, (uint64_t)ocount);
+      if (error.Fail())
+        LLDB_LOG(log, "error: {0}, ::posix_spawnattr_setbinpref_np ( &attr, 1, "
+                      "cpu_type = {1:x}, count => {2} )",
+                 error, cpu, ocount);
 
       if (error.Fail() || ocount != 1)
         return error;
@@ -813,10 +814,12 @@ Error Host::LaunchProcessPosixSpawn(cons
     posix_spawn_file_actions_t file_actions;
     error.SetError(::posix_spawn_file_actions_init(&file_actions),
                    eErrorTypePOSIX);
-    if (error.Fail() || log)
-      error.PutToLog(log, "::posix_spawn_file_actions_init ( &file_actions )");
-    if (error.Fail())
+    if (error.Fail()) {
+      LLDB_LOG(log,
+               "error: {0}, ::posix_spawn_file_actions_init ( &file_actions )",
+               error);
       return error;
+    }
 
     // Make a quick class that will cleanup the posix spawn attributes in case
     // we return in the middle of this function.
@@ -838,16 +841,14 @@ Error Host::LaunchProcessPosixSpawn(cons
         ::posix_spawnp(&result_pid, exe_path, &file_actions, &attr, argv, envp),
         eErrorTypePOSIX);
 
-    if (error.Fail() || log) {
-      error.PutToLog(
-          log, "::posix_spawnp ( pid => %i, path = '%s', file_actions = %p, "
-               "attr = %p, argv = %p, envp = %p )",
-          result_pid, exe_path, static_cast<void *>(&file_actions),
-          static_cast<void *>(&attr), reinterpret_cast<const void *>(argv),
-          reinterpret_cast<const void *>(envp));
+    if (error.Fail()) {
+      LLDB_LOG(log, "error: {0}, ::posix_spawnp(pid => {1}, path = '{2}', "
+                    "file_actions = {3}, "
+                    "attr = {4}, argv = {5}, envp = {6} )",
+               error, result_pid, exe_path, &file_actions, &attr, argv, envp);
       if (log) {
         for (int ii = 0; argv[ii]; ++ii)
-          log->Printf("argv[%i] = '%s'", ii, argv[ii]);
+          LLDB_LOG(log, "argv[{0}] = '{1}'", ii, argv[ii]);
       }
     }
 
@@ -856,16 +857,13 @@ Error Host::LaunchProcessPosixSpawn(cons
         ::posix_spawnp(&result_pid, exe_path, NULL, &attr, argv, envp),
         eErrorTypePOSIX);
 
-    if (error.Fail() || log) {
-      error.PutToLog(log, "::posix_spawnp ( pid => %i, path = '%s', "
-                          "file_actions = NULL, attr = %p, argv = %p, envp = "
-                          "%p )",
-                     result_pid, exe_path, static_cast<void *>(&attr),
-                     reinterpret_cast<const void *>(argv),
-                     reinterpret_cast<const void *>(envp));
+    if (error.Fail()) {
+      LLDB_LOG(log, "error: {0}, ::posix_spawnp ( pid => {1}, path = '{2}', "
+                    "file_actions = NULL, attr = {3}, argv = {4}, envp = {5} )",
+               error, result_pid, exe_path, &attr, argv, envp);
       if (log) {
         for (int ii = 0; argv[ii]; ++ii)
-          log->Printf("argv[%i] = '%s'", ii, argv[ii]);
+          LLDB_LOG("argv[{0}] = '{1}'", ii, argv[ii]);
       }
     }
   }
@@ -908,10 +906,10 @@ bool Host::AddPosixSpawnFileAction(void
       error.SetError(
           ::posix_spawn_file_actions_addclose(file_actions, info->GetFD()),
           eErrorTypePOSIX);
-      if (log && (error.Fail() || log))
-        error.PutToLog(log,
-                       "posix_spawn_file_actions_addclose (action=%p, fd=%i)",
-                       static_cast<void *>(file_actions), info->GetFD());
+      if (error.Fail())
+        LLDB_LOG(log, "error: {0}, posix_spawn_file_actions_addclose "
+                      "(action={1}, fd={2})",
+                 error, file_actions, info->GetFD());
     }
     break;
 
@@ -927,12 +925,10 @@ bool Host::AddPosixSpawnFileAction(void
           ::posix_spawn_file_actions_adddup2(file_actions, info->GetFD(),
                                              info->GetActionArgument()),
           eErrorTypePOSIX);
-      if (log && (error.Fail() || log))
-        error.PutToLog(
-            log,
-            "posix_spawn_file_actions_adddup2 (action=%p, fd=%i, dup_fd=%i)",
-            static_cast<void *>(file_actions), info->GetFD(),
-            info->GetActionArgument());
+      if (error.Fail())
+        LLDB_LOG(log, "error: {0}, posix_spawn_file_actions_adddup2 "
+                      "(action={1}, fd={2}, dup_fd={3})",
+                 error, file_actions, info->GetFD(), info->GetActionArgument());
     }
     break;
 
@@ -952,11 +948,11 @@ bool Host::AddPosixSpawnFileAction(void
                          file_actions, info->GetFD(),
                          info->GetPath().str().c_str(), oflag, mode),
                      eErrorTypePOSIX);
-      if (error.Fail() || log)
-        error.PutToLog(log, "posix_spawn_file_actions_addopen (action=%p, "
-                            "fd=%i, path='%s', oflag=%i, mode=%i)",
-                       static_cast<void *>(file_actions), info->GetFD(),
-                       info->GetPath().str().c_str(), oflag, mode);
+      if (error.Fail())
+        LLDB_LOG(
+            log, "error: {0}, posix_spawn_file_actions_addopen (action={1}, "
+                 "fd={2}, path='{3}', oflag={4}, mode={5})",
+            error, file_actions, info->GetFD(), info->GetPath(), oflag, mode);
     }
     break;
   }

Modified: lldb/trunk/source/Plugins/OperatingSystem/Go/OperatingSystemGo.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/OperatingSystem/Go/OperatingSystemGo.cpp?rev=294210&r1=294209&r2=294210&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/OperatingSystem/Go/OperatingSystemGo.cpp (original)
+++ lldb/trunk/source/Plugins/OperatingSystem/Go/OperatingSystemGo.cpp Mon Feb  6 12:31:44 2017
@@ -319,7 +319,7 @@ bool OperatingSystemGo::UpdateThreadList
   for (uint64_t i = 0; i < allglen; ++i) {
     goroutines.push_back(CreateGoroutineAtIndex(i, err));
     if (err.Fail()) {
-      err.PutToLog(log, "OperatingSystemGo::UpdateThreadList");
+      LLDB_LOG(log, "error: {0}", err);
       return new_thread_list.GetSize(false) > 0;
     }
   }

Modified: lldb/trunk/source/Utility/Error.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/Error.cpp?rev=294210&r1=294209&r2=294210&view=diff
==============================================================================
--- lldb/trunk/source/Utility/Error.cpp (original)
+++ lldb/trunk/source/Utility/Error.cpp Mon Feb  6 12:31:44 2017
@@ -130,72 +130,6 @@ ErrorType Error::GetType() const { retur
 bool Error::Fail() const { return m_code != 0; }
 
 //----------------------------------------------------------------------
-// Log the error given a string with format. If the this object
-// contains an error code, update the error string to contain the
-// "error: " followed by the formatted string, followed by the error
-// value and any string that describes the current error. This
-// allows more context to be given to an error string that remains
-// cached in this object. Logging always occurs even when the error
-// code contains a non-error value.
-//----------------------------------------------------------------------
-void Error::PutToLog(Log *log, const char *format, ...) {
-  char *arg_msg = nullptr;
-  va_list args;
-  va_start(args, format);
-  ::vasprintf(&arg_msg, format, args);
-  va_end(args);
-
-  if (arg_msg != nullptr) {
-    if (Fail()) {
-      const char *err_str = AsCString();
-      if (err_str == nullptr)
-        err_str = "???";
-
-      SetErrorStringWithFormat("error: %s err = %s (0x%8.8x)", arg_msg, err_str,
-                               m_code);
-      if (log != nullptr)
-        log->Error("%s", m_string.c_str());
-    } else {
-      if (log != nullptr)
-        log->Printf("%s err = 0x%8.8x", arg_msg, m_code);
-    }
-    ::free(arg_msg);
-  }
-}
-
-//----------------------------------------------------------------------
-// Log the error given a string with format. If the this object
-// contains an error code, update the error string to contain the
-// "error: " followed by the formatted string, followed by the error
-// value and any string that describes the current error. This
-// allows more context to be given to an error string that remains
-// cached in this object. Logging only occurs even when the error
-// code contains a error value.
-//----------------------------------------------------------------------
-void Error::LogIfError(Log *log, const char *format, ...) {
-  if (Fail()) {
-    char *arg_msg = nullptr;
-    va_list args;
-    va_start(args, format);
-    ::vasprintf(&arg_msg, format, args);
-    va_end(args);
-
-    if (arg_msg != nullptr) {
-      const char *err_str = AsCString();
-      if (err_str == nullptr)
-        err_str = "???";
-
-      SetErrorStringWithFormat("%s err = %s (0x%8.8x)", arg_msg, err_str,
-                               m_code);
-      if (log != nullptr)
-        log->Error("%s", m_string.c_str());
-
-      ::free(arg_msg);
-    }
-  }
-}
-
-//----------------------------------------------------------------------
 // Set accesssor for the error value to "err" and the type to
 // "eErrorTypeMachKernel"
 //----------------------------------------------------------------------
@@ -334,3 +268,10 @@ bool Error::Success() const { return m_c
 bool Error::WasInterrupted() const {
   return (m_type == eErrorTypePOSIX && m_code == EINTR);
 }
+
+void llvm::format_provider<lldb_private::Error>::format(
+    const lldb_private::Error &error, llvm::raw_ostream &OS,
+    llvm::StringRef Options) {
+  llvm::format_provider<llvm::StringRef>::format(error.AsCString(), OS,
+                                                 Options);
+}




More information about the lldb-commits mailing list