[Lldb-commits] [PATCH] D108351: [lldb server] Tidy up LLDB server return codes and associated tests

[Sebastian Schwartz via Phabricator via lldb-commits]

saschwartz created this revision.
saschwartz added a reviewer: clayborg.
saschwartz requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This diff modifies the LLDB server return codes to more accurately
reflect usage error paths. It additionally modifies the associated unit test
to check that we have nonzero return codes on error conditions.

Test Plan:
LLDB tests pass:

  ninja check-lldb


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108351

Files:
  lldb/test/Shell/lldb-server/TestErrorMessages.test
  lldb/tools/lldb-server/lldb-server.cpp


Index: lldb/tools/lldb-server/lldb-server.cpp
===
--- lldb/tools/lldb-server/lldb-server.cpp
+++ lldb/tools/lldb-server/lldb-server.cpp
@@ -52,29 +52,29 @@
   llvm::InitLLVM IL(argc, argv, /*InstallPipeSignalExitHandler=*/false);
   llvm::PrettyStackTraceProgram X(argc, argv);
 
-  int option_error = 0;
   const char *progname = argv[0];
   if (argc < 2) {
 display_usage(progname);
-exit(option_error);
+exit(1);
   }
 
+  int ret = 0;
   switch (argv[1][0]) {
   case 'g':
 llgs::initialize();
-main_gdbserver(argc, argv);
+ret = main_gdbserver(argc, argv);
 llgs::terminate_debugger();
-break;
   case 'p':
 llgs::initialize();
-main_platform(argc, argv);
+ret = main_platform(argc, argv);
 llgs::terminate_debugger();
-break;
   case 'v':
 fprintf(stderr, "%s\n", lldb_private::GetVersion());
+ret = 0;
 break;
   default:
 display_usage(progname);
-exit(option_error);
+ret = 1;
   }
+  return ret;
 }
Index: lldb/test/Shell/lldb-server/TestErrorMessages.test
===
--- lldb/test/Shell/lldb-server/TestErrorMessages.test
+++ lldb/test/Shell/lldb-server/TestErrorMessages.test
@@ -1,13 +1,13 @@
-RUN: %lldb-server gdbserver --fd 2>&1 | FileCheck --check-prefixes=FD1,ALL %s
+RUN: not %lldb-server gdbserver --fd 2>&1 | FileCheck --check-prefixes=FD1,ALL 
%s
 FD1: error: --fd: missing argument
 
-RUN: %lldb-server gdbserver --fd three 2>&1 | FileCheck 
--check-prefixes=FD2,ALL %s
+RUN: not %lldb-server gdbserver --fd three 2>&1 | FileCheck 
--check-prefixes=FD2,ALL %s
 FD2: error: invalid '--fd' argument
 
-RUN: %lldb-server gdbserver --bogus 2>&1 | FileCheck 
--check-prefixes=BOGUS,ALL %s
+RUN: not %lldb-server gdbserver --bogus 2>&1 | FileCheck 
--check-prefixes=BOGUS,ALL %s
 BOGUS: error: unknown argument '--bogus'
 
-RUN: %lldb-server gdbserver 2>&1 | FileCheck --check-prefixes=CONN,ALL %s
+RUN: not %lldb-server gdbserver 2>&1 | FileCheck --check-prefixes=CONN,ALL %s
 CONN: error: no connection arguments
 
 ALL: Use '{{.*}} g[dbserver] --help' for a complete list of options.


Index: lldb/tools/lldb-server/lldb-server.cpp
===
--- lldb/tools/lldb-server/lldb-server.cpp
+++ lldb/tools/lldb-server/lldb-server.cpp
@@ -52,29 +52,29 @@
   llvm::InitLLVM IL(argc, argv, /*InstallPipeSignalExitHandler=*/false);
   llvm::PrettyStackTraceProgram X(argc, argv);
 
-  int option_error = 0;
   const char *progname = argv[0];
   if (argc < 2) {
 display_usage(progname);
-exit(option_error);
+exit(1);
   }
 
+  int ret = 0;
   switch (argv[1][0]) {
   case 'g':
 llgs::initialize();
-main_gdbserver(argc, argv);
+ret = main_gdbserver(argc, argv);
 llgs::terminate_debugger();
-break;
   case 'p':
 llgs::initialize();
-main_platform(argc, argv);
+ret = main_platform(argc, argv);
 llgs::terminate_debugger();
-break;
   case 'v':
 fprintf(stderr, "%s\n", lldb_private::GetVersion());
+ret = 0;
 break;
   default:
 display_usage(progname);
-exit(option_error);
+ret = 1;
   }
+  return ret;
 }
Index: lldb/test/Shell/lldb-server/TestErrorMessages.test
===
--- lldb/test/Shell/lldb-server/TestErrorMessages.test
+++ lldb/test/Shell/lldb-server/TestErrorMessages.test
@@ -1,13 +1,13 @@
-RUN: %lldb-server gdbserver --fd 2>&1 | FileCheck --check-prefixes=FD1,ALL %s
+RUN: not %lldb-server gdbserver --fd 2>&1 | FileCheck --check-prefixes=FD1,ALL %s
 FD1: error: --fd: missing argument
 
-RUN: %lldb-server gdbserver --fd three 2>&1 | FileCheck --check-prefixes=FD2,ALL %s
+RUN: not %lldb-server gdbserver --fd three 2>&1 | FileCheck --check-prefixes=FD2,ALL %s
 FD2: error: invalid '--fd' argument
 
-RUN: %lldb-server gdbserver --bogus 2>&1 | FileCheck --check-prefixes=BOGUS,ALL %s
+RUN: not %lldb-server gdbserver --bogus 2>&1 | FileCheck --check-prefixes=BOGUS,ALL %s
 BOGUS: error: unknown argument '--bogus'
 
-RUN: %lldb-server gdbserver 2>&1 | FileCheck --check-prefixes=CONN,ALL %s
+RUN: not %lldb-server gdbserver 2>&1 | FileCheck --check-prefixes=CONN,ALL %s
 CONN: error: no connection arguments
 
 ALL: Use '{{.*}} g[dbserver] --help' for a 

[Lldb-commits] [PATCH] D108351: [lldb server] Tidy up LLDB server return codes and associated tests

[Sebastian Schwartz via Phabricator via lldb-commits]

saschwartz updated this revision to Diff 367397.
saschwartz added a comment.

Fix unannotated fallthrough in switch statement.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108351/new/

https://reviews.llvm.org/D108351

