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

[Raphael Isemann via Phabricator via lldb-commits]

teemperor requested changes to this revision.
teemperor added a comment.
This revision now requires changes to proceed.

Thanks for the patch (and tests!), this LGTM for the most part.

I'm maybe being nitpicky here, but could we replace all the `ret = ...` stuff and `exit(...)` with just `return ...`? That is more in line with LLVM's early-exit approach and makes the control flow easier to follow. The g/p cases could just use a scope_exit to guarantee we destroy the global state:

  case 'g':
      llgs::initialize();
      auto deinit = llvm::make_scope_exit([]() { llgs::terminate_debugger(); });
      return main_gdbserver(argc, argv);
  case 'p':
      llgs::initialize();
      auto deinit = llvm::make_scope_exit([]() { llgs::terminate_debugger(); });
      return main_platform(argc, argv);



================
Comment at: lldb/tools/lldb-server/lldb-platform.cpp:324
       WithColor::error() << error.AsCString() << '\n';
-      exit(socket_error);
+      return socket_error;
     }
----------------
FWIW, `socket_error` seems to be just a (constant) `-1`? I think this could just be `return 1`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108351