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
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
