labath added a comment.

I have some questions (but no definitive answers) inline...



================
Comment at: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp:64
+
+  if (out) {
+    lua_pushstring(m_lua_state, "stdout");
----------------
What should be the behavior if this is null? Can it even be null (should we be 
asserting that it isn't)?
It's not clear to me that we should be reusing the previously set stdout if 
these arguments are null... Maybe we should be saving the original stdout value 
and restoring it when we are done? That's what the python interpreter seems to 
be doing..


================
Comment at: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp:65-66
+  if (out) {
+    lua_pushstring(m_lua_state, "stdout");
+    lua_gettable(m_lua_state, -2);
+    luaL_Stream *s =
----------------
`lua_getfield(m_lua_state, -1, "stdout")`


================
Comment at: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp:68
+    luaL_Stream *s =
+        static_cast<luaL_Stream *>(lua_touserdata(m_lua_state, -1));
+    if (!s) {
----------------
I guess it would be better to use `luaL_testudata(m_lua_state, -1, 
LUA_FILEHANDLE)` as that also checks that the user data is of the expected type 
(and e.g. it was not replaced by the user). 

Or it might actually be better to just overwrite the value of `io.stdout/err` 
with a new value/userdata object, instead of fiddling with the existing one.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82273/new/

https://reviews.llvm.org/D82273



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to