JDevlieghere added inline comments.
================ Comment at: lldb/source/Plugins/Process/mach-core/ProcessMachCore.h:89 void CreateMemoryRegions(); - void LoadBinariesViaMetadata(); + bool LoadBinariesViaMetadata(); void LoadBinariesViaExhaustiveSearch(); ---------------- bulbazord wrote: > Since `LoadBinariesViaMetadata` can fail and we now care about the return > value, what do you think about the name `TryLoadBinariesViaMetadata`? I think > a name that indicates that it may fail to actually load would make it easier > to read at a glance. The same is true for the other methods though, so I don't know if that is really all that meaningful. IIUC, before this patch, all these methods conveyed whether they were successful by setting `m_dyld_plugin_name`. Now, `LoadBinariesViaMetadata` does something extra, which is conveyed by the boolean. I dislike the inconsistency, but I can't really think of a better way of doing this, I considered suggesting an out-parameter but that doesn't really improve anything. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157168/new/ https://reviews.llvm.org/D157168 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
