[Lldb-commits] [PATCH] D70458: [NFC] Refactor and improve comments in CommandObjectTarget

[Adrian McCarthy via Phabricator via lldb-commits]

amccarth created this revision.
amccarth added a reviewer: labath.

Made small improvements while debugging through CommandObjectTarget::AddModuleSymbols.

1. Refactored error case for an early out, reducing the indentation of the rest of this long function.
2. Clarified some comments by correcting spelling and punctuation.
3. Reduced duplicate code at the end of the function.

Tested with `ninja check-lldb`


https://reviews.llvm.org/D70458

Files:
  lldb/source/Commands/CommandObjectTarget.cpp


Index: lldb/source/Commands/CommandObjectTarget.cpp
===================================================================
--- lldb/source/Commands/CommandObjectTarget.cpp
+++ lldb/source/Commands/CommandObjectTarget.cpp
@@ -4044,14 +4044,21 @@
   bool AddModuleSymbols(Target *target, ModuleSpec &module_spec, bool &flush,
                         CommandReturnObject &result) {
     const FileSpec &symbol_fspec = module_spec.GetSymbolFileSpec();
-    if (symbol_fspec) {
+    if (!symbol_fspec) {
+      result.AppendError(
+          "one or more executable image paths must be specified");
+      result.SetStatus(eReturnStatusFailed);
+      return false;
+    }
+
     char symfile_path[PATH_MAX];
     symbol_fspec.GetPath(symfile_path, sizeof(symfile_path));
 
     if (!module_spec.GetUUID().IsValid()) {
       if (!module_spec.GetFileSpec() && !module_spec.GetPlatformFileSpec())
-          module_spec.GetFileSpec().GetFilename() = symbol_fspec.GetFilename();
+        module_spec.GetFileSpec() = symbol_fspec;
     }
+
     // We now have a module that represents a symbol file that can be used
     // for a module that might exist in the current target, so we need to
     // find that module in the target
@@ -4112,18 +4119,16 @@
     while (num_matches == 0) {
       ConstString filename_no_extension(
           module_spec.GetFileSpec().GetFileNameStrippingExtension());
-        // Empty string returned, lets bail
+      // Empty string returned, let's bail
       if (!filename_no_extension)
         break;
 
-        // Check if there was no extension to strip and the basename is the
-        // same
+      // Check if there was no extension to strip and the basename is the same
       if (filename_no_extension == module_spec.GetFileSpec().GetFilename())
         break;
 
-        // Replace basename with one less extension
+      // Replace basename with one fewer extension
       module_spec.GetFileSpec().GetFilename() = filename_no_extension;
-
       target->GetImages().FindModules(module_spec, matching_module_list);
       num_matches = matching_module_list.GetSize();
     }
@@ -4186,27 +4191,18 @@
     }
 
     namespace fs = llvm::sys::fs;
-      if (module_spec.GetUUID().IsValid()) {
     StreamString ss_symfile_uuid;
+    if (module_spec.GetUUID().IsValid()) {
+      ss_symfile_uuid << " (";
       module_spec.GetUUID().Dump(&ss_symfile_uuid);
+      ss_symfile_uuid << ')';
+    }
     result.AppendErrorWithFormat(
-            "symbol file '%s' (%s) does not match any existing module%s\n",
+        "symbol file '%s'%s does not match any existing module%s\n",
         symfile_path, ss_symfile_uuid.GetData(),
         !fs::is_regular_file(symbol_fspec.GetPath())
             ? "\n       please specify the full path to the symbol file"
             : "");
-      } else {
-        result.AppendErrorWithFormat(
-            "symbol file '%s' does not match any existing module%s\n",
-            symfile_path,
-            !fs::is_regular_file(symbol_fspec.GetPath())
-                ? "\n       please specify the full path to the symbol file"
-                : "");
-      }
-    } else {
-      result.AppendError(
-          "one or more executable image paths must be specified");
-    }
     result.SetStatus(eReturnStatusFailed);
     return false;
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D70458.230132.patch
Type: text/x-patch
Size: 3317 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20191119/0942d191/attachment-0001.bin>