ravitheja added inline comments.

================
Comment at: 
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:3293
+      if (custom_params_sp) {
+        if (custom_params_sp->GetType() != 
StructuredData::Type::eTypeDictionary) {
+          error.SetErrorString("Invalid Configuration obtained");
----------------
clayborg wrote:
> Might be simpler to just do:
> 
> ```
> if (!custom_params_sp->GetAsDictionary())
> ```
> 
Yes that could be dangerous to do, coz it returns a pointer to the Dictionary , 
while setTraceParams needs a shared_pointer and I would have a to create a 
shared_pointer which is not really being shared.


================
Comment at: 
unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp:400
+
+  HandlePacket(server, "jTraceStart:{\"buffersize\" : 8192,\"jparams\" : 
{\"psb\" : 1,\"tracetech\" : \"intel-pt\"},\"metabuffersize\" : 
8192,\"threadid\" : 35,\"type\" : 1}", "1");
+  ASSERT_EQ(result.get(),1);
----------------
clayborg wrote:
> Use the R"( so you don't have to desensitize the double quotes and so you can 
> format nicely:
> 
> ```
> const char *json = R"(
> jTraceStart: {
>   "buffersize" : 8192,
>   "metabuffersize" : 8192,
>   "threadid" : 35,
>   "type" : 1,
>   "jparams" : {
>     "psb" : 1,
>     "tracetech" : "intel-pt"
>   }
> })";
> HandlePacket(server, json, "1");
> ```
That won't match coz the new lines and extra spaces cause mismatch in the 
packet string produced. Also unfortunately the Dump Function in the 
StrucuturedData::Dictionary adds spaces around the colons between "Key" : Value 
, so removing all whitespaces won't work. Now I can still add some functions to 
format the expexted string according to the one printed by Dump function, but 
that would be more code.


https://reviews.llvm.org/D32585



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

Reply via email to