> On Mar 6, 2017, at 4:10 PM, Greg Clayton via Phabricator > <revi...@reviews.llvm.org> wrote: > > clayborg requested changes to this revision. > clayborg added a comment. > This revision now requires changes to proceed. > > Very close. Can we try to get UpdateAutomaticSignalFiltering out of > lldb_private::Process as my inline comments suggest? It would be cleaner and > I am not sure we actually need Process::UpdateAutomaticSignalFiltering() for > all processes. > > > > ================ > Comment at: include/lldb/Target/Process.h:3148 > > + virtual Error UpdateAutomaticSignalFiltering(); > + > ---------------- > Can we remove this and only have it in ProcessGDBRemote? Then we just call it > when we need to in ProcessGDBRemote? This seems like something that > ProcessGDBRemote should take care of without having Process knowing about it. > Seems like a adding a call to > ProcessGDBRemote::UpdateAutomaticSignalFiltering() can be done in > ProcessGDBRemote::WillResume() and ProcessGDBRemote::DidLaunch() might do the > trick? It seems like each process plug-in will have different needs regarding > signals.
I disagree. The different processes are at this point more about transport than about the platform details. That indicates to me that it's more likely that different process implementations will have different ways of implementing signal filtering, but the input of which signals will be filtered will be the same. Plus for each process the place where you would implement turning on and changing the filtering are the same. So it makes more sense to have the part that says "go do this filtering" all be in common code, and the particular plugins implement how to do this. Jim > > > ================ > Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.h:401 > + Error UpdateAutomaticSignalFiltering() override; > + > private: > ---------------- > Remove the override if we handle this in UpdateAutomaticSignalFiltering in > specific to ProcessGDBRemote only? > > > https://reviews.llvm.org/D30520 > > > _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
