Re: RFC: remote-sim.c -vs- immediate_quit

[Pedro Alves <palves at redhat dot com>]

On 07/30/2012 04:14 PM, Tom Tromey wrote:
> I would appreciate comments on this patch.
> 
> I happened to be looking at immediate_quit again today, and I noticed
> the use in remote-sim.c:
> 
>     static int
>     gdb_os_poll_quit (host_callback *p)
>     {
>       if (deprecated_ui_loop_hook != NULL)
>         deprecated_ui_loop_hook (0);
> 
>       if (quit_flag)		/* gdb's idea of quit */
>         {
>           quit_flag = 0;		/* we've stolen it */
>           return 1;
>         }
>       else if (immediate_quit)
>         {
>           return 1;
>         }
>       return 0;
>     }
> 
> I think this is just wrong... it appears to be treating immediate_quit
> as if it were some kind of synonym for quit_flag -- but this is
> backward, as immediate_quit affects the operation of the SIGINT handler,
> and is not a consequence of it.

That line was added in 4.17:

https://github.com/palves/gdb-old-releases/blob/66364a69b462ce859a14cef45a0a9e39aa23441d/gdb/utils.c

along with all of remote-sim.c, actually.

Note that that gdb_os_poll_quit is only called from within sim/sim-io.c, while
the simulation is ongoing, and that the regular sigint handler is replaced while the
sim is running, by gdbsim_cntrl_c.

At the time, gdb_os_poll_quit had a notice_quit call:

/* GDB version of os_poll_quit callback.
   Taken from gdb/util.c - should be in a library */

static int
gdb_os_poll_quit (p)
     host_callback *p;
{
  notice_quit ();
  if (quit_flag) /* gdb's idea of quit */
    {
      quit_flag = 0; /* we've stolen it */
      return 1;
    }
  else if (immediate_quit)
    {
      return 1;
    }
  return 0;
}


and notice_quit was defined like:

#if defined(__GO32__)

/* In the absence of signals, poll keyboard for a quit.
   Called from #define QUIT pollquit() in xm-go32.h. */

void
notice_quit()
{
  if (kbhit ())
    switch (getkey ())
      {
      case 1:
	quit_flag = 1;
	break;
      case 2:
	immediate_quit = 2;
	break;
      default:
	/* We just ignore it */
	/* FIXME!! Don't think this actually works! */
	fprintf_unfiltered (gdb_stderr, "CTRL-A to quit, CTRL-B to quit harder\n");
	break;
      }
}

#elif defined(_MSC_VER) /* should test for wingdb instead? */

/*
 * Windows translates all keyboard and mouse events
 * into a message which is appended to the message
 * queue for the process.
 */

void notice_quit()
{
  int k = win32pollquit();
  if (k == 1)
    quit_flag = 1;
  else if (k == 2)
    immediate_quit = 1;
}

#else /* !defined(__GO32__) && !defined(_MSC_VER) */

void notice_quit()
{
  /* Done by signals */
}


So looks like that immediate_quit check is checking if notice_quit had
set immediate_quit, for MS-DOS and Windows.


> I propose the appended as a fix.
> 
> What do you think?

Looks right to me.

-- 
Pedro Alves