[Lldb-commits] [PATCH] D22629: Rewrite gdb-remote's SendContinuePacketAndWaitForResponse
Pavel Labath via lldb-commits
lldb-commits at lists.llvm.org
Fri Jul 29 04:08:40 PDT 2016
labath added a comment.
Comment at: source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp:269
@@ +268,3 @@
Will do. At that point this whole function becomes pretty irrelevant and i will inline it back into the caller.
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????
I understand your concerns, but i also see the other side's point, who consider a null pointer an invalid input and the responsibility of the user to deal with that.
That said, we as a user have a lot of places which treat null as an empty string, so we need to do something about that. Instead of a new (sub-)class, I'd go for making a factory function instead (`lldb::makeStringRef()`?) and say we use that whereever we are not sure about the nullness of the pointer. The advantage of that being you don't have to worry about type conversions when dealing with different StringRefs (and once our StringRef is constructed, it works just like the vanilla object). In time, the need for that function should even diminish, as once we are already using a StringRef, we don't have to worry about the whole null pointer business.
BTW, constructing a std::string from a null pointer is also undefined behaviour, and implementations I've seen either assert or crash when trying do to a strlen().
More information about the lldb-commits