Files:
  lldb/test/Shell/lldb-server/TestErrorMessages.test
  lldb/tools/lldb-server/lldb-server.cpp


Index: lldb/tools/lldb-server/lldb-server.cpp
===
--- lldb/tools/lldb-server/lldb-server.cpp
+++ lldb/tools/lldb-server/lldb-server.cpp
@@ -52,29 +52,32 @@
   llvm::InitLLVM IL(argc, argv, /*InstallPipeSignalExitHandler=*/false);
   llvm::PrettyStackTraceProgram X(argc, argv);
 
-  int option_error = 0;
   const char *progname = argv[0];
   if (argc < 2) {
 display_usage(progname);
-exit(option_error);
+exit(1);
   }
 
+  int ret = 0;
   switch (argv[1][0]) {
   case 'g':
 llgs::initialize();
-main_gdbserver(argc, argv);
+ret = main_gdbserver(argc, argv);
 llgs::terminate_debugger();
 break;
   case 'p':
 llgs::initialize();
-main_platform(argc, argv);
+ret = main_platform(argc, argv);
 llgs::terminate_debugger();
 break;
   case 'v':
 fprintf(stderr, "%s\n", lldb_private::GetVersion());
+ret = 0;
 break;
   default:
 display_usage(progname);
-exit(option_error);
+ret = 1;
+break;
   }
+  return ret;
 }
Index: lldb/test/Shell/lldb-server/TestErrorMessages.test
===
--- lldb/test/Shell/lldb-server/TestErrorMessages.test
+++ lldb/test/Shell/lldb-server/TestErrorMessages.test
@@ -1,13 +1,13 @@
-RUN: %lldb-server gdbserver --fd 2>&1 | FileCheck --check-prefixes=FD1,ALL %s
+RUN: not %lldb-server gdbserver --fd 2>&1 | FileCheck --check-prefixes=FD1,ALL 
%s
 FD1: error: --fd: missing argument
 
-RUN: %lldb-server gdbserver --fd three 2>&1 | FileCheck 
--check-prefixes=FD2,ALL %s
+RUN: not %lldb-server gdbserver --fd three 2>&1 | FileCheck 
--check-prefixes=FD2,ALL %s
 FD2: error: invalid '--fd' argument
 
-RUN: %lldb-server gdbserver --bogus 2>&1 | FileCheck 
--check-prefixes=BOGUS,ALL %s
+RUN: not %lldb-server gdbserver --bogus 2>&1 | FileCheck 
--check-prefixes=BOGUS,ALL %s
 BOGUS: error: unknown argument '--bogus'
 
-RUN: %lldb-server gdbserver 2>&1 | FileCheck --check-prefixes=CONN,ALL %s
+RUN: not %lldb-server gdbserver 2>&1 | FileCheck --check-prefixes=CONN,ALL %s
 CONN: error: no connection arguments
 
 ALL: Use '{{.*}} g[dbserver] --help' for a complete list of options.


Index: lldb/tools/lldb-server/lldb-server.cpp
===
--- lldb/tools/lldb-server/lldb-server.cpp
+++ lldb/tools/lldb-server/lldb-server.cpp
@@ -52,29 +52,32 @@
   llvm::InitLLVM IL(argc, argv, /*InstallPipeSignalExitHandler=*/false);
   llvm::PrettyStackTraceProgram X(argc, argv);
 
-  int option_error = 0;
   const char *progname = argv[0];
   if (argc < 2) {
 display_usage(progname);
-exit(option_error);
+exit(1);
   }
 
+  int ret = 0;
   switch (argv[1][0]) {
   case 'g':
 llgs::initialize();
-main_gdbserver(argc, argv);
+ret = main_gdbserver(argc, argv);
 llgs::terminate_debugger();
 break;
   case 'p':
 llgs::initialize();
-main_platform(argc, argv);
+ret = main_platform(argc, argv);
 llgs::terminate_debugger();
 break;
   case 'v':
 fprintf(stderr, "%s\n", lldb_private::GetVersion());
+ret = 0;
 break;
   default:
 display_usage(progname);
-exit(option_error);
+ret = 1;
+break;
   }
+  return ret;
 }
Index: lldb/test/Shell/lldb-server/TestErrorMessages.test
===
--- lldb/test/Shell/lldb-server/TestErrorMessages.test
+++ lldb/test/Shell/lldb-server/TestErrorMessages.test
@@ -1,13 +1,13 @@
-RUN: %lldb-server gdbserver --fd 2>&1 | FileCheck --check-prefixes=FD1,ALL %s
+RUN: not %lldb-server gdbserver --fd 2>&1 | FileCheck --check-prefixes=FD1,ALL %s
 FD1: error: --fd: missing argument
 
-RUN: %lldb-server gdbserver --fd three 2>&1 | FileCheck --check-prefixes=FD2,ALL %s
+RUN: not %lldb-server gdbserver --fd three 2>&1 | FileCheck --check-prefixes=FD2,ALL %s
 FD2: error: invalid '--fd' argument
 
-RUN: %lldb-server gdbserver --bogus 2>&1 | FileCheck --check-prefixes=BOGUS,ALL %s
+RUN: not %lldb-server gdbserver --bogus 2>&1 | FileCheck --check-prefixes=BOGUS,ALL %s
 BOGUS: error: unknown argument '--bogus'
 
-RUN: %lldb-server gdbserver 2>&1 | FileCheck --check-prefixes=CONN,ALL %s
+RUN: not %lldb-server gdbserver 2>&1 | FileCheck --check-prefixes=CONN,ALL %s
 CONN: error: no connection arguments
 
 ALL: Use '{{.*}} g[dbserver] --help' for a complete list of options.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D108351: [lldb server] Tidy up LLDB server return codes and associated tests

[Sebastian Schwartz via Phabricator via lldb-commits]

saschwartz updated this revision to Diff 367401.
saschwartz added a comment.

Also propagate platform mode return codes and add additional test cases


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108351/new/

https://reviews.llvm.org/D108351

Files:
  lldb/test/Shell/lldb-server/TestErrorMessages.test
  lldb/test/Shell/lldb-server/TestGdbserverPort.test
  lldb/tools/lldb-server/lldb-platform.cpp
  lldb/tools/lldb-server/lldb-server.cpp

Index: lldb/tools/lldb-server/lldb-server.cpp
===
--- lldb/tools/lldb-server/lldb-server.cpp
+++ lldb/tools/lldb-server/lldb-server.cpp
@@ -52,29 +52,32 @@
   llvm::InitLLVM IL(argc, argv, /*InstallPipeSignalExitHandler=*/false);
   llvm::PrettyStackTraceProgram X(argc, argv);
 
