fix #H7667 - maybe_reset_pick(), other bad context

When deciding whether to discard interrupted lock/unlock context while
changing levels, maybe_reset_pick() checks whether xlock.box is being
carried.  But it was doing so after the old level had been saved and
memory for non-carried container there had been freed.

That led to a couple of other issues.  context.travelcc was using -1
for 'no cached value', but the fields of travelcc have type 'xchar' and
shouldn't be given negative values.  0 should be fine for 'no cache'.

Failed partial restore which occurred after old game's context had been
loaded would begin a new game with old game's stale context.  Restoring
goes out of its way to avoid that for 'flags' but didn't for 'context'.

diff --git a/doc/fixes36.2 b/doc/fixes36.2
index f5649056c07f1e738ad2a62a89a04b583664c1c6..4073cef20fd0de04536278f05c08a1f06e647eaa 100644 (file)
--- a/doc/fixes36.2
+++ b/doc/fixes36.2
@@ -238,6 +238,10 @@ glowing Sting quivers if hero becomes blind and quivering Sting glows if
        blindness ends; it worked for timed blindness but not for blindfold
 weapon (wielded pie, egg, potion, boomerang) might be destroyed when hitting a
        long worm, then freed memory was accessed to decide whether to cut it
+level change after being interruped locking or unlocking a container might
+       access freed memory
+if a restore attempt failed and a new game was started instead, it would use
+       stale context from old game if restoration got far enough to load that
 
 
 Fixes to Post-3.6.1 Problems that Were Exposed Via git Repository

diff --git a/src/cmd.c b/src/cmd.c
index 92b2fc0d2eb2370b22b9fac2a2818b1369470a86..8924923da4f1c853a359634f87f6a0f1c92ba4be 100644 (file)
--- a/src/cmd.c
+++ b/src/cmd.c
@@ -1,4 +1,4 @@
-/* NetHack 3.6 cmd.c   $NHDT-Date: 1543797825 2018/12/03 00:43:45 $  $NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.312 $ */
+/* NetHack 3.6 cmd.c   $NHDT-Date: 1543972186 2018/12/05 01:09:46 $  $NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.313 $ */
 /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */
 /*-Copyright (c) Robert Patrick Rankin, 2013. */
 /* NetHack may be freely redistributed.  See license for details. */
@@ -5694,7 +5694,7 @@ dotravel(VOID_ARGS)
     cmd[1] = 0;
     cc.x = iflags.travelcc.x;
     cc.y = iflags.travelcc.y;
