labath updated this revision to Diff 80744.
labath added a comment.

Update the example to the formatv-based API


https://reviews.llvm.org/D27459

Files:
  include/lldb/Core/Error.h
  include/lldb/Core/Log.h
  include/lldb/Host/FileSpec.h
  source/Plugins/Process/Linux/NativeProcessLinux.cpp

Index: source/Plugins/Process/Linux/NativeProcessLinux.cpp
===================================================================
--- source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -41,6 +41,7 @@
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/PseudoTerminal.h"
 #include "lldb/Utility/StringExtractor.h"
+#include "llvm/Support/FormatVariadic.h"
 
 #include "NativeThreadLinux.h"
 #include "Plugins/Process/POSIX/ProcessPOSIXLog.h"
@@ -72,6 +73,11 @@
 using namespace lldb_private::process_linux;
 using namespace llvm;
 
+// Obviously, these would not live in this file.
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, Error &error) {
+  return OS << error.AsCString();
+}
+
 // Private bits we only need internally.
 
 static bool ProcessVmReadvSupported() {
@@ -93,16 +99,13 @@
     // value from our own process.
     ssize_t res = process_vm_readv(getpid(), &local, 1, &remote, 1, 0);
     is_supported = (res == sizeof(source) && source == dest);
-    if (log) {
-      if (is_supported)
-        log->Printf("%s: Detected kernel support for process_vm_readv syscall. "
-                    "Fast memory reads enabled.",
-                    __FUNCTION__);
-      else
-        log->Printf("%s: syscall process_vm_readv failed (error: %s). Fast "
-                    "memory reads disabled.",
-                    __FUNCTION__, strerror(errno));
-    }
+    if (is_supported)
+      LLDB_LOG(log, "Detected kernel support for process_vm_readv syscall. "
+                    "Fast memory reads enabled.");
+    else
+      LLDB_LOG(log, "syscall process_vm_readv failed (error: {0}). Fast memory "
+                    "reads disabled.",
+               strerror(errno));
   });
 
   return is_supported;
@@ -115,16 +118,14 @@
     return;
 
   if (const FileAction *action = info.GetFileActionForFD(STDIN_FILENO))
-    log->Printf("%s: setting STDIN to '%s'", __FUNCTION__,
-                action->GetFileSpec().GetCString());
+    LLDB_LOG(log, "setting STDIN to {0}", action->GetFileSpec());
   else
-    log->Printf("%s leaving STDIN as is", __FUNCTION__);
+    LLDB_LOG(log, "leaving STDIN as is");
 
   if (const FileAction *action = info.GetFileActionForFD(STDOUT_FILENO))
-    log->Printf("%s setting STDOUT to '%s'", __FUNCTION__,
-                action->GetFileSpec().GetCString());
+    LLDB_LOG(log, "setting STDOUT to '{0}'", action->GetFileSpec());
   else
-    log->Printf("%s leaving STDOUT as is", __FUNCTION__);
+    LLDB_LOG(log, "leaving STDOUT as is");
 
   if (const FileAction *action = info.GetFileActionForFD(STDERR_FILENO))
     log->Printf("%s setting STDERR to '%s'", __FUNCTION__,
@@ -135,8 +136,7 @@
   int i = 0;
   for (const char **args = info.GetArguments().GetConstArgumentVector(); *args;
        ++args, ++i)
-    log->Printf("%s arg %d: \"%s\"", __FUNCTION__, i,
-                *args ? *args : "nullptr");
+    LLDB_LOG(log, "arg {0}: \"{1}\"", i, *args);
 }
 
 void DisplayBytes(StreamString &s, void *bytes, uint32_t count) {
@@ -256,9 +256,7 @@
 
   if (error.Fail()) {
     native_process_sp.reset();
-    if (log)
-      log->Printf("NativeProcessLinux::%s failed to launch process: %s",
-                  __FUNCTION__, error.AsCString());
+    LLDB_LOG(log, "failed to launch process: ", error);
     return error;
   }
 
@@ -308,9 +306,7 @@
 void NativeProcessLinux::AttachToInferior(MainLoop &mainloop, lldb::pid_t pid,
                                           Error &error) {
   Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));