-  int option_error = 0;
   const char *progname = argv[0];
   if (argc < 2) {
 display_usage(progname);
-exit(option_error);
+exit(1);
   }
 
+  int ret = 0;
   switch (argv[1][0]) {
   case 'g':
 llgs::initialize();
-main_gdbserver(argc, argv);
+ret = main_gdbserver(argc, argv);
 llgs::terminate_debugger();
 break;
   case 'p':
 llgs::initialize();
-main_platform(argc, argv);
+ret = main_platform(argc, argv);
 llgs::terminate_debugger();
 break;
   case 'v':
 fprintf(stderr, "%s\n", lldb_private::GetVersion());
+ret = 0;
 break;
   default:
 display_usage(progname);
-exit(option_error);
+ret = 1;
+break;
   }
+  return ret;
 }
Index: lldb/tools/lldb-server/lldb-platform.cpp
===
--- lldb/tools/lldb-server/lldb-platform.cpp
+++ lldb/tools/lldb-server/lldb-platform.cpp
@@ -92,7 +92,6 @@
   "log-channel-list] [--port-file port-file-path] --server "
   "--listen port\n",
   progname, subcommand);
-  exit(0);
 }
 
 static Status save_socket_id_to_file(const std::string &socket_id,
@@ -269,7 +268,7 @@
 
   if (show_usage || option_error) {
 display_usage(progname, subcommand);
-exit(option_error);
+return option_error;
   }
 
   // Skip any options we consumed with getopt_long_only.
@@ -288,13 +287,13 @@
   listen_host_port, children_inherit_listen_socket, error));
   if (error.Fail()) {
 fprintf(stderr, "failed to create acceptor: %s", error.AsCString());
-exit(socket_error);
+return socket_error;
   }
 
   error = acceptor_up->Listen(backlog);
   if (error.Fail()) {
 printf("failed to listen: %s\n", error.AsCString());
-exit(socket_error);
+return socket_error;
   }
   if (socket_file) {
 error =
@@ -322,7 +321,7 @@
 error = acceptor_up->Accept(children_inherit_accept_socket, conn);
 if (error.Fail()) {
   WithColor::error() << error.AsCString() << '\n';
-  exit(socket_error);
+  return socket_error;
 }
 printf("Connection established.\n");
 if (g_server) {
Index: lldb/test/Shell/lldb-server/TestGdbserverPort.test
===
--- lldb/test/Shell/lldb-server/TestGdbserverPort.test
+++ lldb/test/Shell/lldb-server/TestGdbserverPort.test
@@ -1,4 +1,4 @@
 # Windows does not build lldb-server.
 # UNSUPPORTED: system-windows
-# RUN: %platformserver --server --listen :1234 --min-gdbserver-port 1234 --max-gdbserver-port 1234 2>&1 | FileCheck %s
+# RUN: not %platformserver --server --listen :1234 --min-gdbserver-port 1234 --max-gdbserver-port 1234 2>&1 | FileCheck %s
 # CHECK: error: --min-gdbserver-port (1234) is not lower than --max-gdbserver-port (1234)
Index: lldb/test/Shell/lldb-server/TestErrorMessages.test
===
--- lldb/test/Shell/lldb-server/TestErrorMessages.test
+++ lldb/test/Shell/lldb-server/TestErrorMessages.test
@@ -1,14 +1,26 @@
-RUN: %lldb-server gdbserver --fd 2>&1 | FileCheck --check-prefixes=FD1,ALL %s
+RUN: not %lldb-server gdbserver --fd 2>&1 | FileCheck --check-prefixes=FD1,GDB_REMOTE_ALL %s
 FD1: error: --fd: missing argument
 
-RUN: %lldb-server gdbserver --fd three 2>&1 | FileCheck --check-prefixes=FD2,ALL %s
+RUN: not %lldb-server gdbserver --fd three 2>&1 | FileCheck --check-prefixes=FD2,GDB_REMOTE_ALL %s
 FD2: error: invalid '--fd' argument
 
-RUN: %lldb-server gdbserver --bogus 2>&1 | FileCheck --check-prefixes=BOGUS,ALL %s
+RUN: not %lldb-server gdbserver --bogus 2>&1 | FileCheck --check-prefixes=BOGUS,GDB_REMOTE_ALL %s
 BOGUS: error: unknown argument '--bogus'
 
-RUN: %lldb-server gdbserver 2>&1 | FileCheck --check-prefixes=CONN,ALL %s
+RUN: not %lldb-server gdbserver 2>&1 | FileCheck --check-prefixes=CONN,GDB_REMOTE_ALL %s
 CONN: error: no connection arguments
 
-ALL: Use '{{.*}} g[dbserver] --help' for a complete list of options.
+RUN: %lldb-server platform 2>&1 | FileCheck --check-prefixes=LLDB_PLATFORM_ALL %s
+
+RUN: %lldb-server platform --fd 2>&1 | FileCheck --check-prefixes=FD3,LLDB_PLATFORM_

[Lldb-commits] [PATCH] D108351: [lldb server] Tidy up LLDB server return codes and associated tests

[Sebastian Schwartz via Phabricator via lldb-commits]

saschwartz updated this revision to Diff 367498.
saschwartz added a comment.

Use `make_scope_exit` to avoid superfluous return variable declaration


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108351/new/

https://reviews.llvm.org/D108351

Files:
  lldb/test/Shell/lldb-server/TestErrorMessages.test
  lldb/test/Shell/lldb-server/TestGdbserverPort.test
  lldb/tools/lldb-server/lldb-platform.cpp
  lldb/tools/lldb-server/lldb-server.cpp

Index: lldb/tools/lldb-server/lldb-server.cpp
===
--- lldb/tools/lldb-server/lldb-server.cpp
+++ lldb/tools/lldb-server/lldb-server.cpp
@@ -10,6 +10,7 @@
 #include "lldb/Initialization/SystemLifetimeManager.h"
 #include "lldb/lldb-private.h"
 
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/InitLLVM.h"
@@ -52,29 +53,32 @@
   llvm::InitLLVM IL(argc, argv, /*InstallPipeSignalExitHandler=*/false);
   llvm::PrettyStackTraceProgram X(argc, argv);
 
-  int option_error = 0;
   const char *progname = argv[0];
   if (argc < 2) {
 display_usage(progname);
-exit(option_error);
+exit(1);
   }
 
   switch (argv[1][0]) {
-  case 'g':
+  case 'g': {
 llgs::initialize();
-main_gdbserver(argc, argv);
-llgs::terminate_debugger();
+auto deinit = llvm::make_scope_exit([]() { llgs::terminate_debugger(); });
+return main_gdbserver(argc, argv);
 break;
-  case 'p':
+  }
+  case 'p': {
 llgs::initialize();
-main_platform(argc, argv);
-llgs::terminate_debugger();
+auto deinit = llvm::make_scope_exit([]() { llgs::terminate_debugger(); });
+return main_platform(argc, argv);
 break;
+  }
   case 'v':
 fprintf(stderr, "%s\n", lldb_private::GetVersion());
+return 0;
 break;
   default:
 display_usage(progname);
-exit(option_error);
+return 1;
+break;
   }
 }
Index: lldb/tools/lldb-server/lldb-platform.cpp
===
--- lldb/tools/lldb-server/lldb-platform.cpp
+++ lldb/tools/lldb-server/lldb-platform.cpp
@@ -92,7 +92,6 @@
   "log-channel-list] [--port-file port-file-path] --server "
   "--listen port\n",
   progname, subcommand);
