Aj0SK marked 9 inline comments as done. Aj0SK added a comment. Thanks for the review! Some requested changes need to be clarified for me. I have a problem mainly with successfully registering the Plugin when it inherits only from PluginInterface.
================ Comment at: lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp:30-31 + GetPluginNameStatic(), GetPluginDescriptionStatic(), CreateInstance, + /*create_memory_callback=*/nullptr, + /*get_module_specifications=*/nullptr, SaveCore); +} ---------------- clayborg wrote: > We have two options here: > 1 - if we want to pass NULL for any callbacks, then we need to fix all > callsites to that use > PluginManager::GetObjectFileCreateMemoryCallbackAtIndex(...) and > PluginManager::GetObjectFileGetModuleSpecificationsCallbackAtIndex(...) to > have this done inside of new plug-in manager > PluginManager::CreateMemoryCallbackAtIndex(...) and > PluginManager::GetModuleSpecifications(...) where these new functions iterate > over all plug-ins and skip any plug-ins that have NULL callbacks. > 2 - add valid create_memory_callback and get_module_specifications callbacks > that return invalid values > > Otherwise this plug-in might stop the iteration early for existing callers of > PluginManager::GetObjectFileCreateMemoryCallbackAtIndex(...) and > PluginManager::GetObjectFileGetModuleSpecificationsCallbackAtIndex(...). I would prefer modifying code outside of this functionality as little as possible so I would stick to the second option. However, maybe returning the invalid values is also not the best solution. Also, does this option works with ObjectFileMinidump inheriting from PluginInterface? I think that I am unable to use the same PluginManager::RegisterPlugin method, so not sure how this plugin gets to ObjectFilesInstances... Maybe I am missing out something. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108233/new/ https://reviews.llvm.org/D108233 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
