clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land.
I am fine with the llvm::StringRef until it causes a crash. So really look at all the ways you construct a llvm::StringRef and make sure it won't assert ever if you leave it as a llvm::StringRef. ================ Comment at: source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp:268 @@ +267,3 @@ + inferior_stdout.append(1, (char)ch); + } + delegate.HandleAsyncStdout(inferior_stdout); ---------------- Actually, feel free to switch over to using GetHexByteString, but maybe just put the reserve part into that function's implementation? Then this code would be just: ``` packet. GetHexByteString(inferior_stdout); ``` ================ Comment at: source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h:31 @@ +30,3 @@ + virtual void + HandleAsyncMisc(llvm::StringRef data) = 0; + virtual void ---------------- I just really don't like that a llvm::StringRef constructed with a null "const char *" will crash LLDB due to an assertion. It has cause crashes in the past when you have the result of a function being fed into a llvm::StringRef. I really want to make a lldb::StringRef that subclasses llvm::StringRef and fixes this as I really don't think it is useful. Why can't an llvm::StringRef be constructed with NULL???? https://reviews.llvm.org/D22629 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