-  exit(0);
 }
 
 static Status save_socket_id_to_file(const std::string &socket_id,
@@ -165,7 +164,6 @@
   FileSpec socket_file;
   bool show_usage = false;
   int option_error = 0;
-  int socket_error = -1;
 
   std::string short_options(OptionParser::GetShortOptionString(g_long_options));
 
@@ -269,7 +267,7 @@
 
   if (show_usage || option_error) {
 display_usage(progname, subcommand);
-exit(option_error);
+return option_error;
   }
 
   // Skip any options we consumed with getopt_long_only.
@@ -288,13 +286,13 @@
   listen_host_port, children_inherit_listen_socket, error));
   if (error.Fail()) {
 fprintf(stderr, "failed to create acceptor: %s", error.AsCString());
-exit(socket_error);
+return -1;
   }
 
   error = acceptor_up->Listen(backlog);
   if (error.Fail()) {
 printf("failed to listen: %s\n", error.AsCString());
-exit(socket_error);
+return -1;
   }
   if (socket_file) {
 error =
@@ -322,7 +320,7 @@
 error = acceptor_up->Accept(children_inherit_accept_socket, conn);
 if (error.Fail()) {
   WithColor::error() << error.AsCString() << '\n';
-  exit(socket_error);
+  return -1;
 }
 printf("Connection established.\n");
 if (g_server) {
Index: lldb/test/Shell/lldb-server/TestGdbserverPort.test
===
--- lldb/test/Shell/lldb-server/TestGdbserverPort.test
+++ lldb/test/Shell/lldb-server/TestGdbserverPort.test
@@ -1,4 +1,4 @@
 # Windows does not build lldb-server.
 # UNSUPPORTED: system-windows
-# RUN: %platformserver --server --listen :1234 --min-gdbserver-port 1234 --max-gdbserver-port 1234 2>&1 | FileCheck %s
+# RUN: not %platformserver --server --listen :1234 --min-gdbserver-port 1234 --max-gdbserver-port 1234 2>&1 | FileCheck %s
 # CHECK: error: --min-gdbserver-port (1234) is not lower than --max-gdbserver-port (1234)
Index: lldb/test/Shell/lldb-server/TestErrorMessages.test
===
--- lldb/test/Shell/lldb-server/TestErrorMessages.test
+++ lldb/test/Shell/lldb-server/TestErrorMessages.test
@@ -1,14 +1,26 @@
-RUN: %lldb-server gdbserver --fd 2>&1 | FileCheck --check-prefixes=FD1,ALL %s
+RUN: not %lldb-server gdbserver --fd 2>&1 | FileCheck --check-prefixes=FD1,GDB_REMOTE_ALL %s
 FD1: error: --fd: missing argument
 
-RUN: %lldb-server gdbserver --fd three 2>&1 | FileCheck --check-prefixes=FD2,ALL %s
+RUN: not %lldb-server gdbserver --fd three 2>&1 | FileCheck --check-prefixes=FD2,GDB_REMOTE_ALL %s
 FD2: error: invalid '--fd' argument
 
-RUN: %lldb-server gdbserver --bogus 2>&1 | FileCheck --check-prefixes=BOGUS,

[Lldb-commits] [PATCH] D108351: [lldb server] Tidy up LLDB server return codes and associated tests

[Sebastian Schwartz via Phabricator via lldb-commits]

saschwartz added inline comments.



Comment at: lldb/tools/lldb-server/lldb-platform.cpp:324
   WithColor::error() << error.AsCString() << '\n';
-  exit(socket_error);
+  return socket_error;
 }

teemperor wrote:
> FWIW, `socket_error` seems to be just a (constant) `-1`? I think this could 
> just be `return 1`.
Sounds fine, will change.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108351/new/

https://reviews.llvm.org/D108351

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D108351: [lldb server] Tidy up LLDB server return codes and associated tests

[Sebastian Schwartz via Phabricator via lldb-commits]

saschwartz updated this revision to Diff 367508.
saschwartz added a comment.

Fix lint


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108351/new/

https://reviews.llvm.org/D108351

Files:
  lldb/test/Shell/lldb-server/TestErrorMessages.test
  lldb/test/Shell/lldb-server/TestGdbserverPort.test
  lldb/tools/lldb-server/lldb-platform.cpp
  lldb/tools/lldb-server/lldb-server.cpp

Index: lldb/tools/lldb-server/lldb-server.cpp
===
--- lldb/tools/lldb-server/lldb-server.cpp
+++ lldb/tools/lldb-server/lldb-server.cpp
@@ -11,6 +11,7 @@
 #include "lldb/lldb-private.h"
 
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/ManagedStatic.h"
@@ -52,29 +53,32 @@
   llvm::InitLLVM IL(argc, argv, /*InstallPipeSignalExitHandler=*/false);
   llvm::PrettyStackTraceProgram X(argc, argv);
 
-  int option_error = 0;
   const char *progname = argv[0];
   if (argc < 2) {
 display_usage(progname);
-exit(option_error);
+exit(1);
   }
 
   switch (argv[1][0]) {
-  case 'g':
+  case 'g': {
 llgs::initialize();
-main_gdbserver(argc, argv);
-llgs::terminate_debugger();
+auto deinit = llvm::make_scope_exit([]() { llgs::terminate_debugger(); });
+return main_gdbserver(argc, argv);
 break;
-  case 'p':
+  }
+  case 'p': {
 llgs::initialize();
-main_platform(argc, argv);
-llgs::terminate_debugger();
+auto deinit = llvm::make_scope_exit([]() { llgs::terminate_debugger(); });
+return main_platform(argc, argv);
 break;
+  }
   case 'v':
 fprintf(stderr, "%s\n", lldb_private::GetVersion());
+return 0;
 break;
   default:
 display_usage(progname);
-exit(option_error);
+return 1;
+break;
   }
 }
