labath added a comment.
Doing it in the common code is a great idea, and it will make the code more
robust. However, instead of "sneaking" the listener through a member variable,
it would be better to pass it through function arguments instead (normally in
the LaunchInfo struct, and as a probably as separate argument for the last
mile). I believe this is what Jim had in mind as well.
See inline comments for details.
================
Comment at: lldb/source/Target/Process.cpp:2538-2539
RestoreProcessEvents();
+ // Releasing the scope guard to avoid double restoring.
+ on_exit.release();
SetPublicState(state, false);
----------------
It would be better if this could be achieved by restructuring the code so that
the hijacking is really scope-based. E.g., moving the "file doesn't exist"
check up front and making it an early exit, moving some stuff into helper
functions, or whatever.
================
Comment at: lldb/source/Target/Target.cpp:202
llvm::StringRef plugin_name,
const FileSpec *crash_file,
bool can_connect) {
----------------
Add an extra argument here.
================
Comment at: lldb/source/Target/Target.cpp:3044-3045
+ // they invoke Process::Launch.
+ m_hijacker_on_process_creation_sp =
+ Listener::MakeListener(hijacking_listener_name);
+ launch_info.SetHijackListener(m_hijacker_on_process_creation_sp);
----------------
Remove this. Make sure the listener in the launch_info is plumbed to the launch
function.
================
Comment at: lldb/source/Target/Target.cpp:3069
m_process_sp =
GetPlatform()->DebugProcess(launch_info, debugger, *this, error);
----------------
Remove hijack listener creation from DebugProcess implementations (you can
replace with an assert if you want).
================
Comment at: lldb/source/Target/Target.cpp:3106-3107
+ hijack_listener_sp = Listener::MakeListener(hijacking_listener_name);
launch_info.SetHijackListener(hijack_listener_sp);
m_process_sp->HijackProcessEvents(hijack_listener_sp);
}
----------------
I guess we should just delete this now, as it is never correct to hijack events
this late in the game.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D119548/new/
https://reviews.llvm.org/D119548
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
