[Lldb-commits] [PATCH] D57780: Don't include UnixSignals.h from Host

Pavel Labath via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Wed Feb 6 00:48:06 PST 2019


labath added reviewers: jingham, davide.
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

The rationale you give for this class being in Target is sound from the POV of lldb client. (the CreateForHost function is slightly fishy, but not too much.) However, it starts to break down when you start to consider lldb-server, whose large chunk of responsibility revolves around signals, but it doesn't need anything else in the Target module. For that, I think the ultimate place of UnixSignals should be somewhere else (I was planning to try to move it to Utility, which would mean that GetHostSignals could stay where it is).

However, we still have a long way to go before we can make lldb-server more independent, and the changes you make here are fairly small and easy to back out if needed. Since they solve the immediate problems with the Host module, I think it's fine to do this change now, and then revisit the placement of this class later.



================
Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp:401
 
-  const auto &signals = Host::GetUnixSignals();
+  lldb::UnixSignalsSP signals = UnixSignals::CreateForHost();
   for (auto signo = signals->GetFirstSignalNumber();
----------------
Despite being burried in `Plugins/Process/gdb-remote`, this code is actually part of lldb-server (and only lldb-server).


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

https://reviews.llvm.org/D57780





More information about the lldb-commits mailing list