[Lldb-commits] [PATCH] D77287: Windows: add very basic support for `DoLoadImage`
Pavel Labath via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Mon Apr 6 02:08:30 PDT 2020
labath added a comment.
In D77287#1960390 <https://reviews.llvm.org/D77287#1960390>, @compnerd wrote:
> I think that the basic test is sufficient for this.
That test does not seem to be exercising the "unload" part of this patch. It would also be nice to run some basic command like "image list" to verify that the loaded binary is indeed listed.
(I don't really have a hard objection to this being a lit test, but it does sound to me like at that point, this will be reimplementing TestLoadUnload.py)
> I think that the path sanitizing and conversion should be a subsequent change.
Why is that? The need for sanitation is a direct consequence of how you've chosen to implement this patch -- the posix version of this function does not do sanitation, but it does not need to, as it does not embed the library name into the compiled expression. I can see how the support for wide strings might be considered a separate feature, but given that supporting that is a matter of adding a single `L` to the compiled expression, I don't see a problem with including that here.
CHANGES SINCE LAST ACTION
More information about the lldb-commits