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

[Tamas Berghammer via lldb-commits]

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.
Done

================
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.


http://reviews.llvm.org/D15152