From 8bf16b940e15cac75cea2a4de92738c90fa71688 Mon Sep 17 00:00:00 2001 From: PatR Date: Thu, 31 Jan 2019 15:50:12 -0800 Subject: [PATCH] stale lock picking context Lock context wasn't being cleared if it was for a container and that container got destroyed. Case discovered was forcelock() -> breakchestlock() -> delobj() (sometimes the container is destroyed rather than just breaking its lock) followed by #wizmakemap (replace current level) and maybe_reset_pick() trying to check whether xlock.box was being carried. But being interrupted, destroying the container or dropping it down a hole to ship it to another level, then attempting to resume picking the lock would also find a stale pointer. --- doc/fixes36.2 | 4 +++- src/cmd.c | 4 ++-- src/do.c | 4 ++-- src/lock.c | 33 +++++++++++++++++++++++++-------- src/mkobj.c | 6 +++++- src/shk.c | 4 +++- 6 files changed, 40 insertions(+), 15 deletions(-) diff --git a/doc/fixes36.2 b/doc/fixes36.2 index c5c37abea..1c5b9edfa 100644 --- a/doc/fixes36.2 +++ b/doc/fixes36.2 @@ -1,4 +1,4 @@ -$NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.241 $ $NHDT-Date: 1548937318 2019/01/31 12:21:58 $ +$NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.242 $ $NHDT-Date: 1548978603 2019/01/31 23:50:03 $ This fixes36.2 file is here to capture information about updates in the 3.6.x lineage following the release of 3.6.1 in April 2018. Please note, however, @@ -357,6 +357,8 @@ smudging of an engraving has been relocated to after a succesful move subject to the smudging monster with multiple items in inventory could trigger 'dealloc_obj with nobj' panic when turned into a statue if separate mon->minvent items merged +lock picking context could end up with stale container pointer if container + being forced/unlocked/locked got destroyed or sent to another level Fixes to Post-3.6.1 Problems that Were Exposed Via git Repository diff --git a/src/cmd.c b/src/cmd.c index 75319f5e5..5b77eb1a4 100644 --- a/src/cmd.c +++ b/src/cmd.c @@ -1,4 +1,4 @@ -/* NetHack 3.6 cmd.c $NHDT-Date: 1547512504 2019/01/15 00:35:04 $ $NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.328 $ */ +/* NetHack 3.6 cmd.c $NHDT-Date: 1548978603 2019/01/31 23:50:03 $ $NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.330 $ */ /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */ /*-Copyright (c) Robert Patrick Rankin, 2013. */ /* NetHack may be freely redistributed. See license for details. */ @@ -823,7 +823,7 @@ wiz_makemap(VOID_ARGS) unplacebc(); } /* reset lock picking unless it's for a carried container */ - maybe_reset_pick(); + maybe_reset_pick((struct obj *) 0); /* reset interrupted digging if it was taking place on this level */ if (on_level(&context.digging.level, &u.uz)) (void) memset((genericptr_t) &context.digging, 0, diff --git a/src/do.c b/src/do.c index 7467356c7..20aadf751 100644 --- a/src/do.c +++ b/src/do.c @@ -1,4 +1,4 @@ -/* NetHack 3.6 do.c $NHDT-Date: 1547680082 2019/01/16 23:08:02 $ $NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.186 $ */ +/* NetHack 3.6 do.c $NHDT-Date: 1548978604 2019/01/31 23:50:04 $ $NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.189 $ */ /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */ /*-Copyright (c) Derek S. Ray, 2015. */ /* NetHack may be freely redistributed. See license for details. */ @@ -1322,7 +1322,7 @@ boolean at_stairs, falling, portal; 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(); + maybe_reset_pick((struct obj *) 0); 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 */ diff --git a/src/lock.c b/src/lock.c index 0f4c4dbff..3e2337351 100644 --- a/src/lock.c +++ b/src/lock.c @@ -1,4 +1,4 @@ -/* NetHack 3.6 lock.c $NHDT-Date: 1547086531 2019/01/10 02:15:31 $ $NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.83 $ */ +/* NetHack 3.6 lock.c $NHDT-Date: 1548978605 2019/01/31 23:50:05 $ $NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.84 $ */ /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */ /*-Copyright (c) Robert Patrick Rankin, 2011. */ /* NetHack may be freely redistributed. See license for details. */ @@ -253,10 +253,14 @@ forcelock(VOID_ARGS) return 1; /* still busy */ You("succeed in forcing the lock."); + exercise(xlock.picktyp ? A_DEX : A_STR, TRUE); + /* breakchestlock() might destroy xlock.box; if so, xlock context will + be cleared (delobj -> obfree -> maybe_reset_pick); but it might not, + so explicitly clear that manually */ breakchestlock(xlock.box, (boolean) (!xlock.picktyp && !rn2(3))); + reset_pick(); /* lock-picking context is no longer valid */ - exercise((xlock.picktyp) ? A_DEX : A_STR, TRUE); - return ((xlock.usedtime = 0)); + return 0; } void @@ -264,15 +268,28 @@ reset_pick() { xlock.usedtime = xlock.chance = xlock.picktyp = 0; xlock.magic_key = FALSE; - xlock.door = 0; - xlock.box = 0; + xlock.door = (struct rm *) 0; + xlock.box = (struct obj *) 0; } -/* level change; don't reset if hero is carrying xlock.box with him/her */ +/* level change or object deletion; context may no longer be valid */ void -maybe_reset_pick() +maybe_reset_pick(container) +struct obj *container; /* passed from obfree() */ { - if (!xlock.box || !carried(xlock.box)) + /* + * If a specific container, only clear context if it is for that + * particular container (which is being deleted). Other stuff on + * the current dungeon level remains valid. + * However if 'container' is Null, clear context if not carrying + * xlock.box (which might be Null if context is for a door). + * Used for changing levels, where a floor container or a door is + * being left behind and won't be valid on the new level but a + * carried container will still be. There might not be any context, + * in which case redundantly clearing it is harmless. + */ + if (container ? (container == xlock.box) + : (!xlock.box || !carried(xlock.box))) reset_pick(); } diff --git a/src/mkobj.c b/src/mkobj.c index 0c6b960ca..195de0b4a 100644 --- a/src/mkobj.c +++ b/src/mkobj.c @@ -1,4 +1,4 @@ -/* NetHack 3.6 mkobj.c $NHDT-Date: 1547086532 2019/01/10 02:15:32 $ $NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.141 $ */ +/* NetHack 3.6 mkobj.c $NHDT-Date: 1548978605 2019/01/31 23:50:05 $ $NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.142 $ */ /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */ /*-Copyright (c) Derek S. Ray, 2015. */ /* NetHack may be freely redistributed. See license for details. */ @@ -2076,6 +2076,10 @@ struct obj *obj; if (obj->where != OBJ_FREE) panic("add_to_migration: obj not free"); + /* lock picking context becomes stale if it's for this object */ + if (Is_container(obj)) + maybe_reset_pick(obj); + obj->where = OBJ_MIGRATING; obj->nobj = migrating_objs; migrating_objs = obj; diff --git a/src/shk.c b/src/shk.c index 154924550..09892e709 100644 --- a/src/shk.c +++ b/src/shk.c @@ -1,4 +1,4 @@ -/* NetHack 3.6 shk.c $NHDT-Date: 1547849604 2019/01/18 22:13:24 $ $NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.153 $ */ +/* NetHack 3.6 shk.c $NHDT-Date: 1548978606 2019/01/31 23:50:06 $ $NHDT-Branch: NetHack-3.6.2-beta01 $:$NHDT-Revision: 1.154 $ */ /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */ /*-Copyright (c) Robert Patrick Rankin, 2012. */ /* NetHack may be freely redistributed. See license for details. */ @@ -912,6 +912,8 @@ register struct obj *obj, *merge; book_disappears(obj); if (Has_contents(obj)) delete_contents(obj); + if (Is_container(obj)) + maybe_reset_pick(obj); shkp = 0; if (obj->unpaid) {