Repair buggy error-handling in makeclass/getclass

luprex/core/cpp/luastack.cpp

@@ -188,66 +188,142 @@ lua_State *LuaStack::newthread(LuaSlot target) const {
     return result;
 }
 
-void LuaStack::getclass(LuaSlot classtab, LuaSlot classname) const {
+bool LuaStack::validclassname(const std::string &cname) {
+    if (cname.empty()) return false;
+    if (cname == "_G") return false;
+    return true;
+}
+
+bool LuaStack::validclassname(LuaSlot slot) const {
+    if (!isstring(slot)) return false;
+    return validclassname(ckstring(slot));
+}
+
+std::string LuaStack::classname(LuaSlot tab) const {
+    std::string result;
+    if (istable(tab)) {
+        lua_pushstring(L_, "__class");
+        lua_rawget(L_, tab);
+        if (lua_type(L_, -1) == LUA_TSTRING) {
+            lua_pushglobaltable(L_); // cname table
+            lua_pushvalue(L_, -2); // cname table cname
+            lua_rawget(L_, -2); // cname table ctab
+            if (lua_rawequal(L_, -1, tab)) {
+                size_t len;
+                const char *s = lua_tolstring(L_, -3, &len);
+                result = std::string(s, len);
+                if (!validclassname(result)) {
+                    result = "";
+                }
+            }
+            lua_pop(L_, 3);
+        } else {
+            lua_pop(L_, 1);
+        }
+    }
+    return result;
+}
+
+std::string LuaStack::getclass(LuaSlot classtab, LuaSlot classname) const {
     lua_checkstack(L_, 20);
     LuaVar globtab, cname;
     LuaStack LS(L_, globtab, cname);
-
-    if (!LS.isstring(classname)) {
-        luaL_error(L_, "Not a class name: %s", lua_tostring(L_, classname));
-    }
-
-    // Convert class name to a table.
-    if (LS.rawequal(classname, "_G")) {
-        luaL_error(L_, "_G is explicitly not allowed as a class name");
-    }
     LS.getglobaltable(globtab);
-    LS.rawget(classtab, globtab, classname);
-    if (!LS.istable(classtab)) {
-        luaL_error(L_, "Not a class: %s", lua_tostring(L_, classname));
-    }
-    LS.rawget(cname, classtab, "__class");
-    if (!LS.rawequal(cname, classname)) {
-        luaL_error(L_, "Not a class: %s", lua_tostring(L_, classname));
-    }
 
-    // OK, we're done.
+    if (LS.isstring(classname)) {
-    LS.result();
+        if (!validclassname(LS.ckstring(classname))) {
+            std::string err = "Not allowed as a class name: " + LS.ckstring(classname);
+            LS.result();
+            return err;
+        }
+        LS.rawget(classtab, globtab, classname);
+        if (!LS.istable(classtab)) {
+            std::string err = "Not a class: " + LS.ckstring(classname);
+            LS.result();
+            return err;
+        }
+        LS.rawget(cname, classtab, "__class");
+        if (!LS.rawequal(cname, classname)) {
+            std::string err = "Not a valid class: " + LS.ckstring(classname);
+            LS.result();
+            return err;
+        }
+        LS.result();
+        return "";
+    } else if (LS.istable(classname)) {
+        LS.rawget(cname, classname, "__class");
+        if (!LS.isstring(cname)) {
+            std::string err = "Table is not a class.";
+            LS.result();
+            return err;
+        }
+        if (!validclassname(LS.ckstring(cname))) {
+            std::string err = "Not allowed as a class name: " + LS.ckstring(cname);
+            LS.result();
+            return err;
+        }
+        LS.rawget(classtab, globtab, cname);
+        if (!LS.rawequal(classtab, classname)) {
+            std::string err = "Not a valid class: " + LS.ckstring(cname);
+            LS.result();
+            return err;
+        }
+        LS.result();
+        return "";
+    } else {
+        std::string err = "getclass expects a string or a classtab";
+        LS.result();
+        return err;
+    }
+}
+
+std::string LuaStack::getclass(LuaSlot tab, const char *name) const {
+    push_any_value(name);
+    LuaSpecial classname(lua_gettop(L_));
+    std::string err = getclass(tab, classname);
+    lua_pop(L_, 1);
+    return err;
+}
+
+std::string LuaStack::getclass(LuaSlot tab, const std::string &name) const {
+    push_any_value(name);
+    LuaSpecial classname(lua_gettop(L_));
+    std::string err = getclass(tab, classname);
+    lua_pop(L_, 1);
+    return err;
 }
 
 void LuaStack::makeclass(LuaSlot classtab, LuaSlot classname) const {
     lua_checkstack(L_, 20);
-    LuaVar globtab;
+    LuaVar globtab, cname;
-    LuaStack LS(L_, globtab);
+    LuaStack LS(L_, globtab, cname);
 
     // Validate the class name.
-    if (!LS.isstring(classname)) {
+    assert(LS.validclassname(classname));
-        luaL_error(L_, "Not a class name: %s", lua_tostring(L_, classname));
+    
-    }
-    if (LS.rawequal(classname, "_G")) {
-        luaL_error(L_, "_G is explicitly not allowed as a class name");
-    }
-
     // Fetch the global environment from the registry.
     LS.getglobaltable(globtab);
     
     // Get the classtab from the global environment.
-    // Create it if it doesn't exist.
     LS.rawget(classtab, globtab, classname);
-    if (LS.isnil(classtab)) {
+
+    // Make sure we're not reusing a table that isn't a class.
+    if (LS.istable(classtab)) {
+        LS.rawget(cname, classtab, "__class");
+    } else {
+        LS.set(cname, LuaNil);
+    }
+
+    // Make a new table if necessary.
+    if (!LS.rawequal(cname, classname)) {
         LS.newtable(classtab);
         LS.rawset(globtab, classname, classtab);
+        LS.rawset(classtab, "__class", classname);
     }
-
+    
-    // If the name isn't bound to a table, abort.
-    if (!LS.istable(classtab)) {
-        luaL_error(L_, "%s is not a class", ckstring(classname).c_str());
-    }
-
     // Repair the special fields.
     LS.settabletype(classtab, LUA_TT_CLASS);
     LS.rawset(classtab, "__index", classtab);
-    LS.rawset(classtab, "__class", classname);
 
     // Put the stack back.
     LS.result();
@@ -267,27 +343,6 @@ void LuaStack::makeclass(LuaSlot tab, const std::string &name) const {
     lua_pop(L_, 1);
 }
 
-std::string LuaStack::classname(LuaSlot tab) const {
-    std::string result;
-    if (istable(tab)) {
-        lua_pushstring(L_, "__class");
-        lua_rawget(L_, tab);
-        if (lua_type(L_, -1) == LUA_TSTRING) {
-            lua_pushglobaltable(L_); // cname table
-            lua_pushvalue(L_, -2); // cname table cname
-            lua_rawget(L_, -2); // cname table ctab
-            if (lua_rawequal(L_, -1, tab)) {
-                size_t len;
-                const char *s = lua_tolstring(L_, -3, &len);
-                result = std::string(s, len);
-            }
-            lua_pop(L_, 3);
-        } else {
-            lua_pop(L_, 1);
-        }
-    }
-    return result;
-}
 
 int64_t LuaStack::tanid(LuaSlot tab) const {
     int64_t result = 0;

luprex/core/cpp/luastack.hpp

@@ -386,17 +386,35 @@ public:
     
     int next(LuaSlot tab, LuaSlot key, LuaSlot value) const;
 
-    void getclass(LuaSlot tab, LuaSlot name) const;
+    // Return true if the classname is legal.
+    bool validclassname(LuaSlot value) const;
+    static bool validclassname(const std::string &cname);
+
+    // Return the class name if x is a valid classtab.
+    // Otherwise, returns empty string.  If nonempty, the
+    // result is guaranteed to be a validclassname.
+    // This can also function as an "isclass" operator.
+    std::string classname(LuaSlot x) const;
+
+    // Look up a class.
+    // If there is a problem, returns an error message.
+    // There are lots of error conditions, including such things
+    // as no such class, corrupted class, classname invalid, etc.
+    std::string getclass(LuaSlot tab, LuaSlot name) const;
+    std::string getclass(LuaSlot tab, const char *name) const;
+    std::string getclass(LuaSlot tab, const std::string &name) const;
+
+    // Create a class, or look up an existing class.
+    // WARNING: this routine assert-fails if the parameter is not
+    // a valid classname.  Check the classname before calling this!
     void makeclass(LuaSlot tab, LuaSlot name) const;
     void makeclass(LuaSlot tab, const char *name) const;
     void makeclass(LuaSlot tab, const std::string &name) const;
-    std::string classname(LuaSlot tab) const;
 
+    // Get the ID of a tangible.  It's a little weird to put this in
+    // this module.
     int64_t tanid(LuaSlot tab) const;
 
-    // There's no 'isclass' operator, but 'classname' will return empty
-    // string if tab is not a valid class.
-
     void movesortablekey(LuaSlot val, LuaStack &other, LuaSlot otherslot);
 
     bool rawequal(LuaSlot v1, LuaSlot v2) const {

luprex/core/cpp/source.cpp

@@ -19,10 +19,27 @@ LuaDefine(makeclass, "classname", "create a class if it doesn't already exist")
     LuaArg classname;
     LuaRet classtab;
     LuaStack LS(L, classname, classtab);
+    if (!LS.isstring(classname)) {
+        luaL_error(L, "class name must be a string");
+    }
+    if (!LS.validclassname(classname)) {
+        luaL_error(L, "invalid class name: %s", LS.ckstring(classname).c_str());
+    };
     LS.makeclass(classtab, classname);
     return LS.result();
 }
 
+LuaDefine(getclass, "classname", "get the classtab with the specified name") {
+    LuaArg classname;
+    LuaRet classtab;
+    LuaStack LS(L, classname, classtab);
+    std::string err = LS.getclass(classtab, classname);
+    if (err != "") {
+        luaL_error(L, "%s", err.c_str());
+    }
+    return LS.result();
+}
+
 LuaDefine(classname, "classtable", "get the class name from a class table") {
     LuaArg table;
     LuaRet result;
@@ -41,6 +58,12 @@ LuaDefine(maketangible, "classname", "create a class if it doesn't already exist
     LuaRet classtab;
     LuaVar subtab;
     LuaStack LS(L, classname, classtab, subtab);
+    if (!LS.isstring(classname)) {
+        luaL_error(L, "class name must be a string");
+    }
+    if (!LS.validclassname(classname)) {
+        luaL_error(L, "invalid class name: %s", LS.ckstring(classname).c_str());
+    };
     LS.makeclass(classtab, classname);
     LS.makesubtable(subtab, classtab, "action");
     return LS.result();

luprex/core/cpp/table.cpp

@@ -229,6 +229,10 @@ LuaDefine(deque_create, "", "create a deque") {
     LuaVar classobj;
     LuaStack LS(L, rdeque, classobj);
     const int imax = 4;
+    std::string err = LS.getclass(classobj, "deque");
+    if (err != "") {
+        luaL_error(L, "Class deque has been corrupted");
+    }
     LS.createtable(rdeque, DEQUE_BASE + imax - 1, 0);
     LS.rawset(rdeque, DEQUE_LEFT, 0);
     LS.rawset(rdeque, DEQUE_FILL, 0);
@@ -236,7 +240,6 @@ LuaDefine(deque_create, "", "create a deque") {
     for (int i = 0; i < imax; i++) {
         LS.rawset(rdeque, DEQUE_BASE + i, 0);
     }
-    LS.makeclass(classobj, "deque");
     LS.setmetatable(rdeque, classobj);
     return LS.result();
 }

luprex/core/cpp/world-accessor.cpp

@@ -80,10 +80,9 @@ LuaDefine(tangible_setclass, "tan,class",
     LuaStack LS(L, tanobj, classname, classtab, mt);
     World *w = World::fetch_global_pointer(L);
     w->tangible_get(LS, tanobj);
-    if (LS.classname(classname) != "") {
+    std::string err = LS.getclass(classtab, classname);
-        LS.set(classtab, classname);
+    if (err != "") {
-    } else {
+        luaL_error(L, "%s", err.c_str());
-        LS.getclass(classtab, classname);
     }
     LS.getmetatable(mt, tanobj);
     LS.rawset(mt, "__index", classtab);
@@ -140,13 +139,9 @@ LuaDefine(tangible_build, "configtable",
     LS.checktable(config);
     // Get the class of the new tangible.
     LS.rawget(classname, config, "class");
-
+    std::string err = LS.getclass(classtab, classname);
-    if (LS.isnil(classname)) {
+    if (err != "") {
-        luaL_error(L, "must specify a class name");
+        luaL_error(L, "%s", err.c_str());
-    } else if (LS.classname(classname) != "") {
-        LS.set(classtab, classname);
-    } else {
-        LS.getclass(classtab, classname);
     }
 
     // Parse the initial animation step.