[Lldb-commits] [PATCH] D153735: [lldb][TargetGetModuleCallback] Implement Python interface

Mon Jul 10 16:09:25 PDT 2023

clayborg added inline comments.

================
Comment at: lldb/include/lldb/API/SBDefines.h:129

+typedef SBError (*SBTargetGetModuleCallback)(SBDebugger debugger,
+                                             SBModuleSpec &module_spec,
----------------
If we are putting this into SBPlatform, this should probably be named "SBPlatformGetModuleCallback". 

The name might be a bit more clear if it was "SBPlatformLocateModuleCallback"? Open to suggestions or fine to lave this as "SBPlatformGetModuleCallback" if everyone likes that.

================
Comment at: lldb/include/lldb/API/SBPlatform.h:181
+  /// nullptr or None is set.
+  SBError SetTargetGetModuleCallback(lldb::SBTargetGetModuleCallback callback,
+                                     void *callback_baton);
----------------
remove "Target" from the function name? Or rename to "SetLocateModuleCallback(...)" if we end up renaming the callback type to SBPlatformLocateModuleCallback

================
Comment at: lldb/source/API/SBPlatform.cpp:668
+    // Platform. 'callback_baton' is the actual callback Python callable object.
+    platform_sp->SetTargetGetModuleCallback(callback_baton);
+    return SBError();
----------------
You are not passing the "callback" into Platform::SetTargetGetModuleCallback??

Are we missing changes to the file "lldb/include/lldb/Target/Platform.h"? 

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153735