Index: lldb/tools/lldb-server/lldb-platform.cpp
===
--- lldb/tools/lldb-server/lldb-platform.cpp
+++ lldb/tools/lldb-server/lldb-platform.cpp
@@ -92,7 +92,6 @@
   "log-channel-list] [--port-file port-file-path] --server "
   "--listen port\n",
   progname, subcommand);
-  exit(0);
 }
 
 static Status save_socket_id_to_file(const std::string &socket_id,
@@ -165,7 +164,6 @@
   FileSpec socket_file;
   bool show_usage = false;
   int option_error = 0;
-  int socket_error = -1;
 
   std::string short_options(OptionParser::GetShortOptionString(g_long_options));
 
@@ -269,7 +267,7 @@
 
   if (show_usage || option_error) {
 display_usage(progname, subcommand);
-exit(option_error);
+return option_error;
   }
 
   // Skip any options we consumed with getopt_long_only.
@@ -288,13 +286,13 @@
   listen_host_port, children_inherit_listen_socket, error));
   if (error.Fail()) {
 fprintf(stderr, "failed to create acceptor: %s", error.AsCString());
-exit(socket_error);
+return -1;
   }
 
   error = acceptor_up->Listen(backlog);
   if (error.Fail()) {
 printf("failed to listen: %s\n", error.AsCString());
-exit(socket_error);
+return -1;
   }
   if (socket_file) {
 error =
@@ -322,7 +320,7 @@
 error = acceptor_up->Accept(children_inherit_accept_socket, conn);
 if (error.Fail()) {
   WithColor::error() << error.AsCString() << '\n';
-  exit(socket_error);
+  return -1;
 }
 printf("Connection established.\n");
 if (g_server) {
Index: lldb/test/Shell/lldb-server/TestGdbserverPort.test
===
--- lldb/test/Shell/lldb-server/TestGdbserverPort.test
+++ lldb/test/Shell/lldb-server/TestGdbserverPort.test
@@ -1,4 +1,4 @@
 # Windows does not build lldb-server.
 # UNSUPPORTED: system-windows
-# RUN: %platformserver --server --listen :1234 --min-gdbserver-port 1234 --max-gdbserver-port 1234 2>&1 | FileCheck %s
+# RUN: not %platformserver --server --listen :1234 --min-gdbserver-port 1234 --max-gdbserver-port 1234 2>&1 | FileCheck %s
 # CHECK: error: --min-gdbserver-port (1234) is not lower than --max-gdbserver-port (1234)
Index: lldb/test/Shell/lldb-server/TestErrorMessages.test
===
--- lldb/test/Shell/lldb-server/TestErrorMessages.test
+++ lldb/test/Shell/lldb-server/TestErrorMessages.test
@@ -1,14 +1,26 @@
-RUN: %lldb-server gdbserver --fd 2>&1 | FileCheck --check-prefixes=FD1,ALL %s
+RUN: not %lldb-server gdbserver --fd 2>&1 | FileCheck --check-prefixes=FD1,GDB_REMOTE_ALL %s
 FD1: error: --fd: missing argument
 
-RUN: %lldb-server gdbserver --fd three 2>&1 | FileCheck --check-prefixes=FD2,ALL %s
+RUN: not %lldb-server gdbserver --fd three 2>&1 | FileCheck --check-prefixes=FD2,GDB_REMOTE_ALL %s
 FD2: error: invalid '--fd' argument
 
-RUN: %lldb-server gdbserver --bogus 2>&1 | FileCheck --check-prefixes=BOGUS,ALL %s
+RUN: not %lldb-server gdbserver --bogus 2>&1 | FileCheck --check-pref

[Lldb-commits] [PATCH] D108351: [lldb server] Tidy up LLDB server return codes and associated tests

[Sebastian Schwartz via Phabricator via lldb-commits]

saschwartz added a comment.

Responded to comments - will tidy up the nits.




Comment at: lldb/tools/lldb-server/lldb-platform.cpp:289
 fprintf(stderr, "failed to create acceptor: %s", error.AsCString());
-exit(socket_error);
+return -1;
   }

teemperor wrote:
> clayborg wrote:
> > Should we return error.GetError() if it is non zero? IIRC it will be the 
> > actual errno. And best to not return -1, just return 1.
> > 
> > ```
> > uint32_t SBError::GetError() const;
> > ```
> If we force the caller to convert errno to an exit code, then we could also 
> just return the `Status error` itself (and then the caller can just return 
> 0/1 depending on success or error)? That seems more clear than returning 
> `errno` from a function with main signature (which makes it look like it 
> would return an exit code).
Sounds fine to me - I went with `-1` because that was the original value for 
`socket_error`, but don't think anything should be conditioning on that. 

I'm pretty ambivalent to the `Status error` vs an error code directly myself, 
mainly because I don't know LLVM well enough to know what the convention might 
be. Will `error.GetError` always be nonzero if `error.Fail()` is true?



Comment at: lldb/tools/lldb-server/lldb-server.cpp:59
 display_usage(progname);
-exit(option_error);
+exit(1);
   }

teemperor wrote:
> `return 1;`
Thanks, good catch.



Comment at: lldb/tools/lldb-server/lldb-server.cpp:67
+return main_gdbserver(argc, argv);
 break;
+  }

teemperor wrote:
> That break and the ones below are now dead code.
Hmm, I thought the compiler would complain without a break statement but let me 
try. Thanks for review.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108351/new/

https://reviews.llvm.org/D108351

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D108351: [lldb server] Tidy up LLDB server return codes and associated tests

[Sebastian Schwartz via Phabricator via lldb-commits]

saschwartz updated this revision to Diff 367685.
saschwartz added a comment.

Address review feedback


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108351/new/

https://reviews.llvm.org/D108351

Files:
  lldb/test/Shell/lldb-server/TestErrorMessages.test
  lldb/test/Shell/lldb-server/TestGdbserverPort.test
  lldb/tools/lldb-server/lldb-platform.cpp
  lldb/tools/lldb-server/lldb-server.cpp

