RE: MI testsuite to use PTY for inferior

[Nick Roberts <nickrob@snap.net.nz>]

Bob,

I've looked at your patch and have some comments.  I'm not very familiar
with expect so some of what I say might be wrong.


> +# INFERIOR_PTY should be set to TRUE to have the inferior work with it's
> +# own PTY. If set to FALSE, the inferior shares GDB's PTY.
>  #
>  # When running over NFS, particularly if running many simultaneous
>  # tests on different hosts all using the same server, things can
>  # get really slow.  Give gdb at least 3 minutes to start up.
>  #
> -proc mi_gdb_start { } {
> +proc mi_gdb_start { args } {
>      global verbose
>      global GDB
>      global GDBFLAGS
> @@ -99,6 +105,10 @@
>  
>      gdb_stop_suppressing_tests;
>  
> +    if { [llength $args] == 1} {
> +	set inferior_pty [lindex $args 0]
> +    }
> +
>      # Start SID.
>      if { [info procs sid_start] != "" } {
>  	verbose "Spawning SID"
> @@ -117,6 +127,16 @@
>  	    exit 1
>  	}
>      }
> +
> +    # spawn off the new pty for the inferior process

Open a new pty for the inferior process? (spawn means new process?)

> +    if { [ info exists inferior_pty ] } {
> +	spawn -pty
> +	global mi_inferior_spawn_id
> +	global mi_inferior_tty_name
> +	set mi_inferior_spawn_id $spawn_id
> +	set mi_inferior_tty_name $spawn_out(slave,name)
> +    }
> +

This seems to try to open a new pty even if inferior_pty is false.  Shouldn't
the condition, here and elsewhere, be:

if { $inferior_pty } {

> -# mi_gdb_test COMMAND PATTERN MESSAGE -- send a command to gdb; test the result.
> +# mi_gdb_test COMMAND PATTERN IPATTERN MESSAGE -- send a command to gdb; test the result.
>  #
>  # COMMAND is the command to execute, send to GDB with send_gdb.  If
>  #   this is the null string no command is sent.
>  # PATTERN is the pattern to match for a PASS, and must NOT include
>  #   the \r\n sequence immediately before the gdb prompt.
> +# IPATTERN is the pattern to match for the inferior's output. This will not 
> +#   produce a PASS if successfull, but will produce a FAIL if unsuccessful.
>  # MESSAGE is an optional message to be printed.  If this is
>  #   omitted, then the pass/fail messages use the command string as the
>  #   message.  (If this is the empty string, then sometimes we don't
> @@ -533,14 +568,20 @@

With these arguments, you can't discriminate between

mi_gdb_test COMMAND PATTERN MESSAGE

and

mi_gdb_test COMMAND PATTERN IPATTERN

(MESSAGE is optional).

Re your ChangeLog entry,

> +	* gdb.mi/mi-console.exp: Ditto.
> +	(47-exec-next): Use mi_gdb_test to get GDB and Inferior output.

47-exec-next isn't a procedure.  I would just do:

* gdb.mi/mi-console.exp: Use mi_gdb_test to get GDB and Inferior output.

I know that "Ditto." and "Likewise." are used often in GDB but I prefer:

	* gdb.mi/gdb669.exp, gdb.mi/gdb680.exp, gdb.mi/gdb701.exp,
	gdb.mi/gdb792.exp, gdb.mi/mi-break.exp, gdb.mi/mi-disassemble.exp,
	gdb.mi/mi-eval.exp: Tell mi_gdb_start to use a PTY for inferior.
	Update Copyright.

I can't really comment yet on the essence of what you are doing yet, but it
looks sound to me.

Nick