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.

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

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

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;
+	if(!get_adjacent_loc((char *)0, (char *)0, u.ux, u.uy, &cc)) return;
 
-	x = u.ux + u.dx;
-	y = u.uy + u.dy;
-
-	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;
 	}

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;

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));