Index: lldb/tools/lldb-server/lldb-server.cpp
===
--- lldb/tools/lldb-server/lldb-server.cpp
+++ lldb/tools/lldb-server/lldb-server.cpp
@@ -11,6 +11,7 @@
 #include "lldb/lldb-private.h"
 
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/ManagedStatic.h"
@@ -52,29 +53,30 @@
   llvm::InitLLVM IL(argc, argv, /*InstallPipeSignalExitHandler=*/false);
   llvm::PrettyStackTraceProgram X(argc, argv);
 
-  int option_error = 0;
   const char *progname = argv[0];
   if (argc < 2) {
 display_usage(progname);
-exit(option_error);
+exit(1);
   }
 
   switch (argv[1][0]) {
-  case 'g':
+  case 'g': {
 llgs::initialize();
-main_gdbserver(argc, argv);
-llgs::terminate_debugger();
-break;
-  case 'p':
+auto deinit = llvm::make_scope_exit([]() { llgs::terminate_debugger(); });
+return main_gdbserver(argc, argv);
+  }
+  case 'p': {
 llgs::initialize();
-main_platform(argc, argv);
-llgs::terminate_debugger();
-break;
-  case 'v':
+auto deinit = llvm::make_scope_exit([]() { llgs::terminate_debugger(); });
+return main_platform(argc, argv);
+  }
+  case 'v': {
 fprintf(stderr, "%s\n", lldb_private::GetVersion());
-break;
-  default:
+return 0;
+  }
+  default: {
 display_usage(progname);
-exit(option_error);
+return 1;
+  }
   }
 }
Index: lldb/tools/lldb-server/lldb-platform.cpp
===
--- lldb/tools/lldb-server/lldb-platform.cpp
+++ lldb/tools/lldb-server/lldb-platform.cpp
@@ -92,7 +92,6 @@
   "log-channel-list] [--port-file port-file-path] --server "
   "--listen port\n",
   progname, subcommand);
-  exit(0);
 }
 
 static Status save_socket_id_to_file(const std::string &socket_id,
@@ -165,7 +164,6 @@
   FileSpec socket_file;
   bool show_usage = false;
   int option_error = 0;
-  int socket_error = -1;
 
   std::string short_options(OptionParser::GetShortOptionString(g_long_options));
 
@@ -249,7 +247,7 @@
   }
 
   if (!LLDBServerUtilities::SetupLogging(log_file, log_channels, 0))
-return -1;
+return 1;
 
   // Make a port map for a port range that was specified.
   if (min_gdbserver_port && min_gdbserver_port < max_gdbserver_port) {
@@ -269,7 +267,7 @@
 
   if (show_usage || option_error) {
 display_usage(progname, subcommand);
-exit(option_error);
+return option_error;
   }
 
   // Skip any options we consumed with getopt_long_only.
@@ -288,13 +286,13 @@
   listen_host_port, children_inherit_listen_socket, error));
   if (error.Fail()) {
 fprintf(stderr, "failed to create acceptor: %s", error.AsCString());
-exit(socket_error);
+return error.GetError() ? error.GetError() : 1;
   }
 
   error = acceptor_up->Listen(backlog);
   if (error.Fail()) {
 printf("failed to listen: %s\n", error.AsCString());
-exit(socket_error);
+return error.GetError() ? error.GetError() : 1;
   }
   if (socket_file) {
 error =
@@ -302,7 +300,7 @@
 if (error.Fail()) {
   fprintf(stderr, "failed to write socket id to %s: %s\n",
   socket_file.GetPath().c_str(), error.AsCString());
-  return 1;
+  return error.GetError() ? error.GetError() : 1;
 }
   }
 
@@ -322,7 +320,7 @@
 error = acceptor_up->Accept(children_inherit_accept_socket, conn);
 if (error.Fail()) {
   WithColor::error() << error.AsCString() << '\n';
-  exit(socket_error);
+  return error.GetError() ? error.GetError() : 1;
 }
 printf("Connection established.\n");
 if (g_server) {
Index: lldb/test/Shell/lldb-server/TestGdbserverPort.test
===
--- lldb/test/Shell/lldb-server/TestGdbserverPort.test
+++ lldb/test/Shell/lldb-server/TestGdbserverPort.test
@@ -1,4 +1,4 @@
 # Windows does not build lldb-server.
 # UNSUPPORTED: system-windows
-# RUN: %platformserver --server --listen :1234 --min-gdbserver-port 1234 --max-gdbserver-port 1234 2>&1 | FileCheck %s
+# RUN: not %platformserver --server --listen :1234 --min-gdbserver-port 1234 --max-gdbserver-port 1234 2>&1 | FileCheck %s
 # CHECK: error: --min-gdbserver-port (1234) is not lower than --max-gdbserver-port (1234)
Index: lldb/test/Shell/lldb-server/TestErrorMessages.test
===
--- lldb/test/Shell/lldb-server/TestErrorM

[Lldb-commits] [PATCH] D108351: [lldb server] Tidy up LLDB server return codes and associated tests

[Sebastian Schwartz via Phabricator via lldb-commits]

saschwartz updated this revision to Diff 367688.
saschwartz marked an inline comment as done.
saschwartz added a comment.

Fix one missed return nit.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108351/new/

https://reviews.llvm.org/D108351

Files:
  lldb/test/Shell/lldb-server/TestErrorMessages.test
  lldb/test/Shell/lldb-server/TestGdbserverPort.test
  lldb/tools/lldb-server/lldb-platform.cpp
  lldb/tools/lldb-server/lldb-server.cpp

Index: lldb/tools/lldb-server/lldb-server.cpp
===
--- lldb/tools/lldb-server/lldb-server.cpp
+++ lldb/tools/lldb-server/lldb-server.cpp
@@ -11,6 +11,7 @@
 #include "lldb/lldb-private.h"
 
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/ManagedStatic.h"
@@ -52,29 +53,30 @@
   llvm::InitLLVM IL(argc, argv, /*InstallPipeSignalExitHandler=*/false);
   llvm::PrettyStackTraceProgram X(argc, argv);
 
-  int option_error = 0;
   const char *progname = argv[0];
   if (argc < 2) {
 display_usage(progname);
-exit(option_error);
+return 1;
   }
 
   switch (argv[1][0]) {
-  case 'g':
+  case 'g': {
 llgs::initialize();
-main_gdbserver(argc, argv);
-llgs::terminate_debugger();
-break;
-  case 'p':
+auto deinit = llvm::make_scope_exit([]() { llgs::terminate_debugger(); });
+return main_gdbserver(argc, argv);
+  }
+  case 'p': {
 llgs::initialize();
-main_platform(argc, argv);
-llgs::terminate_debugger();
-break;
-  case 'v':
+auto deinit = llvm::make_scope_exit([]() { llgs::terminate_debugger(); });
+return main_platform(argc, argv);
+  }
+  case 'v': {
 fprintf(stderr, "%s\n", lldb_private::GetVersion());
-break;
-  default:
+return 0;
+  }
+  default: {
 display_usage(progname);
-exit(option_error);
+return 1;
+  }
   }
 }
