[Lldb-commits] [PATCH] D45298: [debugserver] Fix LC_BUILD_VERSION load command handling.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I thing the changes are fine. The only part that worries me is the in-class initialization of the simulator variables. I think this will fail on non-apple hosts.



================
Comment at: packages/Python/lldbsuite/test/tools/lldb-server/TestAppleSimulatorOSType.py:16-18
+    sim_devices_str = subprocess.check_output(['xcrun', 'simctl', 'list', '-j',
+                                               'devices'])
+    sim_devices = json.loads(sim_devices_str)['devices']
----------------
I think these will cause a problem as they will be executed even if the test is skipped. Will this throw an exception if "xcrun" fails (e.g. because you're on linux). It might be best to move this into some function..


================
Comment at: packages/Python/lldbsuite/test/tools/lldb-server/TestAppleSimulatorOSType.py:113
+        self.check_simulator_ostype(sdk='watchsimulator',
+                                    platform_names=('watchos',), arch='i386')
----------------
If you make this a list (`['watchos']`), you won't have to add the weird-looking comma at the end.


================
Comment at: packages/Python/lldbsuite/test/tools/lldb-server/main.cpp:50
 static const char *const THREAD_COMMAND_SEGFAULT = "segfault";
+static const char *const THREAD_COMMAND_PRINT_PID = "print-pid";
 
----------------
I don't think this needs to be a thread command, as the pid is not thread-specific. We could just make this a regular command like print-message et al.


https://reviews.llvm.org/D45298