]> granicus.if.org Git - nethack/commitdiff
fix #K1963 - warning after placing worm tail
authorPatR <rankin@nethack.org>
Tue, 8 Sep 2020 10:03:03 +0000 (03:03 -0700)
committerPatR <rankin@nethack.org>
Tue, 8 Sep 2020 10:03:03 +0000 (03:03 -0700)
Report described this as a panic triggered by the sanity_check
option, but that's because it was running under the fuzzer, which
escalates any impossible() to panic(), rather than because nethack
panicked.

I couldn't find anything wrong--which doesn't mean that there
isn't something wrong--with place_worm_tail_randomly() and
random_dir().  They use xchar for map coordinates which should be
fine as long as no negative values are generated and I couldn't
discover any such.  The suggested fix of changing xchar to int
might indicate a compiler bug (although the odds of that are low).
The bogus coordinate of -15000 in the report suggests that
 typedef short int schar;
(which changes xchar too) is being used in the configuration but
I don't recall having any problems attributable to that.

This switches from xchar to int as a side-effect of replacing the
offending code entirely.  The new code might produce an 'ny' of -1
before goodpos() rejects it, so xchar would be inappropriate now.
The old code is commented out via #if 0 _after_ changing it from
xchar to int.

This also adds an extra sanity_check for worm tails, unrelated to
the current bug.  I'm not aware of any instance where it fails.
EXTRA_SANITY_CHECKS needs to be defined for it to do anything.

doc/fixes37.0
include/extern.h
src/mon.c
src/worm.c

index 3364e09c41e9976d9fdd0f246d2aca61305f54fb..94944bd90b8adf2b4e37364c946ec13cdd9492dc 100644 (file)
@@ -1,4 +1,4 @@
-NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.298 $ $NHDT-Date: 1599434249 2020/09/06 23:17:29 $
+NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.299 $ $NHDT-Date: 1599559379 2020/09/08 10:02:59 $
 
 General Fixes and Modified Features
 -----------------------------------
@@ -328,6 +328,8 @@ the fix to make worm visibility checks work as intended forced the coordinates
 using 'O' to try to change 'symset' was a no-op; 'roguesymset' worked
 change default for lit attribute in special level des.terrain directives to
        'unchanged' instead of 'unlit'
+replace worm tail placement code that reportedly led to a sanity_check warning
+       [no actual code problem found; might be compiler bug for 'xchar']
 
 curses: 'msg_window' option wasn't functional for curses unless the binary
        also included tty support
index 4374310353ab89afc2945841b01041122bdfaf3a..c1f2ae298c3f223dc34c8cce9949498ec4f3101b 100644 (file)
@@ -1,4 +1,4 @@
-/* NetHack 3.7 extern.h        $NHDT-Date: 1597069374 2020/08/10 14:22:54 $  $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.855 $ */
+/* NetHack 3.7 extern.h        $NHDT-Date: 1599559379 2020/09/08 10:02:59 $  $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.856 $ */
 /* Copyright (c) Steve Creps, 1988.                              */
 /* NetHack may be freely redistributed.  See license for details. */
 
@@ -3078,6 +3078,7 @@ E void FDECL(save_worm, (NHFILE *));
 E void FDECL(rest_worm, (NHFILE *));
 E void FDECL(place_wsegs, (struct monst *, struct monst *));
 E void FDECL(sanity_check_worm, (struct monst *));
+E void NDECL(wormno_sanity_check);
 E void FDECL(remove_worm, (struct monst *));
 E void FDECL(place_worm_tail_randomly, (struct monst *, XCHAR_P, XCHAR_P));
 E int FDECL(size_wseg, (struct monst *));
