[Lldb-commits] [PATCH] D61687: Update Python tests for lldb-server on Windows

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.


================
Comment at: packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemoteModuleInfo.py:24-27
+        # Replace path separators in the json string either with "\\\\" or "/" on Windows.
+        triple = self.dbg.GetSelectedPlatform().GetTriple()
+        if re.match(".*-.*-windows", triple):
+            module_path = module_path.replace(os.path.sep, '/')
----------------
Hui wrote:
> clayborg wrote:
> > labath wrote:
> > > It sounds like you could just unconditionally replace all backslashes with double-backslashes here. That would result in us also correctly handling posix paths that happen to contain a backslash.
> > Remove
> The 'jModulesInfo' packet is a json string. I tested with json.loads as follows.
> It seemed to me that module_path needs to be escaped, i.e. 'd:\\\\abc' or be  'd:/abc'.
> 
> not-working case:
> 
> ```
> >>> module_path = 'd:\abc'
> >>> json.dumps(module_path)
> '"d:\\u0007bc"'
> >>> json.loads('[{"[file":"%s"}]' % json.dumps(module_path))
> Traceback (most recent call last):
>   File "<stdin>", line 1, in <module>
>   File "/usr/lib/python2.7/json/__init__.py", line 339, in loads
>     return _default_decoder.decode(s)
>   File "/usr/lib/python2.7/json/decoder.py", line 364, in decode
>     obj, end = self.raw_decode(s, idx=_w(s, 0).end())
>   File "/usr/lib/python2.7/json/decoder.py", line 380, in raw_decode
>     obj, end = self.scan_once(s, idx)
> ValueError: Expecting , delimiter: line 1 column 13 (char 12)
> 
> ```
> working case:
> 
> ```
> >>> module_path = 'd:\\\\abc'
> >>> json.loads('[{"[file":"%s"}]' % module_path)
> [{u'[file': u'd:\\abc'}]
> ```
> 
> 
> 
There are multiple levels of quoting happening here, and I believe you're getting them mixed up:
> >>> module_path = 'd:\abc'
This is already wrong, because python will interpret the \a as the ASCII BEL character, resulting in the string consisting of: 'd', ':', BEL, 'b', 'c'
```
>>> "d:\abc"
'd:\x07bc'
```
The easiest fix is to use "raw" python strings:
```
r"d:\abc"
'd:\\abc'
```
Note that the '\' in the output string it's not doubled, it is just how the python dumper makes sure the output is unambiguous. You can for instance check that with: `len(r"d:\abc")` which returns `6` as expected.

After that, your not-working case works almost fine:
```
>>> json.loads('[{"[file":"%s"}]' % json.dumps(module_path))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib64/python3.6/json/__init__.py", line 354, in loads
    return _default_decoder.decode(s)
  File "/usr/lib64/python3.6/json/decoder.py", line 339, in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
  File "/usr/lib64/python3.6/json/decoder.py", line 355, in raw_decode
    obj, end = self.scan_once(s, idx)
json.decoder.JSONDecodeError: Expecting ',' delimiter: line 1 column 13 (char 12)
>>> json.loads('[{"[file":%s}]' % json.dumps(module_path))
[{'[file': 'd:\\abc'}]
```
Note I removed the superfluous quotes in the second attempt. However, if we're going to be using the json package for this, then there's an even simpler way to write this:
```
>>> json.dumps([{"file": module_path}])
'[{"file": "d:\\\\abc"}]'
>>> json.loads(json.dumps([{"file": module_path}]))
[{'file': 'd:\\abc'}]
```


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D61687