-    if (cc.x == -1 && cc.y == -1) {
+    if (cc.x == 0 && cc.y == 0) {
         /* No cached destination, start attempt from current position */
         cc.x = u.ux;
         cc.y = u.uy;

diff --git a/src/do.c b/src/do.c
index 4817d150a82e2733247eaad880e04d1e9ab38dfd..febfcf9b5d55d5d88be48edbcd000fb30c885cd1 100644 (file)
--- a/src/do.c
+++ b/src/do.c
@@ -1,4 +1,4 @@
-/* NetHack 3.6 do.c    $NHDT-Date: 1543052696 2018/11/24 09:44:56 $  $NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.175 $ */
+/* NetHack 3.6 do.c    $NHDT-Date: 1543972190 2018/12/05 01:09:50 $  $NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.176 $ */
 /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */
 /*-Copyright (c) Derek S. Ray, 2015. */
 /* NetHack may be freely redistributed.  See license for details. */
@@ -1237,6 +1237,17 @@ boolean at_stairs, falling, portal;
     if (fd < 0)
         return;
 
+    /* discard context which applies to the level we're leaving;
+       for lock-picking, container may be carried, in which case we
+       keep context; if on the floor, it's about to be saved+freed and
+       maybe_reset_pick() needs to do its carried() check before that */
+    maybe_reset_pick();
+    reset_trapset(); /* even if to-be-armed trap obj is accompanying hero */
+    iflags.travelcc.x = iflags.travelcc.y = 0; /* travel destination cache */
+    context.polearm.hitmon = (struct monst *) 0; /* polearm target */
+    /* digging context is level-aware and can actually be resumed if
+       hero returns to the previous level without any intervening dig */
+
     if (falling) /* assuming this is only trap door or hole */
         impact_drop((struct obj *) 0, u.ux, u.uy, newlevel->dlevel);
 
@@ -1576,14 +1587,6 @@ boolean at_stairs, falling, portal;
     /* assume this will always return TRUE when changing level */
     (void) in_out_region(u.ux, u.uy);
     (void) pickup(1);
-
-    /* discard context which applied to previous level */
-    maybe_reset_pick(); /* for door or for box not accompanying hero */
-    reset_trapset(); /* even if to-be-armed trap obj is accompanying hero */
-    iflags.travelcc.x = iflags.travelcc.y = -1; /* travel destination cache */
-    context.polearm.hitmon = (struct monst *) 0; /* polearm target */
-    /* digging context is level-aware and can actually be resumed if
-       hero returns to the previous level without any intervening dig */
 }
 
 STATIC_OVL void

diff --git a/src/hack.c b/src/hack.c
index 7a5e48116d9f382bbfe71a5af07bb6eb3bf77b27..c0c0313f2238bc036e9d2c8a2e131dc623089110 100644 (file)
--- a/src/hack.c
+++ b/src/hack.c
@@ -1,4 +1,4 @@
-/* NetHack 3.6 hack.c  $NHDT-Date: 1540591769 2018/10/26 22:09:29 $  $NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.194 $ */
+/* NetHack 3.6 hack.c  $NHDT-Date: 1543972190 2018/12/05 01:09:50 $  $NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.200 $ */
 /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */
 /*-Copyright (c) Derek S. Ray, 2015. */
 /* NetHack may be freely redistributed.  See license for details. */
@@ -923,7 +923,7 @@ int mode;
                 u.dx = u.tx - u.ux;
                 u.dy = u.ty - u.uy;
                 nomul(0);
-                iflags.travelcc.x = iflags.travelcc.y = -1;
+                iflags.travelcc.x = iflags.travelcc.y = 0;
             }
             return TRUE;
         }
@@ -1045,7 +1045,7 @@ int mode;
                                     nomul(0);
                                     /* reset run so domove run checks work */
                                     context.run = 8;
-                                    iflags.travelcc.x = iflags.travelcc.y = -1;
+                                    iflags.travelcc.x = iflags.travelcc.y = 0;
                                 }
                                 return TRUE;
                             }

diff --git a/src/options.c b/src/options.c
index 8be8cfcd3889be1bb88162b63af930f1e734a30a..d4b7daf4dd068edd060149d813f4a342320bc35c 100644 (file)
--- a/src/options.c
+++ b/src/options.c
@@ -1,4 +1,4 @@
-/* NetHack 3.6 options.c       $NHDT-Date: 1543395749 2018/11/28 09:02:29 $  $NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.334 $ */
+/* NetHack 3.6 options.c       $NHDT-Date: 1543972192 2018/12/05 01:09:52 $  $NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.335 $ */
 /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */
 /*-Copyright (c) Michael Allison, 2008. */
 /* NetHack may be freely redistributed.  See license for details. */
@@ -747,8 +747,6 @@ initoptions_init()
         warnsyms[i] = def_warnsyms[i].sym;
     iflags.bouldersym = 0;
 
-    iflags.travelcc.x = iflags.travelcc.y = -1;
-
     /* for "special achievement" tracking (see obj.h,
        create_object(sp_lev.c), addinv_core1(invent.c) */
     iflags.mines_prize_type = LUCKSTONE;

diff --git a/src/restore.c b/src/restore.c
index 25604f10d1834bc9d93934d183982336a8dafd94..e9f25d72cf84e4e8f12dc4c63d718a9ecede3928 100644 (file)
--- a/src/restore.c
+++ b/src/restore.c
@@ -1,4 +1,4 @@
-/* NetHack 3.6 restore.c       $NHDT-Date: 1542798626 2018/11/21 11:10:26 $  $NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.109 $ */
+/* NetHack 3.6 restore.c       $NHDT-Date: 1543972193 2018/12/05 01:09:53 $  $NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.128 $ */
 /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */
 /*-Copyright (c) Michael Allison, 2009. */
 /* NetHack may be freely redistributed.  See license for details. */
