[Lldb-commits] [PATCH] D51319: Use a RAII guard to lock/unlock DisassemblerLLVMC [NFC]

Vedant Kumar via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Mon Aug 27 13:51:24 PDT 2018


vsk added inline comments.


================
Comment at: source/Plugins/Disassembler/llvm/DisassemblerLLVMC.h:81
+  struct Guard {
+    DisassemblerLLVMC *m_instance;
+    Guard(DisassemblerLLVMC *instance, InstructionLLVMC *inst,
----------------
This is nice. Do you think it might be even safer to have the guard own the disassembler shared_ptr instance? Users could then access the disassembler via the guard's operator->, and there's less chance of the shared_ptr escaping / being abused. We could even have GetDisassembler() return a guard instance.


================
Comment at: source/Plugins/Disassembler/llvm/DisassemblerLLVMC.h:85
+        : m_instance(instance) {
+      m_instance->Lock(inst, exe_ctx);
+    }
----------------
It doesn't make sense to me that DisassemblerLLVMC's "Lock" method conflates locking and initialization. If you decide to have a guard own the disassembler instance, it'd make sense to split the initialization step out.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D51319





More information about the lldb-commits mailing list