[Lldb-commits] [PATCH] D133352: [lldb-server] Report launch error in vRun packets

Wed Sep 7 04:04:03 PDT 2022

mgorny added inline comments.

================
Comment at: lldb/test/API/tools/lldb-server/TestGdbRemoteLaunch.py:18
+        args = [exe_path, "stderr:arg1", "stderr:arg2", "stderr:arg3"]
+        hex_args = [seven.hexlify(x) for x in args]
+
----------------
labath wrote:
> mgorny wrote:
> > labath wrote:
> > > mgorny wrote:
> > > > Since we no longer support Python 2, I'd rather prefer to work towards removing `seven` rather than making more use of it.
> > > Is the problem with the name/location of the function or the functionality (string/byte conversion) itself?
> > > Because, if it's the first, then that could easily be solved by renaming the module (now or later), but in order to avoid elaborate casts we'd have to make all code be byte/string correct. This is not a problem here (because of the fixed strings), but it becomes one once you start working with things that aren't necessarily valid utf8 strings.
> > Ok, I suppose this makes sense given how LLDB's Python API is screwed up :-(.
> Just to clarify, I would like if we used the byte/string separation more (and I started that when I refactored the gdb server test harness), but it's fairly tricky because we're often working with things (e.g. memory contents, or stdout) that can represent arbitrary data in general, but which usually (for our own convenience and sanity) contain simple ASCII strings. So we have a problem where the API (and here I just mean the gdbserver test API, which we can change) really wants us to use bytes, but the the most natural way to write the test is with strings. I haven't come up with a solution to that yet.
Yeah, I know. On the plus side, as long as we're dealing with plain ASCII, using `bytes` is relatively easy and most of the string-like functionality just works. You just have to remember that you want to use byte-strings ;-).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133352