index 83e28ebbc83b7d39a62fc3a04a74fe2480a2e0f7..2df04ef293b6730d1672d6b4aa903f0b12529e44 100644 (file)
--- a/src/mon.c
+++ b/src/mon.c
@@ -1,4 +1,4 @@
-/* NetHack 3.7 mon.c   $NHDT-Date: 1599330921 2020/09/05 18:35:21 $  $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.345 $ */
+/* NetHack 3.7 mon.c   $NHDT-Date: 1599559379 2020/09/08 10:02:59 $  $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.346 $ */
 /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */
 /*-Copyright (c) Derek S. Ray, 2015. */
 /* NetHack may be freely redistributed.  See license for details. */
@@ -150,6 +150,8 @@ mon_sanity_check()
     for (mtmp = g.migrating_mons; mtmp; mtmp = mtmp->nmon) {
         sanity_check_single_mon(mtmp, FALSE, "migr");
     }
+
+    wormno_sanity_check(); /* test for bogus worm tail */
 }
 
 /* Would monster be OK with poison gas? */
@@ -1820,7 +1822,7 @@ struct monst *mtmp, *mtmp2;
     if (mtmp != u.usteed) /* don't place steed onto the map */
         place_monster(mtmp2, mtmp2->mx, mtmp2->my);
     if (mtmp2->wormno)      /* update level.monsters[wseg->wx][wseg->wy] */
