[Lldb-commits] [PATCH] D22629: Rewrite gdb-remote's SendContinuePacketAndWaitForResponse

Greg Clayton via lldb-commits lldb-commits at lists.llvm.org
Thu Jul 28 10:11:55 PDT 2016

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????


More information about the lldb-commits mailing list