[Lldb-commits] [PATCH] D11094: Refactor Unix signals.

[Chaoren Lin]

chaoren added inline comments.

================
Comment at: test/lldbutil.py:935-939
@@ -934,33 +934,7 @@
 
 def get_signal_number(signal_name):
     platform = lldb.remote_platform
-    if platform:
-        if platform.GetName() == 'remote-linux':
-            command = lldb.SBPlatformShellCommand('kill -l %d' % signal_name)
-            if platform.Run(command).Success() and command.GetStatus() == 0:
-                try:
-                    return int(command.GetOutput())
-                except ValueError:
-                    pass
-        elif platform.GetName() == 'remote-android':
-            for signal_number in range(1, 65):
-                command = lldb.SBPlatformShellCommand('kill -l %d' % signal_number)
-                if platform.Run(command).Fail() or command.GetStatus() != 0:
-                    continue
-                output = command.GetOutput().strip().upper()
-                if not output.startswith('SIG'):
-                    output = 'SIG' + output
-                if output == signal_name:
-                    return signal_number
-    if lldb.debugger:
-        for target_index in range(lldb.debugger.GetNumTargets()):
-            target = lldb.debugger.GetTargetAtIndex(target_index)
-            if not target.IsValid():
-                continue
-            process = target.GetProcess()
-            if not process.IsValid():
-                continue
-            signals = process.GetUnixSignals()
-            if not signals.IsValid():
-                continue
+    if platform and platform.IsValid():
+        signals = platform.GetUnixSignals()
+        if signals.IsValid():
             signal_number = signals.GetSignalNumberFromName(signal_name)
----------------
clayborg wrote:
> We should be getting this info from the current process if we can. This seems like it can introduce test suite errors if our const cached notion of what signals are that are retrieved from the static UnixSignals function doesn't actually match the real current process that might have fetched the info from the remote GDB server. Can you check on all of the callsites that use this get_signal_number() function and see if any have a live process they can supply?
> 
> I would like to see this function changed to:
> 
> ```
> def get_signal_number(signal_name, process):
>     signals = None
>     if process:
>        signals = process.GetUnixSignals()
>     else:
>         platform = lldb.remote_platform
>         if platform and platform.IsValid():
>             signals = platform.GetUnixSignals()
>     if signals and signals.IsValid():
> ```
I don't think it's possible for the signals to be different between the platform and a process, since I followed your advice and each process gets a //copy// of the signals that the platform fetched remotely.


http://reviews.llvm.org/D11094