labath added a comment.
Sorry for the delay, I was OOO and I'm still processing the backlog...
================
Comment at: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp:39
+
+ ConstString module_name = file.GetFileNameStrippingExtension();
+ if (!module_name) {
----------------
I guess you also ought to validate the extension here. Otherwise "import
foo.perl" will end up importing "foo.lua".
================
Comment at: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp:49
+ if (llvm::Error e =
+ Run(llvm::formatv("package.path = package.path .. \";{0}\"",
+ file.GetCString())
----------------
Embedding the file name this way seems unfortunate (`"; os.execute("rm -rf
/");`), and it probably will not work at all on windows because of all the
backslashes. Lua formatting function has a `%q` format to format strings
safely. Accessing that from C is a bit tricky, but we can make a utility
function for that.
================
Comment at: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp:58
+ // Reload the module if requested.
+ if (llvm::Error e = Run(
+ llvm::formatv("package.loaded.{0} = nil", module_name.GetStringRef())
----------------
Here we probably ought to validate that `module_name` is a valid lua identifier.
================
Comment at: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp:66
+ return Run(
+ llvm::formatv("{0} = require \"{0}\"",
module_name.GetStringRef()).str());
+}
----------------
Is there any advantage to using `require` here? It doesn't seems like it brings
much into the game, as its main purpose seems to be to _find_ the module, but
here we know exactly where the module is. OTOH, using it has a lot of downsides
(needing to quote/validate all strings, mess with the `package.loaded`
variable, potentially loading the wrong file, etc).
Would it be possible to just `luaL_loadfile` the file and then assign the
result of its evaluation to the appropriate variable manually?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71825/new/
https://reviews.llvm.org/D71825
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
