[Lldb-commits] [PATCH] D99890: [lldb] Fix bug where memory read --outfile is not truncating the file

[Raphael Isemann via Phabricator via lldb-commits]

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

Thanks for the patch! I'll be extra nit-picky about the test to make up for the great meme steal of 2021.



================
Comment at: lldb/test/API/functionalities/memory/read/TestMemoryRead.py:146
+        self.build_run_stop()
+        cmd = "memory read -f d -c 1 `&argc`"
+        res = lldb.SBCommandReturnObject()
----------------
Single use var (I guess you can't reuse that as a common prefix as the address expression has to be the last arg).


================
Comment at: lldb/test/API/functionalities/memory/read/TestMemoryRead.py:149
+        self.ci.HandleCommand(cmd, res)
+        self.assertTrue(res.Succeeded())
+
----------------
`self.assertTrue(res.Succeeded(), "memory read failed:" + res.GetError())`


================
Comment at: lldb/test/API/functionalities/memory/read/TestMemoryRead.py:154
+
+        temp_file = tempfile.NamedTemporaryFile().name
+
----------------
I believe usually we use `self.getBuildArtifact("memory-read-output")` for that. If you open a named temp file here (and don't close it), then the second open can fail on Windows.


================
Comment at: lldb/test/API/functionalities/memory/read/TestMemoryRead.py:166
+                        format(i, expected_line, actual_line))
+                self.assertEqual(len(expected), len(lines))
+
----------------
Can we ever hit this? We just get an exception in the for loop above (and the exception won't be very useful to diagnose the failure).

You can do this:
```
lang=python
lines = [s.strip() for s in lines]
expected = [s.strip() for s in expected]
self.assertEqual(lines, expected)
```

(assertEqual actually explains which elements are missing/additional/different for lists)


================
Comment at: lldb/test/API/functionalities/memory/read/TestMemoryRead.py:173
+        # Make sure the file is truncated when we run the command again.
+        self.runCmd("memory read -f d -c 1 -o '{}' `&argc`".format(temp_file))
+        check_file_content([golden_output])
----------------
Could you write some garbage into the file between the two tests here?


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

https://reviews.llvm.org/D99890