get_adjacent_loc()

use get_adjacent_loc() rather than getdir() directly for some things where
you want to ensure valid adjacent coordinates are returned

<email deleted> wrote:
>>> [...]
>>> I've noticed that the loot adjacent spot code doesn't have any
>>> isok(x,y) test, so will risk crashing if used at the edge of
>>> the screen (whether deliberately, or accidentally due to being
>>> confused or stunned when picking the direction).

>> Would this not be a problem elsewhere, such as use_leash() too?

> Yes, that looks like the same risk. getdir() doesn't validate
> that the <u.ux+u.dx, u.uy,u.dy> is safe and neither does m_at(),
> so their callers need to.
>
> I did manage to provoke a crash with #loot on the plane of earth,
> although an accidental case would be a lot less likely to happen.

diff --git a/doc/fixes34.1 b/doc/fixes34.1
index 52da83b554cf28ac52ee15bd5642815216fa3d7f..7cdc789ff5420cd51aedb390fe373953d9723cae 100644 (file)
--- a/doc/fixes34.1
+++ b/doc/fixes34.1
@@ -306,6 +306,8 @@ all objects carried by a monster who's hit by a polymorph zap are protected
 sparkle option for display effects was ignored on explosions
 level teleport while on a sleeping steed caused panic and possible crash
 breaking wand of digging causing a shopkeeper to fall left unpaid items unpaid
+use get_adjacent_loc() rather than getdir() directly for some things where
+       you want to ensure valid adjacent coordinates are returned
 
 
 Platform- and/or Interface-Specific Fixes

diff --git a/include/extern.h b/include/extern.h
index 0bf4a7a6b046384b490fa732885ba631a35450c8..a076de73ebf0210d98689755aa93e990666b71e0 100644 (file)
--- a/include/extern.h
+++ b/include/extern.h
@@ -172,6 +172,7 @@ E int FDECL(movecmd, (CHAR_P));
 E int FDECL(getdir, (const char *));
 E void NDECL(confdir);
 E int FDECL(isok, (int,int));
+E int FDECL(get_adjacent_loc, (const char *, const char *, XCHAR_P, XCHAR_P, coord *));
 E const char *FDECL(click_to_cmd, (int,int,int));
 E char NDECL(readchar);
 #ifdef WIZARD

diff --git a/src/apply.c b/src/apply.c
index 82b7a7003c9d29471e9f9a3c05017a048254f410..6f27d8f55e5483b17d45c444cf49eb6b70fe0f16 100644 (file)
--- a/src/apply.c
+++ b/src/apply.c
@@ -400,7 +400,7 @@ STATIC_OVL void
 use_leash(obj)
 struct obj *obj;
 {
-       register int x, y;
+       coord cc;
        register struct monst *mtmp;
        int spotmon;
 
@@ -409,12 +409,9 @@ struct obj *obj;
                return;
        }
 
-       if(!getdir((char *)0)) return;
-
-       x = u.ux + u.dx;
-       y = u.uy + u.dy;
+       if(!get_adjacent_loc((char *)0, (char *)0, u.ux, u.uy, &cc)) return;
 