-  if (log)
-    log->Printf("NativeProcessLinux::%s (pid = %" PRIi64 ")", __FUNCTION__,
-                pid);
+  LLDB_LOG(log, "(pid = {0})", pid);
 
   m_sigchld_handle = mainloop.RegisterSignal(
       SIGCHLD, [this](MainLoopBase &) { SigchldHandler(); }, error);
@@ -322,10 +318,8 @@
     return;
 
   // Set the architecture to the exe architecture.
-  if (log)
-    log->Printf("NativeProcessLinux::%s (pid = %" PRIi64
-                ") detected architecture %s",
-                __FUNCTION__, pid, m_arch.GetArchitectureName());
+  LLDB_LOG(log, "(pid = {0}) detected architecture {1}", pid,
+           m_arch.GetArchitectureName());
 
   m_pid = pid;
   SetState(eStateAttaching);
@@ -357,9 +351,7 @@
   int status;
   if ((wpid = waitpid(pid, &status, 0)) < 0) {
     error.SetErrorToErrno();
-    if (log)
-      log->Printf("NativeProcessLinux::%s waitpid for inferior failed with %s",
-                  __FUNCTION__, error.AsCString());
+    LLDB_LOG(log, "waitpid for inferior failed with: ", error);
 
     // Mark the inferior as invalid.
     // FIXME this could really use a new state - eStateLaunchFailure.  For now,
@@ -371,16 +363,11 @@
   assert(WIFSTOPPED(status) && (wpid == static_cast<::pid_t>(pid)) &&
          "Could not sync with inferior process.");
 
-  if (log)
-    log->Printf("NativeProcessLinux::%s inferior started, now in stopped state",
-                __FUNCTION__);
+  LLDB_LOG(log, "inferior started, now in stopped state");
 
   error = SetDefaultPtraceOpts(pid);
   if (error.Fail()) {
-    if (log)
-      log->Printf("NativeProcessLinux::%s inferior failed to set default "
-                  "ptrace options: %s",
-                  __FUNCTION__, error.AsCString());
+    LLDB_LOG(log, "inferior failed to set default ptrace options: {0}", error);
 
     // Mark the inferior as invalid.
     // FIXME this could really use a new state - eStateLaunchFailure.  For now,
@@ -579,11 +566,8 @@
 
   // Handle when the thread exits.
   if (exited) {
-    if (log)
-      log->Printf(
-          "NativeProcessLinux::%s() got exit signal(%d) , tid = %" PRIu64
-          " (%s main thread)",
-          __FUNCTION__, signal, pid, is_main_thread ? "is" : "is not");
+    LLDB_LOG(log, "got exit signal({0}) , tid = {1} ({2} main thread)", signal,
+             pid, is_main_thread ? "is" : "is not");
 
     // This is a thread that exited.  Ensure we're not tracking it anymore.
     const bool thread_found = StopTrackingThread(pid);
@@ -597,15 +581,13 @@
       const bool already_notified = (GetState() == StateType::eStateExited) ||
                                     (GetState() == StateType::eStateCrashed);
       if (!already_notified) {
-        if (log)
-          log->Printf("NativeProcessLinux::%s() tid = %" PRIu64
-                      " handling main thread exit (%s), expected exit state "
-                      "already set but state was %s instead, setting exit "
-                      "state now",
-                      __FUNCTION__, pid,
-                      thread_found ? "stopped tracking thread metadata"
-                                   : "thread metadata not found",
-                      StateAsCString(GetState()));
+        LLDB_LOG(log, "tid = {0} handling main thread exit ({1}), expected "
+                      "exit state already set but state was {2} instead, "
+                      "setting exit state now",
+                 pid,
+                 thread_found ? "stopped tracking thread metadata"
+                              : "thread metadata not found",
+                 StateAsCString(GetState()));
         // The main thread exited.  We're done monitoring.  Report to delegate.
         SetExitStatus(convert_pid_status_to_exit_type(status),
                       convert_pid_status_to_return_code(status), nullptr, true);
@@ -646,24 +628,20 @@
     // received a new thread notification. This is indicated by GetSignalInfo()
     // returning
     // si_code == SI_USER and si_pid == 0
-    if (log)
-      log->Printf("NativeProcessLinux::%s received notification about an "
-                  "unknown tid %" PRIu64 ".",
-                  __FUNCTION__, pid);
+    LLDB_LOG(log, "received notification about an unknown tid {0}", pid);
 
     if (info_err.Fail()) {
-      if (log)
-        log->Printf("NativeProcessLinux::%s (tid %" PRIu64
-                    ") GetSignalInfo failed (%s). Ingoring this notification.",
-                    __FUNCTION__, pid, info_err.AsCString());
+      LLDB_LOG(
+          log,
+          "(tid {0}) GetSignalInfo failed ({1}). Ingoring this notification.",
+          pid, info_err);
       return;
     }
 
-    if (log && (info.si_code != SI_USER || info.si_pid != 0))
-      log->Printf("NativeProcessLinux::%s (tid %" PRIu64
-                  ") unexpected signal info (si_code: %d, si_pid: %d). "
-                  "Treating as a new thread notification anyway.",
-                  __FUNCTION__, pid, info.si_code, info.si_pid);
+    if (info.si_code != SI_USER || info.si_pid != 0)
+      LLDB_LOG(log, "(tid {0}) unexpected signal info (si_code: {1}, si_pid: "
+                    "{2}). Treating as a new thread notification anyway.",
+               pid, info.si_code, info.si_pid);
 
     auto thread_sp = AddThread(pid);
     // Resume the newly created thread.
@@ -764,44 +742,37 @@
   int status = -1;
   ::pid_t wait_pid;
   do {
-    if (log)
-      log->Printf("NativeProcessLinux::%s() received thread creation event for "
-                  "tid %" PRIu32
-                  ". tid not tracked yet, waiting for thread to appear...",
-                  __FUNCTION__, tid);
+    LLDB_LOG(log, "received thread creation event for tid {0}. tid not tracked "
+                  "yet, waiting for thread to appear...",
+             tid);
     wait_pid = waitpid(tid, &status, __WALL);
   } while (wait_pid == -1 && errno == EINTR);
   // Since we are waiting on a specific tid, this must be the creation event.
   // But let's do
   // some checks just in case.
   if (wait_pid != tid) {
-    if (log)
-      log->Printf(
-          "NativeProcessLinux::%s() waiting for tid %" PRIu32
-          " failed. Assuming the thread has disappeared in the meantime",
-          __FUNCTION__, tid);
+    LLDB_LOG(log, "waiting for tid {0} failed. Assuming the thread has "
+                  "disappeared in the meantime",
+             tid);
     // The only way I know of this could happen is if the whole process was
     // SIGKILLed in the mean time. In any case, we can't do anything about that
     // now.
     return;
   }
   if (WIFEXITED(status)) {
-    if (log)
-      log->Printf("NativeProcessLinux::%s() waiting for tid %" PRIu32
-                  " returned an 'exited' event. Not tracking the thread.",
-                  __FUNCTION__, tid);
+    LLDB_LOG(log, "waiting for tid {0} returned an 'exited' event. Not "
+                  "tracking the thread.",
+             tid);
     // Also a very improbable event.
     return;
   }
 
   siginfo_t info;
   Error error = GetSignalInfo(tid, &info);
   if (error.Fail()) {
-    if (log)
-      log->Printf(
-          "NativeProcessLinux::%s() GetSignalInfo for tid %" PRIu32
-          " failed. Assuming the thread has disappeared in the meantime.",
-          __FUNCTION__, tid);
+    LLDB_LOG(log, "GetSignalInfo for tid {0} failed. Assuming the thread has "
+                  "disappeared in the meantime.",
+             tid);
     return;
   }
 
@@ -813,15 +784,12 @@
     // thread is alive and we are receiving events from it, we shall pretend
     // that it was
     // created properly.
-    log->Printf("NativeProcessLinux::%s() GetSignalInfo for tid %" PRIu32
-                " received unexpected signal with code %d from pid %d.",
-                __FUNCTION__, tid, info.si_code, info.si_pid);
+    LLDB_LOG(log, "GetSignalInfo for tid {0} received unexpected signal with "
+                  "code {1} from pid {2}.",
+             tid, info.si_code, info.si_pid);
   }
 
-  if (log)
-    log->Printf("NativeProcessLinux::%s() pid = %" PRIu64
-                ": tracking new thread tid %" PRIu32,
-                __FUNCTION__, GetID(), tid);
+  LLDB_LOG(log, "pid = {0}: tracking new thread tid {1}", GetID(), tid);
 
   new_thread_sp = AddThread(tid);
   ResumeThread(*new_thread_sp, eStateRunning, LLDB_INVALID_SIGNAL_NUMBER);
Index: include/lldb/Host/FileSpec.h
===================================================================
--- include/lldb/Host/FileSpec.h
+++ include/lldb/Host/FileSpec.h
@@ -16,6 +16,7 @@
 #include <string>
 
 // Other libraries and framework includes
+#include "llvm/Support/raw_ostream.h"
 // Project includes
 #include "lldb/Core/ConstString.h"
 #include "lldb/Core/STLUtils.h"
@@ -671,6 +672,10 @@
   //------------------------------------------------------------------
   size_t ReadFileLines(STLStringArray &lines);
 
+  void format(llvm::raw_ostream &Stream, llvm::StringRef Options) const {
+    Stream << this->GetCString();
+  }
+
   //------------------------------------------------------------------
   /// Resolves user name and links in \a path, and overwrites the input
   /// argument with the resolved path.
Index: include/lldb/Core/Log.h
===================================================================
--- include/lldb/Core/Log.h
+++ include/lldb/Core/Log.h
@@ -18,6 +18,10 @@
 
 // C++ Includes
 // Other libraries and framework includes
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/FormatVariadic.h"
+
 // Project includes
 #include "lldb/Core/ConstString.h"
 #include "lldb/Core/Flags.h"
@@ -192,6 +196,31 @@
   DISALLOW_COPY_AND_ASSIGN(LogChannel);
 };
 
+class LogWrapper {
+public:
+  LogWrapper(Log &log) : m_log(log), m_stream(m_message) {}
+  ~LogWrapper() { m_log.Printf("%s", m_stream.str().c_str()); }
+
+  llvm::raw_ostream &stream() { return m_stream; }
+
+private:
+  Log &m_log;
+  std::string m_message;
+  llvm::raw_string_ostream m_stream;
+};
+
+#define LLDB_LOG(log, ...)                                                     \
+  do {                                                                         \
+    Log *log_private = (log);                                                  \
+    if (log_private)                                                           \
+      LogWrapper(*log_private).stream()                                        \
+          << llvm::formatv(                                                    \
+                 "{0,-60}: ",                                                   \
+                 (llvm::sys::path::filename(__FILE__) + "::" + __FUNCTION__)   \
+                     .str())                                                   \
+          << llvm::formatv(__VA_ARGS__);                                       \
+  } while (0)
+
 } // namespace lldb_private
 
 #endif // liblldb_Log_h_
Index: include/lldb/Core/Error.h
===================================================================
--- include/lldb/Core/Error.h
+++ include/lldb/Core/Error.h
@@ -12,6 +12,7 @@
 #if defined(__cplusplus)
 
 #include "llvm/Support/DataTypes.h"
+#include "llvm/Support/raw_ostream.h"
 
 #include <cstdarg>
 #include <cstdio>
@@ -282,6 +283,9 @@
   //------------------------------------------------------------------
   bool WasInterrupted() const;
 
+  void format(llvm::raw_ostream &Stream, llvm::StringRef Options) const {
+    Stream << AsCString();
+  }
 protected:
   //------------------------------------------------------------------
   /// Member variables
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to