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

[Greg Clayton]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

Looks fine, just some possible cleanup with respect to calling "get_signal_number(signal_name, process):" as mentioned in inlined 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)
----------------
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():
```


http://reviews.llvm.org/D11094