-       if((x == u.ux) && (y == u.uy)) {
+       if((cc.x == u.ux) && (cc.y == u.uy)) {
 #ifdef STEED
                if (u.usteed && u.dz > 0) {
                    mtmp = u.usteed;
@@ -426,7 +423,7 @@ struct obj *obj;
                return;
        }
 
-       if(!(mtmp = m_at(x, y))) {
+       if(!(mtmp = m_at(cc.x, cc.y))) {
                There("is no creature there.");
                return;
        }

diff --git a/src/cmd.c b/src/cmd.c
index 6cdcf343642eb012c32793017bd2c453c805168a..de90f468808dc783569683ec9418849f45de2047 100644 (file)
--- a/src/cmd.c
+++ b/src/cmd.c
@@ -1958,6 +1958,38 @@ char sym;
        return !u.dz;
 }
 
+/*
+ * uses getdir() but unlike getdir() it specifically
+ * produces coordinates using the direction from getdir()
+ * and verifies that those coordinates are ok.
+ *
+ * If the call to getdir() returns 0, Never_mind is displayed.
+ * If the resulting coordinates are not okay, emsg is displayed.
+ *
+ * Returns non-zero if coordinates in cc are valid.
+ */
+int get_adjacent_loc(prompt,emsg,x,y,cc)
+const char *prompt, *emsg;
+xchar x,y;
+coord *cc;
+{
+       xchar new_x, new_y;
+       if (!getdir(prompt)) {
+               pline(Never_mind);
+               return 0;
+       }
+       new_x = x + u.dx;
+       new_y = y + u.dy;
+       if (cc && isok(new_x,new_y)) {
+               cc->x = new_x;
+               cc->y = new_y;
+       } else {
+               if (emsg) pline(emsg);
+               return 0;
+       }
+       return 1;
+}
+
 int
 getdir(s)
 const char *s;

diff --git a/src/pickup.c b/src/pickup.c
index a69f8e25c36c735ff20e2b135be21b6406ecd2fe..9ff54c738605c474ee33ab43ebab71d2dae20ed0 100644 (file)
--- a/src/pickup.c
+++ b/src/pickup.c
@@ -1402,12 +1402,12 @@ int x, y;
 }
 
 int
-doloot()       /* loot a container on the floor. */
+doloot()       /* loot a container on the floor or loot saddle from mon. */
 {
     register struct obj *cobj, *nobj;
     register int c = -1;
     int timepassed = 0;
-    int x,y;
+    coord cc;
     boolean underfoot = TRUE;
     const char *dont_find_anything = "don't find anything";
     struct monst *mtmp;
@@ -1423,15 +1423,15 @@ doloot()        /* loot a container on the floor. */
        You("have no hands!");  /* not `body_part(HAND)' */
        return 0;
     }
-    x = u.ux; y = u.uy;
+    cc.x = u.ux; cc.y = u.uy;
 
 lootcont:
 
-    if (container_at(x, y, FALSE)) {
+    if (container_at(cc.x, cc.y, FALSE)) {
        boolean any = FALSE;
 
-       if (!able_to_loot(x, y)) return 0;
-       for (cobj = level.objects[x][y]; cobj; cobj = nobj) {
+       if (!able_to_loot(cc.x, cc.y)) return 0;
+       for (cobj = level.objects[cc.x][cc.y]; cobj; cobj = nobj) {
            nobj = cobj->nexthere;
 
            if (Is_container(cobj)) {
@@ -1520,32 +1520,28 @@ gotit:
                pline("Ok, now there is loot here.");
            }
        }
-    } else if (IS_GRAVE(levl[x][y].typ)) {
+    } else if (IS_GRAVE(levl[cc.x][cc.y].typ)) {
        You("need to dig up the grave to effectively loot it...");
     }
     /*
      * 3.3.1 introduced directional looting for some things.
      */
     if (c != 'y' && mon_beside(u.ux, u.uy)) {
-       if (!getdir("Loot in what direction?")) {
-           pline(Never_mind);
-           return(0);
-       }
-       x = u.ux + u.dx;
-       y = u.uy + u.dy;
-       if (x == u.ux && y == u.uy) {
+       if (!get_adjacent_loc("Loot in what direction?", "Invalid loot location",
+                       u.ux, u.uy, &cc)) return 0;
+       if (cc.x == u.ux && cc.y == u.uy) {
            underfoot = TRUE;
-           if (container_at(x, y, FALSE))
+           if (container_at(cc.x, cc.y, FALSE))
                goto lootcont;
        } else
            underfoot = FALSE;
        if (u.dz < 0) {
            You("%s to loot on the %s.", dont_find_anything,
-               ceiling(x, y));
+               ceiling(cc.x, cc.y));
            timepassed = 1;
            return timepassed;
        }
-       mtmp = m_at(x, y);
+       mtmp = m_at(cc.x, cc.y);
        if (mtmp) timepassed = loot_mon(mtmp, &prev_inquiry, &prev_loot);
 
        /* Preserve pre-3.3.1 behaviour for containers.
@@ -1553,7 +1549,7 @@ gotit:
         * from one square away to change that in the future.
         */
        if (!underfoot) {
-           if (container_at(x, y, FALSE)) {
+           if (container_at(cc.x, cc.y, FALSE)) {
                if (mtmp) {
                    You_cant("loot anything %sthere with %s in the way.",
                            prev_inquiry ? "else " : "", mon_nam(mtmp));