Skip to content

[lldb-dap] Return an llvm::Error instead of calling exit directly (NFC) #128951

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 27, 2025

Conversation

JDevlieghere
Copy link
Member

Return an llvm::Error from LaunchRunInTerminalTarget instead of calling exit() directly.

Return an `llvm::Error` from `LaunchRunInTerminalTarget` instead of
calling `exit()` directly.
@llvmbot
Copy link
Member

llvmbot commented Feb 26, 2025

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

Return an llvm::Error from LaunchRunInTerminalTarget instead of calling exit() directly.


Full diff: https://github.com/llvm/llvm-project/pull/128951.diff

1 Files Affected:

  • (modified) lldb/tools/lldb-dap/lldb-dap.cpp (+19-16)
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index 1c6bd7e903409..6b31f9d3cfe54 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -197,12 +197,13 @@ static void printHelp(LLDBDAPOptTable &table, llvm::StringRef tool_name) {
 //
 // In case of errors launching the target, a suitable error message will be
 // emitted to the debug adaptor.
-static void LaunchRunInTerminalTarget(llvm::opt::Arg &target_arg,
-                                      llvm::StringRef comm_file,
-                                      lldb::pid_t debugger_pid, char *argv[]) {
+static llvm::Error LaunchRunInTerminalTarget(llvm::opt::Arg &target_arg,
+                                             llvm::StringRef comm_file,
+                                             lldb::pid_t debugger_pid,
+                                             char *argv[]) {
 #if defined(_WIN32)
-  llvm::errs() << "runInTerminal is only supported on POSIX systems\n";
-  exit(EXIT_FAILURE);
+  return llvm::createStringError(
+      "runInTerminal is only supported on POSIX systems");
 #else
 
   // On Linux with the Yama security module enabled, a process can only attach
@@ -214,10 +215,8 @@ static void LaunchRunInTerminalTarget(llvm::opt::Arg &target_arg,
 #endif
 
   RunInTerminalLauncherCommChannel comm_channel(comm_file);
-  if (llvm::Error err = comm_channel.NotifyPid()) {
-    llvm::errs() << llvm::toString(std::move(err)) << "\n";
-    exit(EXIT_FAILURE);
-  }
+  if (llvm::Error err = comm_channel.NotifyPid())
+    return err;
 
   // We will wait to be attached with a timeout. We don't wait indefinitely
   // using a signal to prevent being paused forever.
@@ -228,8 +227,7 @@ static void LaunchRunInTerminalTarget(llvm::opt::Arg &target_arg,
       timeout_env_var != nullptr ? atoi(timeout_env_var) : 20000;
   if (llvm::Error err = comm_channel.WaitUntilDebugAdaptorAttaches(
           std::chrono::milliseconds(timeout_in_ms))) {
-    llvm::errs() << llvm::toString(std::move(err)) << "\n";
-    exit(EXIT_FAILURE);
+    return err;
   }
 
   const char *target = target_arg.getValue();
@@ -237,8 +235,8 @@ static void LaunchRunInTerminalTarget(llvm::opt::Arg &target_arg,
 
   std::string error = std::strerror(errno);
   comm_channel.NotifyError(error);
-  llvm::errs() << error << "\n";
-  exit(EXIT_FAILURE);
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+                                 std::move(error));
 #endif
 }
 
@@ -469,13 +467,18 @@ int main(int argc, char *argv[]) {
         }
       }
       int target_args_pos = argc;
-      for (int i = 0; i < argc; i++)
+      for (int i = 0; i < argc; i++) {
         if (strcmp(argv[i], "--launch-target") == 0) {
           target_args_pos = i + 1;
           break;
         }
-      LaunchRunInTerminalTarget(*target_arg, comm_file->getValue(), pid,
-                                argv + target_args_pos);
+      }
+      if (llvm::Error err =
+              LaunchRunInTerminalTarget(*target_arg, comm_file->getValue(), pid,
+                                        argv + target_args_pos)) {
+        llvm::errs() << llvm::toString(std::move(err)) << '\n';
+        return EXIT_FAILURE;
+      }
     } else {
       llvm::errs() << "\"--launch-target\" requires \"--comm-file\" to be "
                       "specified\n";

Copy link
Collaborator

@slackito slackito left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@JDevlieghere JDevlieghere merged commit f409340 into llvm:main Feb 27, 2025
13 checks passed
@JDevlieghere JDevlieghere deleted the lldb-dap-exit branch February 27, 2025 00:34
joaosaffran pushed a commit to joaosaffran/llvm-project that referenced this pull request Mar 3, 2025
…C) (llvm#128951)

Return an `llvm::Error` from `LaunchRunInTerminalTarget` instead of
calling `exit()` directly.
SquallATF pushed a commit to SquallATF/llvm-project that referenced this pull request Mar 8, 2025
…C) (llvm#128951)

Return an `llvm::Error` from `LaunchRunInTerminalTarget` instead of
calling `exit()` directly.
SquallATF pushed a commit to SquallATF/llvm-project that referenced this pull request Mar 20, 2025
…C) (llvm#128951)

Return an `llvm::Error` from `LaunchRunInTerminalTarget` instead of
calling `exit()` directly.
SquallATF pushed a commit to SquallATF/llvm-project that referenced this pull request Apr 2, 2025
…C) (llvm#128951)

Return an `llvm::Error` from `LaunchRunInTerminalTarget` instead of
calling `exit()` directly.
SquallATF pushed a commit to SquallATF/llvm-project that referenced this pull request Apr 17, 2025
…C) (llvm#128951)

Return an `llvm::Error` from `LaunchRunInTerminalTarget` instead of
calling `exit()` directly.
SquallATF pushed a commit to SquallATF/llvm-project that referenced this pull request Apr 30, 2025
…C) (llvm#128951)

Return an `llvm::Error` from `LaunchRunInTerminalTarget` instead of
calling `exit()` directly.
SquallATF pushed a commit to SquallATF/llvm-project that referenced this pull request May 15, 2025
…C) (llvm#128951)

Return an `llvm::Error` from `LaunchRunInTerminalTarget` instead of
calling `exit()` directly.
SquallATF pushed a commit to SquallATF/llvm-project that referenced this pull request May 29, 2025
…C) (llvm#128951)

Return an `llvm::Error` from `LaunchRunInTerminalTarget` instead of
calling `exit()` directly.
SquallATF pushed a commit to SquallATF/llvm-project that referenced this pull request Jun 13, 2025
…C) (llvm#128951)

Return an `llvm::Error` from `LaunchRunInTerminalTarget` instead of
calling `exit()` directly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants