[Lldb-commits] [PATCH] D117547: [lldb] Simplify RemoteAwarePlatform::ResolveExecutable

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: jarin, PatriosTheGreat, EugeneBi, yuri.
labath requested review of this revision.
Herald added a project: LLDB.

The function had three branches:

- host
- remote-but-not-connected
- remote-and-connected

All three branches overlap to varying degrees. This patch reduces them
to two by merging the two remote branches. The main difference between
the two is (or should be -- the code is fairly convoluted, so it's hard
to be certain) that in the connected case we can try to download the
module from the remote system.

However, there's no need to fork the code for that. We can just proceed
with the normal (connected) search sequence and let the sub-operations
that require a connected host fail.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117547

Files:
  lldb/source/Target/RemoteAwarePlatform.cpp


Index: lldb/source/Target/RemoteAwarePlatform.cpp
===================================================================
--- lldb/source/Target/RemoteAwarePlatform.cpp
+++ lldb/source/Target/RemoteAwarePlatform.cpp
@@ -32,62 +32,43 @@
 Status RemoteAwarePlatform::ResolveExecutable(
     const ModuleSpec &module_spec, ModuleSP &exe_module_sp,
     const FileSpecList *module_search_paths_ptr) {
-  Status error;
-  // Nothing special to do here, just use the actual file and architecture
 
-  char exe_path[PATH_MAX];
   ModuleSpec resolved_module_spec(module_spec);
 
-  if (IsHost()) {
-    // If we have "ls" as the exe_file, resolve the executable location based
-    // on the current path variables
-    if (!FileSystem::Instance().Exists(resolved_module_spec.GetFileSpec())) {
-      resolved_module_spec.GetFileSpec().GetPath(exe_path, sizeof(exe_path));
-      resolved_module_spec.GetFileSpec().SetFile(exe_path,
-                                                 FileSpec::Style::native);
-      FileSystem::Instance().Resolve(resolved_module_spec.GetFileSpec());
-    }
-
-    if (!FileSystem::Instance().Exists(resolved_module_spec.GetFileSpec()))
-      FileSystem::Instance().ResolveExecutableLocation(
-          resolved_module_spec.GetFileSpec());
+  if (IsRemote())
+    return GetCachedExecutable(resolved_module_spec, exe_module_sp,
+                               module_search_paths_ptr);
 
-    // Resolve any executable within a bundle on MacOSX
-    Host::ResolveExecutableInBundle(resolved_module_spec.GetFileSpec());
 
-    if (FileSystem::Instance().Exists(resolved_module_spec.GetFileSpec()))
-      error.Clear();
-    else {
-      const uint32_t permissions = FileSystem::Instance().GetPermissions(
-          resolved_module_spec.GetFileSpec());
-      if (permissions && (permissions & eFilePermissionsEveryoneR) == 0)
-        error.SetErrorStringWithFormat(
-            "executable '%s' is not readable",
-            resolved_module_spec.GetFileSpec().GetPath().c_str());
-      else
-        error.SetErrorStringWithFormat(
-            "unable to find executable for '%s'",
-            resolved_module_spec.GetFileSpec().GetPath().c_str());
-    }
-  } else {
-    if (m_remote_platform_sp) {
-      return GetCachedExecutable(resolved_module_spec, exe_module_sp,
-                                 module_search_paths_ptr);
-    }
+  // If we have "ls" as the exe_file, resolve the executable location based
+  // on the current path variables
+  if (!FileSystem::Instance().Exists(resolved_module_spec.GetFileSpec())) {
+    char exe_path[PATH_MAX];
+    resolved_module_spec.GetFileSpec().GetPath(exe_path, sizeof(exe_path));
+    resolved_module_spec.GetFileSpec().SetFile(exe_path,
+                                               FileSpec::Style::native);
+    FileSystem::Instance().Resolve(resolved_module_spec.GetFileSpec());
+  }
 
-    // We may connect to a process and use the provided executable (Don't use
-    // local $PATH).
+  if (!FileSystem::Instance().Exists(resolved_module_spec.GetFileSpec()))
+    FileSystem::Instance().ResolveExecutableLocation(
+        resolved_module_spec.GetFileSpec());
 
-    // Resolve any executable within a bundle on MacOSX
-    Host::ResolveExecutableInBundle(resolved_module_spec.GetFileSpec());
+  // Resolve any executable within a bundle on MacOSX
+  Host::ResolveExecutableInBundle(resolved_module_spec.GetFileSpec());
 
-    if (FileSystem::Instance().Exists(resolved_module_spec.GetFileSpec()))
-      error.Clear();
+  Status error;
+  if (!FileSystem::Instance().Exists(resolved_module_spec.GetFileSpec())) {
+    const uint32_t permissions = FileSystem::Instance().GetPermissions(
+        resolved_module_spec.GetFileSpec());
+    if (permissions && (permissions & eFilePermissionsEveryoneR) == 0)
+      error.SetErrorStringWithFormat(
+          "executable '%s' is not readable",
+          resolved_module_spec.GetFileSpec().GetPath().c_str());
     else
-      error.SetErrorStringWithFormat("the platform is not currently "
-                                     "connected, and '%s' doesn't exist in "
-                                     "the system root.",
-                                     exe_path);
+      error.SetErrorStringWithFormat(
+          "unable to find executable for '%s'",
+          resolved_module_spec.GetFileSpec().GetPath().c_str());
   }
 
   if (error.Success()) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D117547.400764.patch
Type: text/x-patch
Size: 4405 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20220118/b42fdee6/attachment.bin>