From: PatR <rankin@nethack.org>
Date: Fri, 5 Jan 2018 09:23:56 +0000 (-0800)
Subject: fix #H6713 - unpaid_cost: object not on any bill
X-Git-Tag: NetHack-3.6.1_RC01~184^2
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=0c5155584975731f2c37ace88acd046a54ae5aa6;p=nethack

fix #H6713 - unpaid_cost: object not on any bill

Stealing a shop object from outside the shop with a grappling hook
would result in that item being left marked 'unpaid' after the shop's
bill was treated as being bought and not yet paid for.  This led to
"unpaid_cost: object wasn't on any bill" every time inventory was
examined.  The problem was caused by handling the shop robbery after
removing the object from the floor but before adding it to inventory,
so it couldn't be found to have its unpaid bit cleared.

When investigating this I came across a more severe bug:  if the hero
had never entered the shop, the shopkeeper's bill wasn't initialized
properly and add_one_tobill() could crash while attempting to execute
    bp->bo_id = obj->o_id;
because 'bp' was Null.
---

diff --git a/doc/fixes36.1 b/doc/fixes36.1
index 2b0c439e8..d33e86598 100644
--- a/doc/fixes36.1
+++ b/doc/fixes36.1
@@ -498,6 +498,11 @@ if a special level specified the appearance of a mimic and mimics had been
 	genocided prior to creating the level, whatever random monster took
 	the mimic's place got its intended appearance
 redundant "hit by gush of water" message if poly'd into iron golem or gremlin
+a shop object stolen from outside the shop (via grappling hook) would be left
+	marked as 'unpaid' after the shop robbery took place, resulting in
+	"unpaid_cost: object wasn't on any bill" when looking at inventory
+a shop object stolen from outside the shop could trigger a crash if that shop
+	had never been entered by the hero
 
 
 Fixes to Post-3.6.0 Problems that Were Exposed Via git Repository
diff --git a/src/pickup.c b/src/pickup.c
index aa147080b..d09bbbfdb 100644
--- a/src/pickup.c
+++ b/src/pickup.c
@@ -1,4 +1,4 @@
-/* NetHack 3.6	pickup.c	$NHDT-Date: 1508549438 2017/10/21 01:30:38 $  $NHDT-Branch: NetHack-3.6.0 $:$NHDT-Revision: 1.192 $ */
+/* NetHack 3.6	pickup.c	$NHDT-Date: 1515144225 2018/01/05 09:23:45 $  $NHDT-Branch: NetHack-3.6.0 $:$NHDT-Revision: 1.193 $ */
 /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */
 /* NetHack may be freely redistributed.  See license for details. */
 
@@ -1495,15 +1495,30 @@ struct obj *
 pick_obj(otmp)
 struct obj *otmp;
 {
+    struct obj *result;
+    int ox = otmp->ox, oy = otmp->oy;
+    boolean robshop = (!u.uswallow && otmp != uball && costly_spot(ox, oy));
+
     obj_extract_self(otmp);
-    if (!u.uswallow && otmp != uball && costly_spot(otmp->ox, otmp->oy)) {
+    otmp->nomerge = 1;
+    result = addinv(otmp);
+    otmp->nomerge = 0;
+    newsym(ox, oy);
+
+    /* this used to be done before addinv(), but remote_burglary()
+       calls rob_shop() which calls setpaid() after moving costs of
+       unpaid items to shop debt; setpaid() calls clear_unpaid() for
+       lots of object chains, but 'otmp' wasn't on any of those so
+       remained flagged as an unpaid item in inventory, triggering
+       impossible() every time inventory was examined... */
+    if (robshop) {
         char saveushops[5], fakeshop[2];
 
         /* addtobill cares about your location rather than the object's;
            usually they'll be the same, but not when using telekinesis
            (if ever implemented) or a grappling hook */
         Strcpy(saveushops, u.ushops);
-        fakeshop[0] = *in_rooms(otmp->ox, otmp->oy, SHOPBASE);
+        fakeshop[0] = *in_rooms(ox, oy, SHOPBASE);
         fakeshop[1] = '\0';
         Strcpy(u.ushops, fakeshop);
         /* sets obj->unpaid if necessary */
@@ -1511,10 +1526,9 @@ struct obj *otmp;
         Strcpy(u.ushops, saveushops);
         /* if you're outside the shop, make shk notice */
         if (!index(u.ushops, *fakeshop))
-            remote_burglary(otmp->ox, otmp->oy);
+            remote_burglary(ox, oy);
     }
-    newsym(otmp->ox, otmp->oy);
-    return addinv(otmp); /* might merge it with other objects */
+    return result;
 }
 
 /*
diff --git a/src/shk.c b/src/shk.c
index 62052fbe7..c81c788b2 100644
--- a/src/shk.c
+++ b/src/shk.c
@@ -1,4 +1,4 @@
-/* NetHack 3.6	shk.c	$NHDT-Date: 1464138042 2016/05/25 01:00:42 $  $NHDT-Branch: NetHack-3.6.0 $:$NHDT-Revision: 1.132 $ */
+/* NetHack 3.6	shk.c	$NHDT-Date: 1515144230 2018/01/05 09:23:50 $  $NHDT-Branch: NetHack-3.6.0 $:$NHDT-Revision: 1.136 $ */
 /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */
 /* NetHack may be freely redistributed.  See license for details. */
 
@@ -2354,6 +2354,12 @@ struct monst *shkp;
         return;
     }
 
+    /* normally bill_p gets set up whenever you enter the shop, but obj
+       might be going onto the bill because hero just snagged it with
+       a grappling hook from outside without ever having been inside */
+    if (!eshkp->bill_p)
+        eshkp->bill_p = &(eshkp->bill[0]);
+
     bct = eshkp->billct;
     bp = &(eshkp->bill_p[bct]);
     bp->bo_id = obj->o_id;