From 36c2aec2ffe413141dd402f51ee7516b6980cc57 Mon Sep 17 00:00:00 2001 From: PatR Date: Tue, 4 Dec 2018 17:10:15 -0800 Subject: [PATCH] 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'. --- doc/fixes36.2 | 4 ++++ src/cmd.c | 4 ++-- src/do.c | 21 ++++++++++++--------- src/hack.c | 6 +++--- src/options.c | 4 +--- src/restore.c | 14 +++++++++++--- src/save.c | 27 ++++++--------------------- 7 files changed, 39 insertions(+), 41 deletions(-) diff --git a/doc/fixes36.2 b/doc/fixes36.2 index f5649056c..4073cef20 100644 --- 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 92b2fc0d2..8924923da 100644 --- 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 4817d150a..febfcf9b5 100644 --- 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 7a5e48116..c0c0313f2 100644 --- 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 8be8cfcd3..d4b7daf4d 100644 --- 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 25604f10d..e9f25d72c 100644 --- 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 095890f05..18465966d 100644 --- 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 */