labath added a comment.

Thanks for splitting these up. It makes it much more obvious what is going on.

I see now that you're making use of this SetFile method which I wanted to 
remove in the previous patch. I kind of like that, because it pushes the `Set` 
one level up (so it now happens on the Stream instead of the File), but I do 
wonder if we shouldn't remove it completely. It seems like we could have 
Debugger::SetXXXFileHandle just create a fresh stream instead of twiddling with 
the existing one. This has the potential to change behavior as this change will 
not propagate to anybody who has fetched the stream_sp previously. However, I'm 
not sure if this is a change we care about because if anybody was writing to a 
stream while we were in the process of changing it, then he would likely 
trigger a race condition anyway. This way, we risk some code not immediately 
noticing the stream change, but at least we're thread safe. @jingham, what do 
you think about that?

Other than that, I am pretty happy with how this looks, and I think you've 
convinced me that the shared pointer inside the StreamFile object is really 
needed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67891/new/

https://reviews.llvm.org/D67891



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

Reply via email to