-        place_wsegs(mtmp2, NULL); /* locations to mtmp2 not mtmp. */
+        place_wsegs(mtmp2, mtmp); /* locations to mtmp2 not mtmp. */
     if (emits_light(mtmp2->data)) {
         /* since this is so rare, we don't have any `mon_move_light_source' */
         new_light_source(mtmp2->mx, mtmp2->my, emits_light(mtmp2->data),
index c32c8c928ab3e4a04a14372341f8bd7bba893d35..9027f925b717076d6e2da688cbecf5a9d30bb19d 100644 (file)
@@ -1,4 +1,4 @@
-/* NetHack 3.7 worm.c  $NHDT-Date: 1596841504 2020/08/07 23:05:04 $  $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.47 $ */
+/* NetHack 3.7 worm.c  $NHDT-Date: 1599559380 2020/09/08 10:03:00 $  $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.48 $ */
 /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */
 /*-Copyright (c) Robert Patrick Rankin, 2009. */
 /* NetHack may be freely redistributed.  See license for details. */
@@ -16,7 +16,9 @@ struct wseg {
 
 static void FDECL(toss_wsegs, (struct wseg *, BOOLEAN_P));
 static void FDECL(shrink_worm, (int));
-static void FDECL(random_dir, (XCHAR_P, XCHAR_P, xchar *, xchar *));
+#if 0
+static void FDECL(random_dir, (int, int, int *, int *));
+#endif
 static struct wseg *FDECL(create_worm_tail, (int));
 
 /*  Description of long worm implementation.
@@ -680,6 +682,31 @@ struct monst *worm;
     }
 }
 
+/* called from mon_sanity_check(mon.c) */
+void
+wormno_sanity_check()
+{
+#ifdef EXTRA_SANITY_CHECKS
+    struct wseg *seg;
+    int wh = 0, wt = 0;
+
+    /* checking tail management, not a particular monster; since wormno==0
+       means 'not a worm', wheads[0] and wtails[0] should always be empty;
+       note: if erroneously non-Null, tail segment count will include the
+       extra segment for the worm's head that isn't shown on the map */
+    for (seg = wheads[0]; seg; seg = seg->nseg)
+        ++wh;
+    for (seg = wtails[0]; seg; seg = seg->nseg)
+        ++wt;
+    if (wh || wt) {
+        impossible(
+        "phantom worm tail #0 [head=%s, %d segment%s; tail=%s, %d segment%s]",
+                   fmt_ptr(wheads[0]), wh, plur(wh),
+                   fmt_ptr(wtails[0]), wt, plur(wt));
+    }
+#endif /* EXTRA_SANITY_CHECKS */
+}
+
 /*
  *  remove_worm()
  *
@@ -721,7 +748,7 @@ xchar x, y;
     int wnum = worm->wormno;
     struct wseg *curr = wtails[wnum];
     struct wseg *new_tail;
-    xchar ox = x, oy = y;
+    int ox = x, oy = y;
 
     if (wnum && (!wtails[wnum] || !wheads[wnum])) {
         impossible("place_worm_tail_randomly: wormno is set without a tail!");
@@ -729,10 +756,14 @@ xchar x, y;
     }
     if (wtails[wnum] == wheads[wnum]) {
         /* single segment, co-located with worm so nothing to place */
-        if (curr->wx != worm->mx || curr->wy != worm->my)
+        if (curr->wx != worm->mx || curr->wy != worm->my) {
             impossible(
         "place_worm_tail_randomly: tail segement at <%d,%d>, worm at <%d,%d>",
                        curr->wx, curr->wy, worm->mx, worm->my);
+            if (m_at(curr->wx, curr->wy) == worm)
+                remove_monster(curr->wx, curr->wy);
+            curr->wx = worm->mx, curr->wy = worm->my;
+        }
         return;
     }
     /* remove head segment from map in case we end up calling toss_wsegs();
@@ -747,30 +778,56 @@ xchar x, y;
     new_tail->wy = y;
 
     while (curr) {
-        xchar nx, ny;
-        int tryct = 0;
+        int nx = 0, ny = 0;
+#if 0   /* old code */
+        int trycnt = 0;
 
-        /* pick a random direction from x, y and search for goodpos() */
+        /* pick a random direction from x, y and test for goodpos() */
         do {
             random_dir(ox, oy, &nx, &ny);
-        } while (!goodpos(nx, ny, worm, 0) && (tryct++ < 50));
+        } while (!goodpos(nx, ny, worm, 0) && ++tryct <= 50);
+
+        if (tryct <= 50)
+#else   /* new code */
+        int i, j, k, dirs[8];
+
+        /* instead of picking a random direction up to 50 times, try each
+           of the eight directions at most once after shuffling their order */
+        for (i = 0; i < 8; ++i)
+            dirs[i] = i;
+        for (i = 8; i > 0; --i) {
+            j = rn2(i);
+            k = dirs[j];
+            dirs[j] = dirs[i - 1];
+            dirs[i - 1] = k;
+        }
+        for (i = 0; i < 8; ++i) {
+            nx = ox + xdir[dirs[i]];
+            ny = oy + ydir[dirs[i]];
+            if (goodpos(nx, ny, worm, 0)) /* includes an isok() check */
+                break;
+        }
 
-        if (tryct < 50) {
+        if (i < 8)
+#endif
+        {
             place_worm_seg(worm, nx, ny);
-            curr->wx = ox = nx;
-            curr->wy = oy = ny;
+            curr->wx = (xchar) (ox = nx);
+            curr->wy = (xchar) (oy = ny);
             wtails[wnum] = curr;
             curr = curr->nseg;
             wtails[wnum]->nseg = new_tail;
             new_tail = wtails[wnum];
             newsym(nx, ny);
-        } else {                     /* Oops.  Truncate because there was */
-            toss_wsegs(curr, FALSE); /* no place for the rest of it */
+        } else {
+            /* Oops.  Truncate because there is no place for rest of it. */
+            toss_wsegs(curr, FALSE);
             curr = (struct wseg *) 0;
         }
     }
 }
 
+#if 0
 /*
  * Given a coordinate x, y.
  * return in *nx, *ny, the coordinates of one of the <= 8 squares adjoining.
@@ -781,8 +838,8 @@ xchar x, y;
 static
 void
 random_dir(x, y, nx, ny)
-xchar x, y;
-xchar *nx, *ny;
+int x, y;
+int *nx, *ny;
 {
     *nx = x + (x > 1                /* extreme left ? */
                ? (x < COLNO - 1     /* extreme right ? */
@@ -802,6 +859,7 @@ xchar *nx, *ny;
                       : -1)                 /* bottom, use -1 */
                    : 1);                    /* top, use +1 */
 }
+#endif
 
 /* for size_monst(cmd.c) to support #stats */
 int