zturner created this revision.

The goal here is to eventually delete the `StringConvert` class, as the llvm 
APIs for doing string to number conversion have a cleaner syntax.

In order to reduce code churn, since I was in this code anyway, I changed the 
deeply nested indentation of the code blocks I was touching to use early outs.  
This led me to get annoyed by the fact that every time we want to return 
something, it has to be 3 or 4 lines of code to open a scope, set the correct 
status on the error, set the message, eventually return, and then add another 
line of code to close the scope.  We should be able to do all this in one line 
to further reduce boilerplate goo.  So I added some code to `Status` to do 
this.  Whereas before we would have to write this:

  if (!succeed) {
    result.AppendError("some error");
    result.SetStatus(eStatusFailed);
    return result.Succeeded();
  }

which is 5 lines of code, now we only have to write:

  if (!success)
    return result.AppendError(eStatusFailed, "some error");

which is only 2 lines.

There are more occurrences of `StringConvert` to change, but I wanted to get 
the feedback out of the way first before I go and do the rest.


https://reviews.llvm.org/D33167

Files:
  lldb/include/lldb/Interpreter/CommandReturnObject.h
  lldb/include/lldb/Utility/Status.h
  lldb/source/Commands/CommandObjectPlatform.cpp
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Interpreter/CommandReturnObject.cpp

Index: lldb/source/Interpreter/CommandReturnObject.cpp
===================================================================
--- lldb/source/Interpreter/CommandReturnObject.cpp
+++ lldb/source/Interpreter/CommandReturnObject.cpp
@@ -136,7 +136,10 @@
   GetErrorStream() << in_string;
 }
 
-void CommandReturnObject::SetStatus(ReturnStatus status) { m_status = status; }
+bool CommandReturnObject::SetStatus(ReturnStatus status) {
+  m_status = status;
+  return Succeeded();
+}
 
 ReturnStatus CommandReturnObject::GetStatus() { return m_status; }
 
Index: lldb/source/Commands/CommandObjectTarget.cpp
===================================================================
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -20,7 +20,6 @@
 #include "lldb/Core/ValueObjectVariable.h"
 #include "lldb/DataFormatters/ValueObjectPrinter.h"
 #include "lldb/Host/OptionParser.h"
-#include "lldb/Host/StringConvert.h"
 #include "lldb/Host/Symbols.h"
 #include "lldb/Interpreter/Args.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
@@ -51,6 +50,7 @@
 #include "lldb/Target/Thread.h"
 #include "lldb/Target/ThreadSpec.h"
 
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/FileSystem.h"
 
 // C Includes
