compnerd added a comment. In D77287#1963242 <https://reviews.llvm.org/D77287#1963242>, @labath wrote:
> 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. Actually, it changes the APIs used and the path that this goes down on the Windows side, so it has a much broader impact than it appears. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77287/new/ https://reviews.llvm.org/D77287 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
