From 3de1e8be28915337b4908dc3a3500014b7d88f0b Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@pond.sub.org>
Date: Sun, 3 Jul 2011 09:53:39 +0200
Subject: [PATCH] Avoid false positive generation oops in navigate and march

Commit e3cf1e32 (v4.3.27) created make_stale_if_command_arg() to
permit catching more potential yields on input.  Unfortunately, the
implementation of navigate and march sub-commands 'r', 'l' and 's'
breaks it.

do_unit_move() reads units into a unit list at the beginning and at
each stop.  It writes them back when they move or sweep.  If a unit
changed in the file in between, the changes would get wiped out.
Therefore, do_unit_move() must not yield between stops.

do_unit_move() parses sub-commands into player->argp[], then supplies
defaults for missing arguments, so that code using them (radar(),
do_look(), sona(), mine(), landmine()) won't prompt for missing
arguments.  Unclean and brittle.  See also commit 28cc236e and commit
45106ab9.

Unfortunately, make_stale_if_command_arg() doesn't recognize the
difference between these defaulted arguments and parsed arguments, so
it makes objects stale, even though the defaulted arguments can't be
missing.  If a move or sweep follows, it triggers a false positive
generation oops.

To fix, test "points into argument buffer" (only true for parsed
arguments) instead of "is in player->argp[]".  Requires making the
argument buffer accessible: new struct player member argbuf[].  Use it
for parsing commands, in command(), execute(), do_unit_move().  Don't
use it in emp_config(), player_login(), move_ground(), because these
parse something else.
---
 include/player.h        |  1 +
 src/lib/commands/navi.c |  3 +--
 src/lib/player/player.c | 28 +++++++---------------------
 3 files changed, 9 insertions(+), 23 deletions(-)

diff --git a/include/player.h b/include/player.h
index 0997d12c2..afe0ec9c2 100644
--- a/include/player.h
+++ b/include/player.h
@@ -68,6 +68,7 @@ struct player {
     struct cmndstr *command;	/* currently executing command */
     struct iop *iop;
     char combuf[1024];		/* command input buffer, UTF-8 */
+    char argbuf[1024];		/* argument buffer, ASCII */
     char *argp[128];		/* arguments, ASCII, valid if command */
     char *condarg;		/* conditional, ASCII, valid if command */
     char *comtail[128];		/* start of args in combuf[] */
diff --git a/src/lib/commands/navi.c b/src/lib/commands/navi.c
index fec4bb8c0..cc5d51dbd 100644
--- a/src/lib/commands/navi.c
+++ b/src/lib/commands/navi.c
@@ -76,7 +76,6 @@ do_unit_move(struct emp_qelem *ulist, int *together,
     int skip = 0;
     char buf[1024];
     char prompt[128];
-    char scanspace[1024];
     char pathtaken[1024];  /* Doubtful we'll have a path longer than this */
     char *pt = pathtaken;
     char bmap_flag;
@@ -176,7 +175,7 @@ do_unit_move(struct emp_qelem *ulist, int *together,
 	    cp++;
 	    continue;
 	}
-	ac = parse(cp, scanspace, player->argp, NULL, NULL, NULL);
+	ac = parse(cp, player->argbuf, player->argp, NULL, NULL, NULL);
 	if (ac <= 0) {
 	    player->argp[0] = "";
 	    cp = NULL;
diff --git a/src/lib/player/player.c b/src/lib/player/player.c
index 9b8d37a41..6d8571d8a 100644
--- a/src/lib/player/player.c
+++ b/src/lib/player/player.c
@@ -117,7 +117,6 @@ command(void)
 {
     struct natstr *natp = getnatp(player->cnum);
     char *redir;		/* UTF-8 */
-    char scanspace[1024];
     time_t now;
 
     prprompt(natp->nat_timeused / 60, natp->nat_btu);
@@ -129,8 +128,8 @@ command(void)
     if (!player->god && !may_play_now(natp, now))
 	return 0;
 
-    if (parse(player->combuf, scanspace, player->argp, player->comtail,
-	      &player->condarg, &redir) < 0) {
+    if (parse(player->combuf, player->argbuf, player->argp,
+	      player->comtail, &player->condarg, &redir) < 0) {
 	pr("See \"info Syntax\"?\n");
     } else {
 	if (dispatch(player->combuf, redir) < 0)
@@ -204,19 +203,6 @@ status(void)
     return 1;
 }
 
-/* Is ARG one of the player's last command's arguments?  */
-static int
-is_command_arg(char *arg)
-{
-    int i;
-
-    for (i = 1; i < 128 && player->argp[i]; i++) {
-	if (arg == player->argp[i])
-	    return 1;
-    }
-    return 0;
-}
-
 /*
  * Make all objects stale if ARG is one of the player's command arguments.
  * See ef_make_stale() for what "making stale" means.
@@ -230,7 +216,8 @@ is_command_arg(char *arg)
 void
 make_stale_if_command_arg(char *arg)
 {
-    if (is_command_arg(arg))
+    if (player->argbuf <= arg
+	&& arg <= player->argbuf + sizeof(player->argbuf))
 	ef_make_stale();
 }
 
@@ -246,7 +233,6 @@ execute(void)
     int failed;
     char *p;			/* UTF-8 */
     char *redir;		/* UTF-8 */
-    char scanspace[1024];
 
     failed = 0;
 
@@ -259,10 +245,10 @@ execute(void)
 
     while (!failed && status()) {
 	player->nstat &= ~EXEC;
-	if (getcommand(buf) < 0)
+	if (getcommand(player->combuf) < 0)
 	    break;
-	if (parse(buf, scanspace, player->argp, player->comtail,
-		  &player->condarg, &redir) < 0) {
+	if (parse(player->combuf, player->argbuf, player->argp,
+		  player->comtail, &player->condarg, &redir) < 0) {
 	    failed = 1;
 	    continue;
 	}
-- 
2.43.0