@@ -492,50 +492,42 @@
 
 protected:
   bool DoExecute(Args &args, CommandReturnObject &result) override {
-    if (args.GetArgumentCount() == 1) {
-      bool success = false;
-      const char *target_idx_arg = args.GetArgumentAtIndex(0);
-      uint32_t target_idx =
-          StringConvert::ToUInt32(target_idx_arg, UINT32_MAX, 0, &success);
-      if (success) {
-        TargetList &target_list = m_interpreter.GetDebugger().GetTargetList();
-        const uint32_t num_targets = target_list.GetNumTargets();
-        if (target_idx < num_targets) {
-          TargetSP target_sp(target_list.GetTargetAtIndex(target_idx));
-          if (target_sp) {
-            Stream &strm = result.GetOutputStream();
-            target_list.SetSelectedTarget(target_sp.get());
-            bool show_stopped_process_status = false;
-            DumpTargetList(target_list, show_stopped_process_status, strm);
-            result.SetStatus(eReturnStatusSuccessFinishResult);
-          } else {
-            result.AppendErrorWithFormat("target #%u is NULL in target list\n",
-                                         target_idx);
-            result.SetStatus(eReturnStatusFailed);
-          }
-        } else {
-          if (num_targets > 0) {
-            result.AppendErrorWithFormat(
-                "index %u is out of range, valid target indexes are 0 - %u\n",
-                target_idx, num_targets - 1);
-          } else {
-            result.AppendErrorWithFormat(
-                "index %u is out of range since there are no active targets\n",
-                target_idx);
-          }
-          result.SetStatus(eReturnStatusFailed);
-        }
-      } else {
-        result.AppendErrorWithFormat("invalid index string value '%s'\n",
-                                     target_idx_arg);
-        result.SetStatus(eReturnStatusFailed);
-      }
-    } else {
-      result.AppendError(
+    if (args.GetArgumentCount() != 1)
+      return result.AppendError(
+          eReturnStatusFailed,
           "'target select' takes a single argument: a target index\n");
-      result.SetStatus(eReturnStatusFailed);
+
+    uint32_t target_idx;
+    if (!llvm::strton(args[0].ref, target_idx))
+      return result.AppendError(eReturnStatusFailed,
+                                "invalid index string value '{0}'\n",
+                                args[0].ref);
+
+    bool success = false;
+    TargetList &target_list = m_interpreter.GetDebugger().GetTargetList();
+    const uint32_t num_targets = target_list.GetNumTargets();
+    if (target_idx >= num_targets) {
+      constexpr auto S = eReturnStatusFailed;
+      if (num_targets > 0)
+        return result.AppendError(
+            S, "index {0} is out of range, valid target indices are 0 - {1}\n",
+            target_idx, num_targets - 1);
+      return result.AppendError(
+          S, "index {0} is out of range since there are no active targets\n",
+          target_idx);
     }
-    return result.Succeeded();
+
+    TargetSP target_sp(target_list.GetTargetAtIndex(target_idx));
+    if (!target_sp)
+      return result.AppendError(eReturnStatusFailed,
+                                "target #{0} is NULL in target list\n",
+                                target_idx);
+
+    Stream &strm = result.GetOutputStream();
+    target_list.SetSelectedTarget(target_sp.get());
+    bool show_stopped_process_status = false;
+    DumpTargetList(target_list, show_stopped_process_status, strm);
+    return result.SetStatus(eReturnStatusSuccessFinishResult);
   }
 };
 
@@ -1147,54 +1139,42 @@
 protected:
   bool DoExecute(Args &command, CommandReturnObject &result) override {
     Target *target = m_interpreter.GetDebugger().GetSelectedTarget().get();
-    if (target) {
-      size_t argc = command.GetArgumentCount();
-      // check for at least 3 arguments and an odd number of parameters
-      if (argc >= 3 && argc & 1) {
-        bool success = false;
-
-        uint32_t insert_idx = StringConvert::ToUInt32(
-            command.GetArgumentAtIndex(0), UINT32_MAX, 0, &success);
-
-        if (!success) {
-          result.AppendErrorWithFormat(
-              "<index> parameter is not an integer: '%s'.\n",
-              command.GetArgumentAtIndex(0));
-          result.SetStatus(eReturnStatusFailed);
-          return result.Succeeded();
-        }
-
-        // shift off the index
-        command.Shift();
-        argc = command.GetArgumentCount();
-
-        for (uint32_t i = 0; i < argc; i += 2, ++insert_idx) {
-          const char *from = command.GetArgumentAtIndex(i);
-          const char *to = command.GetArgumentAtIndex(i + 1);
-
-          if (from[0] && to[0]) {
-            bool last_pair = ((argc - i) == 2);
-            target->GetImageSearchPathList().Insert(
-                ConstString(from), ConstString(to), insert_idx, last_pair);
-            result.SetStatus(eReturnStatusSuccessFinishNoResult);
-          } else {
-            if (from[0])
-              result.AppendError("<path-prefix> can't be empty\n");
-            else
-              result.AppendError("<new-path-prefix> can't be empty\n");
-            result.SetStatus(eReturnStatusFailed);
-            return false;
-          }
-        }
-      } else {
-        result.AppendError("insert requires at least three arguments\n");
-        result.SetStatus(eReturnStatusFailed);
-        return result.Succeeded();
-      }
-
-    } else {
-      result.AppendError("invalid target\n");
-      result.SetStatus(eReturnStatusFailed);
+    if (!target)
+      return result.AppendError(eReturnStatusFailed, "invalid target\n");
+
+    size_t argc = command.GetArgumentCount();
+    // check for at least 3 arguments and an odd number of parameters
+    if (argc < 3 || argc % 2 == 0)
+      return result.AppendError(eReturnStatusFailed,
+                                "insert requires at least three arguments\n");
+
+    bool success = false;
+
+    uint32_t insert_idx;
+    if (!llvm::strton(command[0].ref, insert_idx))
+      return result.AppendError(eReturnStatusFailed,
+                                "<index> parameter is not an integer: '{0}'.\n",
+                                command[0].ref);
+
+    // shift off the index
+    command.Shift();
+    argc = command.GetArgumentCount();
+
+    for (uint32_t i = 0; i < argc; i += 2, ++insert_idx) {
+      const char *from = command.GetArgumentAtIndex(i);
+      const char *to = command.GetArgumentAtIndex(i + 1);
+
+      if (!to[0])
+        return result.AppendError(eReturnStatusFailed,
+                                  "<new-path-prefix> can't be empty\n");
+      if (!from[0])
+        return result.AppendError(eReturnStatusFailed,
+                                  "<path-prefix> can't be empty\n");
+
+      bool last_pair = ((argc - i) == 2);
+      target->GetImageSearchPathList().Insert(
+          ConstString(from), ConstString(to), insert_idx, last_pair);
+      result.SetStatus(eReturnStatusSuccessFinishNoResult);
     }
     return result.Succeeded();
   }
@@ -2690,56 +2670,53 @@
                   for (size_t i = 0; i < argc; i += 2) {
                     const char *sect_name = args.GetArgumentAtIndex(i);
                     const char *load_addr_cstr = args.GetArgumentAtIndex(i + 1);
-                    if (sect_name && load_addr_cstr) {
-                      ConstString const_sect_name(sect_name);
-                      bool success = false;
-                      addr_t load_addr = StringConvert::ToUInt64(
-                          load_addr_cstr, LLDB_INVALID_ADDRESS, 0, &success);
-                      if (success) {
-                        SectionSP section_sp(
-                            section_list->FindSectionByName(const_sect_name));
-                        if (section_sp) {
-                          if (section_sp->IsThreadSpecific()) {
-                            result.AppendErrorWithFormat(
-                                "thread specific sections are not yet "
-                                "supported (section '%s')\n",
-                                sect_name);
-                            result.SetStatus(eReturnStatusFailed);
-                            break;
-                          } else {
-                            if (target->GetSectionLoadList()
-                                    .SetSectionLoadAddress(section_sp,
-                                                           load_addr))
-                              changed = true;
-                            result.AppendMessageWithFormat(
-                                "section '%s' loaded at 0x%" PRIx64 "\n",
-                                sect_name, load_addr);
-                          }
-                        } else {
-                          result.AppendErrorWithFormat("no section found that "
-                                                       "matches the section "
-                                                       "name '%s'\n",
-                                                       sect_name);
-                          result.SetStatus(eReturnStatusFailed);
-                          break;
-                        }
-                      } else {
-                        result.AppendErrorWithFormat(
-                            "invalid load address string '%s'\n",
-                            load_addr_cstr);
-                        result.SetStatus(eReturnStatusFailed);
-                        break;
-                      }
-                    } else {
-                      if (sect_name)
-                        result.AppendError("section names must be followed by "
-                                           "a load address.\n");
-                      else
-                        result.AppendError("one or more section name + load "
-                                           "address pair must be specified.\n");
+                    if (!sect_name) {
+                      result.AppendError(eReturnStatusFailed,
+                                         "one or more section name + load "
+                                         "address pair must be specified.\n");
+                      break;
+                    }
+                    if (!load_addr_cstr) {
+                      result.AppendError("section names must be followed by "
+                                         "a load address.\n");
+                      break;
+                    }
+
+                    ConstString const_sect_name(sect_name);
+                    bool success = false;
+                    addr_t load_addr;
+                    if (!llvm::strton(load_addr_cstr, load_addr)) {
+                      result.AppendError(eReturnStatusFailed,
+                                         "invalid load address string '{0}'\n",
+                                         load_addr_cstr);
+                      break;
+                    }
+
+                    SectionSP section_sp(
+                        section_list->FindSectionByName(const_sect_name));
+                    if (!section_sp) {
+                      result.AppendError(eReturnStatusFailed,
+                                         "no section found that "
+                                         "matches the section name '{0}'\n",
+                                         sect_name);
+                      break;
+                    }
+
+                    if (section_sp->IsThreadSpecific()) {
+                      result.AppendErrorWithFormat(
+                          "thread specific sections are not yet "
+                          "supported (section '%s')\n",
+                          sect_name);
                       result.SetStatus(eReturnStatusFailed);
                       break;
                     }
+
+                    if (target->GetSectionLoadList().SetSectionLoadAddress(
+                            section_sp, load_addr))
+                      changed = true;
+                    result.AppendMessageWithFormat(
+                        "section '%s' loaded at 0x%" PRIx64 "\n", sect_name,
+                        load_addr);
                   }
                 }
 
@@ -4692,43 +4669,30 @@
 protected:
   bool DoExecute(Args &command, CommandReturnObject &result) override {
     Target *target = GetSelectedOrDummyTarget();
-    if (target) {
-      // FIXME: see if we can use the breakpoint id style parser?
-      size_t num_args = command.GetArgumentCount();
-      if (num_args == 0) {
-        if (!m_interpreter.Confirm("Delete all stop hooks?", true)) {
-          result.SetStatus(eReturnStatusFailed);
-          return false;
-        } else {
-          target->RemoveAllStopHooks();
-        }
-      } else {
-        bool success;
-        for (size_t i = 0; i < num_args; i++) {
-          lldb::user_id_t user_id = StringConvert::ToUInt32(
-              command.GetArgumentAtIndex(i), 0, 0, &success);
-          if (!success) {
-            result.AppendErrorWithFormat("invalid stop hook id: \"%s\".\n",
-                                         command.GetArgumentAtIndex(i));
-            result.SetStatus(eReturnStatusFailed);
-            return false;
-          }
-          success = target->RemoveStopHookByID(user_id);
-          if (!success) {
-            result.AppendErrorWithFormat("unknown stop hook id: \"%s\".\n",
-                                         command.GetArgumentAtIndex(i));
-            result.SetStatus(eReturnStatusFailed);
-            return false;
-          }
-        }
-      }
-      result.SetStatus(eReturnStatusSuccessFinishNoResult);
+    if (!target)
+      return result.AppendError(eReturnStatusFailed, "invalid target\n");
+
+    // FIXME: see if we can use the breakpoint id style parser?
+    size_t num_args = command.GetArgumentCount();
+    if (num_args == 0) {
+      if (!m_interpreter.Confirm("Delete all stop hooks?", true))
+        return result.SetStatus(eReturnStatusFailed);
+      target->RemoveAllStopHooks();
     } else {
-      result.AppendError("invalid target\n");
-      result.SetStatus(eReturnStatusFailed);
+      bool success;
+      for (const auto &arg : command) {
+        lldb::user_id_t user_id;
+        if (!llvm::strton(arg.ref, user_id))
+          return result.AppendError(
+              eReturnStatusFailed, "invalid stop hook id: \"{0}\".\n", arg.ref);
+
+        success = target->RemoveStopHookByID(user_id);
+        if (!success)
+          return result.AppendError(
+              eReturnStatusFailed, "unknown stop hook id: \"{0}\".\n", arg.ref);
+      }
     }
-
-    return result.Succeeded();
+    return result.SetStatus(eReturnStatusSuccessFinishNoResult);
   }
 };
 
@@ -4751,38 +4715,28 @@
 protected:
   bool DoExecute(Args &command, CommandReturnObject &result) override {
     Target *target = GetSelectedOrDummyTarget();
-    if (target) {
-      // FIXME: see if we can use the breakpoint id style parser?
-      size_t num_args = command.GetArgumentCount();
-      bool success;
+    if (!target)
+      return result.AppendError(eReturnStatusFailed, "invalid target\n");
 
-      if (num_args == 0) {
-        target->SetAllStopHooksActiveState(m_enable);
-      } else {
-        for (size_t i = 0; i < num_args; i++) {
-          lldb::user_id_t user_id = StringConvert::ToUInt32(
-              command.GetArgumentAtIndex(i), 0, 0, &success);
-          if (!success) {
-            result.AppendErrorWithFormat("invalid stop hook id: \"%s\".\n",
-                                         command.GetArgumentAtIndex(i));
-            result.SetStatus(eReturnStatusFailed);
-            return false;
-          }
-          success = target->SetStopHookActiveStateByID(user_id, m_enable);
-          if (!success) {
-            result.AppendErrorWithFormat("unknown stop hook id: \"%s\".\n",
-                                         command.GetArgumentAtIndex(i));
-            result.SetStatus(eReturnStatusFailed);
-            return false;
-          }
-        }
-      }
-      result.SetStatus(eReturnStatusSuccessFinishNoResult);
-    } else {
-      result.AppendError("invalid target\n");
-      result.SetStatus(eReturnStatusFailed);
+    // FIXME: see if we can use the breakpoint id style parser?
+    size_t num_args = command.GetArgumentCount();
+
+    if (num_args == 0) {
+      target->SetAllStopHooksActiveState(m_enable);
+      return result.SetStatus(eReturnStatusSuccessFinishNoResult);
     }
-    return result.Succeeded();
+
+    for (const auto &arg : command) {
+      lldb::user_id_t user_id;
+      if (!llvm::strton(arg.ref, user_id))
+        result.AppendError(eReturnStatusFailed,
+                           "invalid stop hook id: \"{0}\".\n", arg.ref);
+
+      if (!target->SetStopHookActiveStateByID(user_id, m_enable))
+        return result.AppendError(eReturnStatusFailed,
+                                  "unknown stop hook id: \"{0}\".\n", arg.ref);
+    }
+    return result.SetStatus(eReturnStatusSuccessFinishNoResult);
   }
 
 private:
Index: lldb/source/Commands/CommandObjectProcess.cpp
===================================================================
--- lldb/source/Commands/CommandObjectProcess.cpp
+++ lldb/source/Commands/CommandObjectProcess.cpp
@@ -20,7 +20,6 @@
 #include "lldb/Core/State.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Host/OptionParser.h"
-#include "lldb/Host/StringConvert.h"
 #include "lldb/Interpreter/Args.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
@@ -32,6 +31,8 @@
 #include "lldb/Target/Thread.h"
 #include "lldb/Target/UnixSignals.h"
 
+#include "llvm/ADT/StringExtras.h"
+
 using namespace lldb;
 using namespace lldb_private;
 
@@ -1154,37 +1155,32 @@
   bool DoExecute(Args &command, CommandReturnObject &result) override {
     Process *process = m_exe_ctx.GetProcessPtr();
 
-    if (command.GetArgumentCount() == 1) {
-      int signo = LLDB_INVALID_SIGNAL_NUMBER;
+    if (command.GetArgumentCount() != 1) {
+      return result.AppendError(
+          eReturnStatusFailed,
+          "'{0}' takes exactly one signal number argument:\nUsage: {1}\n",
+          m_cmd_name, m_cmd_syntax);
+    }
 
-      const char *signal_name = command.GetArgumentAtIndex(0);
-      if (::isxdigit(signal_name[0]))
-        signo =
-            StringConvert::ToSInt32(signal_name, LLDB_INVALID_SIGNAL_NUMBER, 0);
-      else
-        signo = process->GetUnixSignals()->GetSignalNumberFromName(signal_name);
+    int signo = LLDB_INVALID_SIGNAL_NUMBER;
 
-      if (signo == LLDB_INVALID_SIGNAL_NUMBER) {
-        result.AppendErrorWithFormat("Invalid signal argument '%s'.\n",
-                                     command.GetArgumentAtIndex(0));
-        result.SetStatus(eReturnStatusFailed);
-      } else {
-        Status error(process->Signal(signo));
-        if (error.Success()) {
-          result.SetStatus(eReturnStatusSuccessFinishResult);
-        } else {
-          result.AppendErrorWithFormat("Failed to send signal %i: %s\n", signo,
-                                       error.AsCString());
-          result.SetStatus(eReturnStatusFailed);
-        }
-      }
-    } else {
-      result.AppendErrorWithFormat(
-          "'%s' takes exactly one signal number argument:\nUsage: %s\n",
-          m_cmd_name.c_str(), m_cmd_syntax.c_str());
-      result.SetStatus(eReturnStatusFailed);
-    }
-    return result.Succeeded();
+    const char *signal_name = command.GetArgumentAtIndex(0);
+    if (::isxdigit(signal_name[0]))
+      llvm::strton(signal_name, signo, 16);
+    else
+      signo = process->GetUnixSignals()->GetSignalNumberFromName(signal_name);
+
+    if (signo == LLDB_INVALID_SIGNAL_NUMBER)
+      return result.AppendError(eReturnStatusFailed,
+                                "Invalid signal argument '{0}'.\n",
+                                command[0].ref);
+
+    if (auto ST = process->Signal(signo))
+      return result.AppendError(eReturnStatusFailed,
+                                "Failed to send signal {0}: {1}\n", signo,
+                                ST.AsCString());
+
+    return result.SetStatus(eReturnStatusSuccessFinishResult);
   }
 };
 
@@ -1442,22 +1438,16 @@
   Options *GetOptions() override { return &m_options; }
 
   bool VerifyCommandOptionValue(const std::string &option, int &real_value) {
-    bool okay = true;
     bool success = false;
     bool tmp_value = Args::StringToBoolean(option, false, &success);
 
-    if (success && tmp_value)
-      real_value = 1;
-    else if (success && !tmp_value)
-      real_value = 0;
-    else {
-      // If the value isn't 'true' or 'false', it had better be 0 or 1.
-      real_value = StringConvert::ToUInt32(option.c_str(), 3);
-      if (real_value != 0 && real_value != 1)
-        okay = false;
+    if (success) {
+      real_value = static_cast<int>(tmp_value);
+      return true;
     }
 
-    return okay;
+    // If the value isn't 'true' or 'false', it had better be 0 or 1.
+    return llvm::strton(option, real_value, 2);
   }
 
   void PrintSignalHeader(Stream &str) {
Index: lldb/source/Commands/CommandObjectPlatform.cpp
===================================================================
--- lldb/source/Commands/CommandObjectPlatform.cpp
+++ lldb/source/Commands/CommandObjectPlatform.cpp
@@ -17,7 +17,6 @@
 #include "lldb/Core/Module.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Host/OptionParser.h"
-#include "lldb/Host/StringConvert.h"
 #include "lldb/Interpreter/Args.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandOptionValidators.h"
@@ -30,6 +29,7 @@
 #include "lldb/Utility/DataExtractor.h"
 
 #include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Threading.h"
 
 using namespace lldb;
@@ -595,25 +595,23 @@
   bool DoExecute(Args &args, CommandReturnObject &result) override {
     PlatformSP platform_sp(
         m_interpreter.GetDebugger().GetPlatformList().GetSelectedPlatform());
-    if (platform_sp) {
-      std::string cmd_line;
-      args.GetCommandString(cmd_line);
-      const lldb::user_id_t fd =
-          StringConvert::ToUInt64(cmd_line.c_str(), UINT64_MAX);
-      Status error;
-      bool success = platform_sp->CloseFile(fd, error);
-      if (success) {
-        result.AppendMessageWithFormat("file %" PRIu64 " closed.\n", fd);
-        result.SetStatus(eReturnStatusSuccessFinishResult);
-      } else {
-        result.AppendError(error.AsCString());
-        result.SetStatus(eReturnStatusFailed);
-      }
-    } else {
-      result.AppendError("no platform currently selected\n");
-      result.SetStatus(eReturnStatusFailed);
-    }
-    return result.Succeeded();
+    if (!platform_sp)
+      return result.AppendError(eReturnStatusFailed,
+                                "no platform currently selected.\n");
+
+    std::string cmd_line;
+    args.GetCommandString(cmd_line);
+    lldb::user_id_t fd;
+    if (!llvm::strton(cmd_line, fd))
+      return result.AppendError(eReturnStatusFailed, "Invalid fd {0}",
+                                cmd_line);
+
+    Status error;
+    if (!platform_sp->CloseFile(fd, error))
+      return result.AppendError(eReturnStatusFailed, error.AsCString());
+
+    return result.AppendMessage(eReturnStatusSuccessFinishResult,
+                                "file {0} closed.\n", fd);
   }
 };
 
@@ -641,23 +639,23 @@
   bool DoExecute(Args &args, CommandReturnObject &result) override {
     PlatformSP platform_sp(
         m_interpreter.GetDebugger().GetPlatformList().GetSelectedPlatform());
-    if (platform_sp) {
-      std::string cmd_line;
-      args.GetCommandString(cmd_line);
-      const lldb::user_id_t fd =
-          StringConvert::ToUInt64(cmd_line.c_str(), UINT64_MAX);
-      std::string buffer(m_options.m_count, 0);
-      Status error;
-      uint32_t retcode = platform_sp->ReadFile(
-          fd, m_options.m_offset, &buffer[0], m_options.m_count, error);
-      result.AppendMessageWithFormat("Return = %d\n", retcode);
-      result.AppendMessageWithFormat("Data = \"%s\"\n", buffer.c_str());
-      result.SetStatus(eReturnStatusSuccessFinishResult);
-    } else {
-      result.AppendError("no platform currently selected\n");
-      result.SetStatus(eReturnStatusFailed);
-    }
-    return result.Succeeded();
+    if (!platform_sp)
+      return result.AppendError(eReturnStatusFailed,
+                                "no platform currently selected\n");
+
+    std::string cmd_line;
+    args.GetCommandString(cmd_line);
+    lldb::user_id_t fd;
+    if (!llvm::strton(cmd_line, fd))
+      return result.AppendError(eReturnStatusFailed,
+                                "Invalid file descriptor {0}\n", fd);
+    std::string buffer(m_options.m_count, 0);
+    Status error;
+    uint32_t retcode = platform_sp->ReadFile(fd, m_options.m_offset, &buffer[0],
+                                             m_options.m_count, error);
+    return result.AppendMessage(eReturnStatusSuccessFinishResult,
+                                "Return = {0}\nData = \"{1}\"\n", retcode,
+                                buffer);
   }
 
   Options *GetOptions() override { return &m_options; }
@@ -736,22 +734,23 @@
   bool DoExecute(Args &args, CommandReturnObject &result) override {
     PlatformSP platform_sp(
         m_interpreter.GetDebugger().GetPlatformList().GetSelectedPlatform());
-    if (platform_sp) {
-      std::string cmd_line;
-      args.GetCommandString(cmd_line);
-      Status error;
-      const lldb::user_id_t fd =
-          StringConvert::ToUInt64(cmd_line.c_str(), UINT64_MAX);
-      uint32_t retcode =
-          platform_sp->WriteFile(fd, m_options.m_offset, &m_options.m_data[0],
-                                 m_options.m_data.size(), error);
-      result.AppendMessageWithFormat("Return = %d\n", retcode);
-      result.SetStatus(eReturnStatusSuccessFinishResult);
-    } else {
-      result.AppendError("no platform currently selected\n");
-      result.SetStatus(eReturnStatusFailed);
-    }
-    return result.Succeeded();
+    if (!platform_sp)
+      return result.AppendError(eReturnStatusFailed,
+                                "no platform currently selected\n");
+
+    std::string cmd_line;
+    args.GetCommandString(cmd_line);
+    Status error;
+    lldb::user_id_t fd;
+    if (!llvm::strton(cmd_line, fd))
+      return result.AppendError(eReturnStatusFailed,
+                                "Invalid file descriptor {0}\n", cmd_line);
+
+    uint32_t retcode =
+        platform_sp->WriteFile(fd, m_options.m_offset, &m_options.m_data[0],
+                               m_options.m_data.size(), error);
+    return result.AppendMessage(eReturnStatusSuccessFinishResult,
+                                "Return = {0}\n", retcode);
   }
 
   Options *GetOptions() override { return &m_options; }
Index: lldb/include/lldb/Utility/Status.h
===================================================================
--- lldb/include/lldb/Utility/Status.h
+++ lldb/include/lldb/Utility/Status.h
@@ -104,6 +104,8 @@
   //------------------------------------------------------------------
   const Status &operator=(uint32_t err);
 
+  operator bool() const { return Fail(); }
+
   ~Status();
 
   //------------------------------------------------------------------
Index: lldb/include/lldb/Interpreter/CommandReturnObject.h
===================================================================
--- lldb/include/lldb/Interpreter/CommandReturnObject.h
+++ lldb/include/lldb/Interpreter/CommandReturnObject.h
@@ -129,13 +129,29 @@
     AppendError(llvm::formatv(format, std::forward<Args>(args)...).str());
   }
 
+  template <typename... Args>
+  bool AppendError(lldb::ReturnStatus Status, const char *Format,
+                   Args &&... args) {
+    SetStatus(Status);
+    AppendErrorWithFormatv(Format, std::forward<Args>(args)...);
+    return Succeeded();
+  }
+
+  template <typename... Args>
+  bool AppendMessage(lldb::ReturnStatus Status, const char *Format,
+                     Args &&... args) {
+    SetStatus(Status);
+    AppendMessageWithFormatv(Format, std::forward<Args>(args)...);
+    return Succeeded();
+  }
+
   void SetError(const Status &error, const char *fallback_error_cstr = nullptr);
 
   void SetError(llvm::StringRef error_cstr);
 
   lldb::ReturnStatus GetStatus();
 
-  void SetStatus(lldb::ReturnStatus status);
+  bool SetStatus(lldb::ReturnStatus status);
 
   bool Succeeded();
 
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to