Index: lldb/tools/lldb-server/lldb-platform.cpp
===
--- lldb/tools/lldb-server/lldb-platform.cpp
+++ lldb/tools/lldb-server/lldb-platform.cpp
@@ -92,7 +92,6 @@
   "log-channel-list] [--port-file port-file-path] --server "
   "--listen port\n",
   progname, subcommand);
-  exit(0);
 }
 
 static Status save_socket_id_to_file(const std::string &socket_id,
@@ -165,7 +164,6 @@
   FileSpec socket_file;
   bool show_usage = false;
   int option_error = 0;
-  int socket_error = -1;
 
   std::string short_options(OptionParser::GetShortOptionString(g_long_options));
 
@@ -249,7 +247,7 @@
   }
 
   if (!LLDBServerUtilities::SetupLogging(log_file, log_channels, 0))
-return -1;
+return 1;
 
   // Make a port map for a port range that was specified.
   if (min_gdbserver_port && min_gdbserver_port < max_gdbserver_port) {
@@ -269,7 +267,7 @@
 
   if (show_usage || option_error) {
 display_usage(progname, subcommand);
-exit(option_error);
+return option_error;
   }
 
   // Skip any options we consumed with getopt_long_only.
@@ -288,13 +286,13 @@
   listen_host_port, children_inherit_listen_socket, error));
   if (error.Fail()) {
 fprintf(stderr, "failed to create acceptor: %s", error.AsCString());
-exit(socket_error);
+return error.GetError() ? error.GetError() : 1;
   }
 
   error = acceptor_up->Listen(backlog);
   if (error.Fail()) {
 printf("failed to listen: %s\n", error.AsCString());
-exit(socket_error);
+return error.GetError() ? error.GetError() : 1;
   }
   if (socket_file) {
 error =
@@ -302,7 +300,7 @@
 if (error.Fail()) {
   fprintf(stderr, "failed to write socket id to %s: %s\n",
   socket_file.GetPath().c_str(), error.AsCString());
-  return 1;
+  return error.GetError() ? error.GetError() : 1;
 }
   }
 
@@ -322,7 +320,7 @@
 error = acceptor_up->Accept(children_inherit_accept_socket, conn);
 if (error.Fail()) {
   WithColor::error() << error.AsCString() << '\n';
-  exit(socket_error);
+  return error.GetError() ? error.GetError() : 1;
 }
 printf("Connection established.\n");
 if (g_server) {
Index: lldb/test/Shell/lldb-server/TestGdbserverPort.test
===
--- lldb/test/Shell/lldb-server/TestGdbserverPort.test
+++ lldb/test/Shell/lldb-server/TestGdbserverPort.test
@@ -1,4 +1,4 @@
 # Windows does not build lldb-server.
 # UNSUPPORTED: system-windows
-# RUN: %platformserver --server --listen :1234 --min-gdbserver-port 1234 --max-gdbserver-port 1234 2>&1 | FileCheck %s
+# RUN: not %platformserver --server --listen :1234 --min-gdbserver-port 1234 --max-gdbserver-port 1234 2>&1 | FileCheck %s
 # CHECK: error: --min-gdbserver-port (1234) is not lower than --max-gdbserver-port (1234)
Index: lldb/test/Shell/lldb-server/TestErrorMessages.test
=

[Lldb-commits] [PATCH] D108351: [lldb server] Tidy up LLDB server return codes and associated tests

[Sebastian Schwartz via Phabricator via lldb-commits]

saschwartz marked 5 inline comments as done.
saschwartz added inline comments.



Comment at: lldb/tools/lldb-server/lldb-platform.cpp:289
 fprintf(stderr, "failed to create acceptor: %s", error.AsCString());
-exit(socket_error);
+return -1;
   }

teemperor wrote:
> saschwartz wrote:
> > teemperor wrote:
> > > clayborg wrote:
> > > > Should we return error.GetError() if it is non zero? IIRC it will be 
> > > > the actual errno. And best to not return -1, just return 1.
> > > > 
> > > > ```
> > > > uint32_t SBError::GetError() const;
> > > > ```
> > > If we force the caller to convert errno to an exit code, then we could 
> > > also just return the `Status error` itself (and then the caller can just 
> > > return 0/1 depending on success or error)? That seems more clear than 
> > > returning `errno` from a function with main signature (which makes it 
> > > look like it would return an exit code).
> > Sounds fine to me - I went with `-1` because that was the original value 
> > for `socket_error`, but don't think anything should be conditioning on 
> > that. 
> > 
> > I'm pretty ambivalent to the `Status error` vs an error code directly 
> > myself, mainly because I don't know LLVM well enough to know what the 
> > convention might be. Will `error.GetError` always be nonzero if 
> > `error.Fail()` is true?
> As said above, for this to work we need to have the caller still transform 
> the error code into a valid exit code. Status will give us any integer back 
> (errno or something else we made up), but if we return that from `main` then 
> the exit code will be set on UNIX to the lowest 8 bits of that value. So 
> essentially right now we implicitly do `exit_code = error % 256`. That only 
> works if the system agrees to never use a multiple of 256 as an error code 
> and the same goes for LLDB internally. And then there is also all the other 
> weird stuff other operating systems will do here.
> 
> I don't see any other error code in LLVM beside 0/1/-1 so let's just keep 
> this patch simple and return one of those from `main`. I don't think it 
> matters a lot what we return from this artificial main method, but if it's an 
> `int` then it looks like an exit code from the function signature and it 
> should be a reasonable exit code. So if we want to return an actual error 
> code then it should be wrapped in a `Status` to make it clear to the caller 
> that they need to convert it to an exit code.
> 
> > Will error.GetError always be nonzero if error.Fail() is true?
> 
> Yes, `GetError` returns non-zero on failure, but the clearer check is 
> `!error.Success()` (which does the same check under the hood).
Sounds fine. That's a good argument about the implicit `% 256` operation which 
I tend to agree would be an unexpected operation for the caller to have to deal 
with. Sounds fine to stick with the typical semantics of a `main` function and 
just `return 1` for all errors.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108351/new/

