lemo added inline comments.
================
Comment at: source/Interpreter/CommandInterpreter.cpp:2713
+ for (; chunk_size < size; ++chunk_size) {
+ assert(data[chunk_size] != '\0');
+ if (data[chunk_size] == '\n') {
----------------
amccarth wrote:
> Should we be that trusting of a caller? In a non-debug build, if a caller
> doesn't end the (non-empty) string with '\n', this'll just run past the end
> of the buffer. Did I miss something that guarantees the caller won't make a
> mistake? Would it be too expensive to play it safe?
There's no assumption that the string ends with \n, see the loop condition:
chunk_size < size. The assert is just to validate that we don't have embedded
NULs.
================
Comment at: source/Interpreter/CommandInterpreter.cpp:2708-2710
+ const char *data = str.data();
+ size_t size = str.size();
+ while (size > 0 && !WasInterrupted()) {
----------------
clayborg wrote:
> Since we are using "llvm::StringRef" here, why not use its split
> functionality? Something like:
> ```
> bool done = false;
> while (!done) {
> auto pair = str.split('\n');
> auto len = pair.first.size();
> done = pair.second.empty();
> // Include newline if we are not done
> if (!done)
> ++len;
> stream.Write(pair.first.data(),
> }
> ```
>
> This approach also avoids the issue amccarth mentioned below about not ending
> with a newline. It is also quite a bit simpler to follow.
I'll give it a try, thanks for the suggestion.
================
Comment at: source/Interpreter/CommandInterpreter.cpp:2728
+ } else {
+ stream.PutCString(str);
+ }
----------------
clayborg wrote:
> llvm::StringRef can contain NULLs right? Maybe use
>
> ```
> stream.Write(data, size);
> ```
The original code (line 2714) was using PutCString(), so this path is just
preserving the original functionality (which also suggests that the output is
not expected to have embedded nuls)
https://reviews.llvm.org/D37923
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
