edit: Report unit key 'U' failure more nicely, and avoid oops

Two failure modes:

* When the argument is negative, edit() attempts an invalid write,
  triggering an oops.  edit() detects the write failure, and returns
  RET_FAIL.

  Detect the negative argument instead, and return RET_SYN, like we do
  for other invalid edit arguments.

* When the argument requires the table to be extended, but the
  extension fails, edit() returns RET_FAIL.

  Additionally report the failure to the player.

Signed-off-by: Markus Armbruster <armbru@pond.sub.org>

diff --git a/src/lib/commands/edit.c b/src/lib/commands/edit.c
index 28877ad7e58e4703efda557c85a277cb2e6288dc..7f5ed1a1393638daa57731259ac1b673fe27cd27 100644 (file)
--- a/src/lib/commands/edit.c
+++ b/src/lib/commands/edit.c
@@ -193,8 +193,6 @@ edit(void)
                return RET_FAIL;
            if ((err = edit_ship(&ship, key, ptr)) != RET_OK)
                return err;
-           if (!ef_ensure_space(EF_SHIP, ship.shp_uid, 50))
-               return RET_FAIL;
            if (!putship(ship.shp_uid, &ship))
                return RET_FAIL;
            break;
@@ -203,8 +201,6 @@ edit(void)
                return RET_FAIL;
            if ((err = edit_land(&land, key, ptr)) != RET_OK)
                return err;
-           if (!ef_ensure_space(EF_LAND, land.lnd_uid, 50))
-               return RET_FAIL;
            if (!putland(land.lnd_uid, &land))
                return RET_FAIL;
            break;
@@ -213,8 +209,6 @@ edit(void)
                return RET_FAIL;
            if ((err = edit_plane(&plane, key, ptr)) != RET_OK)
                return err;
-           if (!ef_ensure_space(EF_PLANE, plane.pln_uid, 50))
-               return RET_FAIL;
            if (!putplane(plane.pln_uid, &plane))
                return RET_FAIL;
            break;
@@ -749,6 +743,12 @@ edit_unit(struct empobj *unit, char *key, char *p,
  
     switch (toupper(*key)) {
     case 'U':
+       if (arg < 0)
+           return RET_SYN;
+       if (!ef_ensure_space(unit->ef_type, arg, 50)) {
+           pr("Can't copy to %s #%d\n", ef_nameof(unit->ef_type), arg);
+           return RET_FAIL;
+       }
        ef_set_uid(unit->ef_type, unit, arg);
        break;
     case 'O':

diff --git a/tests/actofgod/journal.log b/tests/actofgod/journal.log
index e88857947a5bcfe5ec377ff553754cdb12dda0b7..c80a004bb2062043be10b3fd3b6051e7bca73286 100644 (file)
--- a/tests/actofgod/journal.log
+++ b/tests/actofgod/journal.log
@@ -858,7 +858,7 @@
     Play#0 output Play#0 6 0 640
     Play#0 input edit s 0 U 0 U -1
     Play#0 command edit
-    Play#0 output Play#0 1 command failed
+    Play#0 output Play#0 1 Usage: edit <country|land|ship|plane|unit> <NAT|SECT|SHIP|PLANE|LAND> [<KEY> <VALUE>]...
     Play#0 output Play#0 6 0 640
     Play#0 input edit s 2 L 3,-1
     Play#0 command edit
@@ -896,7 +896,7 @@
     Play#0 output Play#0 6 0 640
     Play#0 input edit p 0 U 0 U -1
     Play#0 command edit
-    Play#0 output Play#0 1 command failed
+    Play#0 output Play#0 1 Usage: edit <country|land|ship|plane|unit> <NAT|SECT|SHIP|PLANE|LAND> [<KEY> <VALUE>]...
     Play#0 output Play#0 6 0 640
     Play#0 input edit p 2 l 3,-1
     Play#0 command edit
@@ -934,7 +934,7 @@
     Play#0 output Play#0 6 0 640
     Play#0 input edit u 0 U 0 U -1
     Play#0 command edit
-    Play#0 output Play#0 1 command failed
+    Play#0 output Play#0 1 Usage: edit <country|land|ship|plane|unit> <NAT|SECT|SHIP|PLANE|LAND> [<KEY> <VALUE>]...
     Play#0 output Play#0 6 0 640
     Play#0 input edit u 2 L 3,-1
     Play#0 command edit

diff --git a/tests/actofgod/server.log b/tests/actofgod/server.log
index 299fdaaa40f3971f0767938d4a3ab36bc1315d9c..15cbfe365ee65d90f303f04da62d5b9a230f6557 100644 (file)
--- a/tests/actofgod/server.log
+++ b/tests/actofgod/server.log
@@ -7,9 +7,6 @@ Connect from 127.0.0.1
 Connect from 127.0.0.1
 tester@127.0.0.1 using country #0
 tester@127.0.0.1 logged in as country #0
-Oops: id < 0 in ../src/lib/common/file.c:973
-Oops: id < 0 in ../src/lib/common/file.c:973
-Oops: id < 0 in ../src/lib/common/file.c:973
 tester@127.0.0.1 logged out, country #0
 Shutdown commencing (cleaning up threads.)
 Server shutting down on signal 15