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.
This commit is contained in:
Markus Armbruster 2011-07-03 09:53:39 +02:00
parent 36015e8c0e
commit 3de1e8be28
3 changed files with 9 additions and 23 deletions

View file

@ -68,6 +68,7 @@ struct player {
struct cmndstr *command; /* currently executing command */ struct cmndstr *command; /* currently executing command */
struct iop *iop; struct iop *iop;
char combuf[1024]; /* command input buffer, UTF-8 */ char combuf[1024]; /* command input buffer, UTF-8 */
char argbuf[1024]; /* argument buffer, ASCII */
char *argp[128]; /* arguments, ASCII, valid if command */ char *argp[128]; /* arguments, ASCII, valid if command */
char *condarg; /* conditional, ASCII, valid if command */ char *condarg; /* conditional, ASCII, valid if command */
char *comtail[128]; /* start of args in combuf[] */ char *comtail[128]; /* start of args in combuf[] */

View file

@ -76,7 +76,6 @@ do_unit_move(struct emp_qelem *ulist, int *together,
int skip = 0; int skip = 0;
char buf[1024]; char buf[1024];
char prompt[128]; char prompt[128];
char scanspace[1024];
char pathtaken[1024]; /* Doubtful we'll have a path longer than this */ char pathtaken[1024]; /* Doubtful we'll have a path longer than this */
char *pt = pathtaken; char *pt = pathtaken;
char bmap_flag; char bmap_flag;
@ -176,7 +175,7 @@ do_unit_move(struct emp_qelem *ulist, int *together,
cp++; cp++;
continue; continue;
} }
ac = parse(cp, scanspace, player->argp, NULL, NULL, NULL); ac = parse(cp, player->argbuf, player->argp, NULL, NULL, NULL);
if (ac <= 0) { if (ac <= 0) {
player->argp[0] = ""; player->argp[0] = "";
cp = NULL; cp = NULL;

View file

@ -117,7 +117,6 @@ command(void)
{ {
struct natstr *natp = getnatp(player->cnum); struct natstr *natp = getnatp(player->cnum);
char *redir; /* UTF-8 */ char *redir; /* UTF-8 */
char scanspace[1024];
time_t now; time_t now;
prprompt(natp->nat_timeused / 60, natp->nat_btu); prprompt(natp->nat_timeused / 60, natp->nat_btu);
@ -129,8 +128,8 @@ command(void)
if (!player->god && !may_play_now(natp, now)) if (!player->god && !may_play_now(natp, now))
return 0; return 0;
if (parse(player->combuf, scanspace, player->argp, player->comtail, if (parse(player->combuf, player->argbuf, player->argp,
&player->condarg, &redir) < 0) { player->comtail, &player->condarg, &redir) < 0) {
pr("See \"info Syntax\"?\n"); pr("See \"info Syntax\"?\n");
} else { } else {
if (dispatch(player->combuf, redir) < 0) 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. * Make all objects stale if ARG is one of the player's command arguments.
* See ef_make_stale() for what "making stale" means. * See ef_make_stale() for what "making stale" means.
@ -230,7 +216,8 @@ is_command_arg(char *arg)
void void
make_stale_if_command_arg(char *arg) 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; int failed;
char *p; /* UTF-8 */ char *p; /* UTF-8 */
char *redir; /* UTF-8 */ char *redir; /* UTF-8 */
char scanspace[1024];
failed = 0; failed = 0;
@ -259,10 +245,10 @@ execute(void)
while (!failed && status()) { while (!failed && status()) {
player->nstat &= ~EXEC; player->nstat &= ~EXEC;
if (getcommand(buf) < 0) if (getcommand(player->combuf) < 0)
break; break;
if (parse(buf, scanspace, player->argp, player->comtail, if (parse(player->combuf, player->argbuf, player->argp,
&player->condarg, &redir) < 0) { player->comtail, &player->condarg, &redir) < 0) {
failed = 1; failed = 1;
continue; continue;
} }