labath added a comment.

Looks like a good starting point. Apart from some style nits, I'd like us to 
remove sending of the `registers` field in the `jstopinfo`, as it is not 
necessary anymore. Also, we should add an lldb-server style test for this.

@jasonmolenda, we already have jThreadsInfo support in lldb-server, albeit 
without the memory snippet parts. A lot less functions use frame pointers on 
linux than on osx, so I was not sure whether implementing the simple 
FP-following will be useful. However, that is the next thing on my mind in 
terms of optimizations here.

Comment at: 
+static void WriteRegisterValueInHexFixedWidth(
+    StreamString &response, NativeRegisterContextSP &reg_ctx_sp,
This function is called in only two sites. My preference would be to update 
them to use the new signature instead of adding another overload.

Comment at: 
     if (JSONObject::SP registers_sp = GetRegistersAsJSON(*thread_sp, abridged))
       thread_obj_sp->SetObject("registers", registers_sp);
We can now change this code to:
if (!abridged) {
  if (JSONObject::SP registers_sp = GetRegistersAsJSON(*thread_sp))

This was my attempt to send the PCs in the `jstopinfo` field, which used to 
work at some point, but now lldb client does not seem to recognize it. When we 
start sending the PC in the `thread-pcs` field, the whole `registers` field 
will become completely redundant, and we should remove it.

This will not affect the `jThreadsInfo` packet, as there we pass `abridged = 

(Obviously we should also clean up the `GetRegistersAsJSON` function to reflect 
the fact it does not need to support PC-only register sending.)

Comment at: 
     thread_obj_sp->SetObject("tid", std::make_shared<JSONNumber>(tid));
Please remove the random whitespace.

lldb-commits mailing list

Reply via email to