From ad8b83e44f99ea0970e89912fcbf61f817292453 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?H=C3=A9ctor=20Luaces?= <hector@luaces-novo.es>
Date: Sun, 28 Apr 2024 22:32:39 +0200
Subject: [PATCH] fix: prevent test_load from corrupting object list

When using `test_load` on a loaded object the object list ends up
corrupted. The details are obscure, but if doing this reproduces the
error:

1. Load any object in memory. Check that both `find_object` and
   `children()` return the single existing object
2. Run `test_load` on the object. Check that `find_object` returns `0`
3. Destroy and create the object. `children()` will now return an array
   with two duplicates of the same object or, sometimes, a `0` and the
   existing copy of the object

In some instances i've seen different objects returned by `children()`
than the one being passed as path (this usually happened with clones).

This PR tries to work around it. I've also reverted #843 because I
believe this PR fixes the underlying cause for the issue, not just the
symptom.

All credit goes to Zoilder (one of the developers on our mud) that
happened to fix this a long time ago. We're just sending these fixes
upstream since we're starting to update our outdated driver.

Please let me know if there's anything I should fix. I've tried to add a
proper test for this, but I don't know if that's the location you're
expecting for it.
---
 src/packages/contrib/contrib.cc           |  3 +++
 testsuite/single/tests/crasher/1063-aux.c |  2 ++
 testsuite/single/tests/crasher/1063.c     | 18 ++++++++++++++++++
 3 files changed, 23 insertions(+)
 create mode 100644 testsuite/single/tests/crasher/1063-aux.c
 create mode 100644 testsuite/single/tests/crasher/1063.c

diff --git a/src/packages/contrib/contrib.cc b/src/packages/contrib/contrib.cc
index 0bf76919d..4f8696185 100644
--- a/src/packages/contrib/contrib.cc
+++ b/src/packages/contrib/contrib.cc
@@ -3039,6 +3039,7 @@ object_t *testloadob;
 static void fix_object_names() {
   if (testloadob) {
     SETOBNAME(testloadob, saved_extra_name);
+    ObjectTable::instance().insert(testloadob->obname,testloadob);
   }
 }
 
@@ -3047,6 +3048,7 @@ void f_test_load() {
   object_t *new_ob;
   if ((testloadob = find_object2(sp->u.string))) {
     tmp = testloadob->obname;
+    ObjectTable::instance().remove(testloadob->obname);
     SETOBNAME(testloadob, "");
   }
   saved_extra_name = tmp;
@@ -3070,6 +3072,7 @@ void f_test_load() {
   push_number(1);
   if (testloadob) {
     SETOBNAME(testloadob, saved_extra_name);
+    ObjectTable::instance().insert(testloadob->obname,testloadob);
   }
 }
 #endif
diff --git a/testsuite/single/tests/crasher/1063-aux.c b/testsuite/single/tests/crasher/1063-aux.c
new file mode 100644
index 000000000..88fa30aa0
--- /dev/null
+++ b/testsuite/single/tests/crasher/1063-aux.c
@@ -0,0 +1,2 @@
+void create() {
+}
diff --git a/testsuite/single/tests/crasher/1063.c b/testsuite/single/tests/crasher/1063.c
new file mode 100644
index 000000000..ac1dd7887
--- /dev/null
+++ b/testsuite/single/tests/crasher/1063.c
@@ -0,0 +1,18 @@
+void do_tests() {
+  object ob, *all;
+  string path;
+
+  path = base_name() + "-aux.c";
+  ob = load_object(path);
+  all = children(path);
+  ASSERT_EQ(sizeof(all), 1);
+
+  test_load(path);
+  all = children(path);
+  ASSERT_EQ(sizeof(all), 1);
+
+  destruct(ob);
+  all = children(path);
+  ASSERT_EQ(sizeof(all), 1);
+  ASSERT_EQ(member_array(0, all), -1);
+}