[Lldb-commits] [PATCH] D15152: Add a new option to Platform::LoadImage to install the image

Tamas Berghammer via lldb-commits lldb-commits at lists.llvm.org
Mon Dec 7 04:35:00 PST 2015

tberghammer added inline comments.

Comment at: include/lldb/API/SBProcess.h:301-307
@@ +300,9 @@
+    // Load an image to the currently running process.
+    // If both local and remote image is specified then copy the local image to the remote image and
+    // then load it from thew remote path.
+    // If only the local image is specified then the image will be copied to the current working
+    // directory and opened from there.
+    // If only the remote image is specified then open the image from the target (no compying done
+    // in this case)
+    uint32_t
clayborg wrote:
> Add correct header doc for this if we are going to add documentation to the header file.

Comment at: include/lldb/Target/Platform.h:1023-1027
@@ -1013,6 +1022,7 @@
-        virtual uint32_t
+        uint32_t
         LoadImage (lldb_private::Process* process,
-                   const lldb_private::FileSpec& image_spec,
+                   const lldb_private::FileSpec& local_file,
+                   const lldb_private::FileSpec& remote_file,
                    lldb_private::Error& error);
clayborg wrote:
> This can still be virtual just in case other platforms want to do something completely different that what is supported in the default implementation.
Currently I don't see any valid reason to override the implementation of this function as it still have to respect the API specification. For now I would prefer to leave it non-virtual and we can change it in the future if we need to.


More information about the lldb-commits mailing list