@@ -539,6 +539,7 @@ unsigned int *stuckid, *steedid;
 #ifdef SYSFLAGS
     struct sysflag newgamesysflags;
 #endif
+    struct context_info newgamecontext; /* all 0, but has some pointers */
     struct obj *otmp, *tmp_bc;
     char timebuf[15];
     unsigned long uid;
@@ -553,9 +554,15 @@ unsigned int *stuckid, *steedid;
         if (!wizard)
             return FALSE;
     }
+
+    newgamecontext = context; /* copy statically init'd context */
     mread(fd, (genericptr_t) &context, sizeof (struct context_info));
-    if (context.warntype.speciesidx >= LOW_PM)
-        context.warntype.species = &mons[context.warntype.speciesidx];
+    context.warntype.species = (context.warntype.speciesidx >= LOW_PM)
+                                  ? &mons[context.warntype.speciesidx]
+                                  : (struct permonst *) 0;
+    /* context.victual.piece, .tin.tin, .spellbook.book, and .polearm.hitmon
+       are pointers which get set to Null during save and will be recovered
+       via corresponding o_id or m_id while objs or mons are being restored */
 
     /* we want to be able to revert to command line/environment/config
        file option values instead of keeping old save file option values
@@ -625,6 +632,7 @@ unsigned int *stuckid, *steedid;
 #ifdef SYSFLAGS
         sysflags = newgamesysflags;
 #endif
+        context = newgamecontext;
         return FALSE;
     }
     /* in case hangup save occurred in midst of level change */

diff --git a/src/save.c b/src/save.c
index 095890f05f1a5429ca68ebf40eedd62bdd7f4823..18465966d8cbf4c6e7a0a5bfe88c2471ea1750d4 100644 (file)
--- a/src/save.c
+++ b/src/save.c
@@ -1,4 +1,4 @@
-/* NetHack 3.6 save.c  $NHDT-Date: 1489192905 2017/03/11 00:41:45 $  $NHDT-Branch: NetHack-3.6.0 $:$NHDT-Revision: 1.101 $ */
+/* NetHack 3.6 save.c  $NHDT-Date: 1543972194 2018/12/05 01:09:54 $  $NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.115 $ */
 /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */
 /*-Copyright (c) Michael Allison, 2009. */
 /* NetHack may be freely redistributed.  See license for details. */
@@ -1035,37 +1035,22 @@ register struct obj *otmp;
         if (Has_contents(otmp))
             saveobjchn(fd, otmp->cobj, mode);
         if (release_data(mode)) {
-            /*  if (otmp->oclass == FOOD_CLASS)
-             *      food_disappears(otmp);
-             */
             /*
-             * If these are on the floor, the discarding could
-             * be because of a game save, or we could just be changing levels.
+             * If these are on the floor, the discarding could be
+             * due to game save, or we could just be changing levels.
              * Always invalidate the pointer, but ensure that we have
              * the o_id in order to restore the pointer on reload.
              */
             if (otmp == context.victual.piece) {
-                /* Store the o_id of the victual if mismatched */
-                if (context.victual.o_id != otmp->o_id)
-                    context.victual.o_id = otmp->o_id;
-                /* invalidate the pointer; on reload it will get restored */
+                context.victual.o_id = otmp->o_id;
                 context.victual.piece = (struct obj *) 0;
             }
             if (otmp == context.tin.tin) {
-                /* Store the o_id of your tin */
-                if (context.tin.o_id != otmp->o_id)
-                    context.tin.o_id = otmp->o_id;
-                /* invalidate the pointer; on reload it will get restored */
+                context.tin.o_id = otmp->o_id;
                 context.tin.tin = (struct obj *) 0;
             }
-            /*  if (otmp->oclass == SPBOOK_CLASS)
-             *      book_disappears(otmp);
-             */
             if (otmp == context.spbook.book) {
-                /* Store the o_id of your spellbook */
-                if (context.spbook.o_id != otmp->o_id)
-                    context.spbook.o_id = otmp->o_id;
-                /* invalidate the pointer; on reload it will get restored */
+                context.spbook.o_id = otmp->o_id;
                 context.spbook.book = (struct obj *) 0;
             }
             otmp->where = OBJ_FREE; /* set to free so dealloc will work */