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.

diff --git a/include/player.h b/include/player.h
index 0997d12c29e1999ebe666daee536980eb16e549d..afe0ec9c2fcf80a35a68815776c35f75c7668c6e 100644 (file)
--- 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 */
     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[] */
     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 fec4bb8c0c75fa901d7e80c304d8a07e3960a119..cc5d51dbded17d3cf380a9c48f3b38db2fb47d0f 100644 (file)
--- 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];
     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;
     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;
        }
            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;
        if (ac <= 0) {
            player->argp[0] = "";
            cp = NULL;

diff --git a/src/lib/player/player.c b/src/lib/player/player.c
index 9b8d37a417872519d873de346a89e37fd38762fb..6d8571d8a28679c6a64c29e47cf2a70b2ceed643 100644 (file)
--- 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 */
 {
     struct natstr *natp = getnatp(player->cnum);
     char *redir;               /* UTF-8 */

-    char scanspace[1024];

     time_t now;
 
     prprompt(natp->nat_timeused / 60, natp->nat_btu);
     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 (!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)
        pr("See \"info Syntax\"?\n");
     } else {
        if (dispatch(player->combuf, redir) < 0)


@@ -204,19 +203,6 @@ status(void)
     return 1;
 }
 
     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.
 /*
  * 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)
 {
 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();
 }
 
        ef_make_stale();
 }
 


@@ -246,7 +233,6 @@ execute(void)
     int failed;
     char *p;                   /* UTF-8 */
     char *redir;               /* UTF-8 */
     int failed;
     char *p;                   /* UTF-8 */
     char *redir;               /* UTF-8 */

-    char scanspace[1024];

 
     failed = 0;
 
 
     failed = 0;
 


@@ -259,10 +245,10 @@ execute(void)
 
     while (!failed && status()) {
        player->nstat &= ~EXEC;
 
     while (!failed && status()) {
        player->nstat &= ~EXEC;

-       if (getcommand(buf) < 0)
+       if (getcommand(player->combuf) < 0)

            break;
            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;
        }
            failed = 1;
            continue;
        }