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

Sorry for the delay on this! Feel free to ping sooner if this gets stuck again.

I think just passing a string here seems fine, but maybe we could let the 
function just take a C-string and call it `SetInputString` (Both in SBDebugger 
and Debugger)? The rest of the code anyway doesn't handle 0 bytes so we might 
as well just use a C-string for that function and avoid the whole string length 
thing (that just seems awkward in Python scripts).

Beside that this seems fine. Some documentation for the SB API part would be 
appreciated but I won't insist on it.



================
Comment at: lldb/include/lldb/Core/Debugger.h:181
+
+  Status SetInputFile(lldb::FileSP file);
+
----------------
Please add a note that this automatically sets up a DataRecorder depending on 
the current reproducer mode.


================
Comment at: lldb/source/Core/Debugger.cpp:833
+
+  ssize_t nrwr = write(fds[WRITE], data, size);
+  if (size_t(nrwr) != size) {
----------------
I think I could have added more info on my previous inline comment: 
`write(PIPE, ..., 0)` is unspecified so that's why I asked for that early exit 
on the previous version with SBStream. So now just early-exiting when data is 
null or size is 0 would workaround that.


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

https://reviews.llvm.org/D104413

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to