[Lldb-commits] [PATCH] D114509: [lldb] Introduce PlatformQemuUser

[Pavel Labath via Phabricator via lldb-commits]

labath marked 4 inline comments as done.
labath added inline comments.


================
Comment at: lldb/packages/Python/lldbsuite/test/gdbclientutils.py:403
         return self._connection.sendall(data)
 
+class TCPServerSocket(ServerSocket):
----------------
mgorny wrote:
> It's customary (read: PEP8) to use two empty lines between global-scope stuff like `class`es (and then one line between functions inside the class).
Ok, sounds good.


================
Comment at: lldb/test/API/qemu/TestQemuLaunch.py:31
+            e.write("sys.path = %r\n"%sys.path)
+            e.write("runpy.run_path('%s', run_name='__main__')\n"%
+                    self.getSourcePath("qemu.py"))
----------------
mgorny wrote:
> `%r` will use the `repr()` form, i.e. suitable for direct reuse in Python with the right escaping and so on.
good point. thanks.


================
Comment at: lldb/test/API/qemu/TestQemuLaunch.py:42-43
+        target = self.dbg.CreateTarget(exe, '', 'qemu-user', False, error)
+        self.assertEqual(target.GetPlatform().GetName(), "qemu-user")
+        self.assertSuccess(error)
+        process = target.LaunchSimple(
----------------
mgorny wrote:
> Wouldn't it make sense to reverse these two? i.e. if it fails, then platform is unlikely to be correct, is it?
Yeah, I suppose it does.


================
Comment at: lldb/test/API/qemu/qemu.py:38
+    main()
+
----------------
mgorny wrote:
> Oh no, trailing empty line!
:P


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114509