https://reviews.llvm.org/D108351

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D108351: [lldb server] Tidy up LLDB server return codes and associated tests

[Sebastian Schwartz via Phabricator via lldb-commits]

saschwartz updated this revision to Diff 367815.
saschwartz added a comment.

Stick with integer return codes from lldb-platform


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108351/new/

https://reviews.llvm.org/D108351

Files:
  lldb/test/Shell/lldb-server/TestErrorMessages.test
  lldb/test/Shell/lldb-server/TestGdbserverPort.test
  lldb/tools/lldb-server/lldb-platform.cpp
  lldb/tools/lldb-server/lldb-server.cpp

Index: lldb/tools/lldb-server/lldb-server.cpp
===
--- lldb/tools/lldb-server/lldb-server.cpp
+++ lldb/tools/lldb-server/lldb-server.cpp
@@ -11,6 +11,7 @@
 #include "lldb/lldb-private.h"
 
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/ManagedStatic.h"
@@ -52,29 +53,30 @@
   llvm::InitLLVM IL(argc, argv, /*InstallPipeSignalExitHandler=*/false);
   llvm::PrettyStackTraceProgram X(argc, argv);
 
-  int option_error = 0;
   const char *progname = argv[0];
   if (argc < 2) {
 display_usage(progname);
-exit(option_error);
+return 1;
   }
 
   switch (argv[1][0]) {
-  case 'g':
+  case 'g': {
 llgs::initialize();
-main_gdbserver(argc, argv);
-llgs::terminate_debugger();
-break;
-  case 'p':
+auto deinit = llvm::make_scope_exit([]() { llgs::terminate_debugger(); });
+return main_gdbserver(argc, argv);
+  }
+  case 'p': {
 llgs::initialize();
-main_platform(argc, argv);
-llgs::terminate_debugger();
-break;
-  case 'v':
+auto deinit = llvm::make_scope_exit([]() { llgs::terminate_debugger(); });
+return main_platform(argc, argv);
+  }
+  case 'v': {
 fprintf(stderr, "%s\n", lldb_private::GetVersion());
-break;
-  default:
+return 0;
+  }
+  default: {
 display_usage(progname);
-exit(option_error);
+return 1;
+  }
   }
 }
Index: lldb/tools/lldb-server/lldb-platform.cpp
===
--- lldb/tools/lldb-server/lldb-platform.cpp
+++ lldb/tools/lldb-server/lldb-platform.cpp
@@ -92,7 +92,6 @@
   "log-channel-list] [--port-file port-file-path] --server "
   "--listen port\n",
   progname, subcommand);
-  exit(0);
 }
 
 static Status save_socket_id_to_file(const std::string &socket_id,
@@ -165,7 +164,6 @@
   FileSpec socket_file;
   bool show_usage = false;
   int option_error = 0;
-  int socket_error = -1;
 
   std::string short_options(OptionParser::GetShortOptionString(g_long_options));
 
@@ -249,7 +247,7 @@
   }
 
   if (!LLDBServerUtilities::SetupLogging(log_file, log_channels, 0))
-return -1;
+return 1;
 
   // Make a port map for a port range that was specified.
   if (min_gdbserver_port && min_gdbserver_port < max_gdbserver_port) {
@@ -269,7 +267,7 @@
 
   if (show_usage || option_error) {
 display_usage(progname, subcommand);
-exit(option_error);
+return option_error;
   }
 
   // Skip any options we consumed with getopt_long_only.
@@ -288,13 +286,13 @@
   listen_host_port, children_inherit_listen_socket, error));
   if (error.Fail()) {
 fprintf(stderr, "failed to create acceptor: %s", error.AsCString());
-exit(socket_error);
+return 1;
   }
 
   error = acceptor_up->Listen(backlog);
   if (error.Fail()) {
 printf("failed to listen: %s\n", error.AsCString());
-exit(socket_error);
+return 1;
   }
   if (socket_file) {
 error =
@@ -322,7 +320,7 @@
 error = acceptor_up->Accept(children_inherit_accept_socket, conn);
 if (error.Fail()) {
   WithColor::error() << error.AsCString() << '\n';
-  exit(socket_error);
+  return 1;
 }
 printf("Connection established.\n");
 if (g_server) {
Index: lldb/test/Shell/lldb-server/TestGdbserverPort.test
===
--- lldb/test/Shell/lldb-server/TestGdbserverPort.test
+++ lldb/test/Shell/lldb-server/TestGdbserverPort.test
@@ -1,4 +1,4 @@
 # Windows does not build lldb-server.
 # UNSUPPORTED: system-windows
-# RUN: %platformserver --server --listen :1234 --min-gdbserver-port 1234 --max-gdbserver-port 1234 2>&1 | FileCheck %s
+# RUN: not %platformserver --server --listen :1234 --min-gdbserver-port 1234 --max-gdbserver-port 1234 2>&1 | FileCheck %s
 # CHECK: error: --min-gdbserver-port (1234) is not lower than --max-gdbserver-port (1234)
Index: lldb/test/Shell/lldb-server/TestErrorMessages.test
===
--- lldb/test/Shell/lldb-server/TestErrorMessages.test
+++ lldb/test/Shell/lldb-server/TestErrorMessages.test
@@ -1,14 +1,26 @@
-RUN: %lldb-server gdbserver --fd 2>&1 | FileCheck --check-prefixes=FD1,ALL %s
+RUN: not %lldb-server gdbserver --fd 2>&1 | FileCheck --check-prefixes=FD1,GDB_REMOTE_ALL %s
 FD1: error: --fd: missing argument
 
-RUN: %lldb-server gdbserver --fd three 2>&1 | Fil

[Lldb-commits] [PATCH] D108351: [lldb server] Tidy up LLDB server return codes and associated tests

[Sebastian Schwartz via Phabricator via lldb-commits]

saschwartz added a comment.

Hey there - just wondering if someone might be able to merge this in a while?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108351/new/

https://reviews.llvm.org/D108351

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D108351: [lldb server] Tidy up LLDB server return codes and associated tests

[Sebastian Schwartz via Phabricator via lldb-commits]

saschwartz added a comment.

Yes, will take a look. I had `check-lldb` pass for the new tests locally 
previously - I'll rebase and rerun and see what happens.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108351/new/

https://reviews.llvm.org/D108351

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits