Re: [PATCH 4/4] GDB/MI: inferior standard I/O redirection

[Pedro Alves <palves@redhat.com>]

(+Vladimir as MI maintainer.)

On 10/05/2015 12:46 PM, Cleber Rosa wrote:

> +@smallexample
> +(gdb)
> +-inferior-stderr-set /dev/null
> +^done
> +(gdb)
> +-inferior-stderr-show
> +^done,inferior_stderr="/dev/null"

(Note that if you want, you're free to use '-', as in 'inferior-stderr',
instead of '_'.  It's actually more common.)

> +(gdb)
> +@end smallexample
> +
>  @subheading The @code{-enable-timings} Command
>  @findex -enable-timings
>  
> diff --git a/gdb/mi/mi-cmd-env.c b/gdb/mi/mi-cmd-env.c
> index fad9297..610070d 100644
> --- a/gdb/mi/mi-cmd-env.c
> +++ b/gdb/mi/mi-cmd-env.c
> @@ -268,6 +268,83 @@ mi_cmd_inferior_tty_show (char *command, char **argv, int argc)
>  			 "inferior_tty_terminal", inferior_io_terminal);
>  }
>  
> +/* Helper function to simplify mi_cmd_inferior_std{in,out,err}_show.  */
> +
> +static void
> +mi_cmd_inferior_std_show_helper(char *command,

Space before parens.  Can 'command' and 'field_name' be const?

> +                                char **argv,
> +                                int argc,
> +                                char *field_name,
> +                                const char *inferior_std_name)
> +{
> +  if ( !mi_valid_noargs ("-inferior-stdin-show", argc, argv))

Spurious space before '!'.

> +  error (_("%s: Usage: No args"), command);
> +
> +  if (inferior_std_name)

  if (inferior_std_name != NULL)


> +    ui_out_field_string (current_uiout,
> +                         field_name, inferior_std_name);
> +}
> +
> +/* Set the inferior stdin file name.  */
> +
> +void
> +mi_cmd_inferior_stdin_set (char *command, char **argv, int argc)
> +{
> +  set_inferior_io_stdin (argv[0]);
> +}
> +
> +/* Print the inferior stdin file name.  */
> +
> +void
> +mi_cmd_inferior_stdin_show (char *command, char **argv, int argc)
> +{
> +  mi_cmd_inferior_std_show_helper("-inferior-stdin-show",

Space before parens.

> +                                  argv,
> +                                  argc,
> +                                  "inferior_stdin",
> +                                  get_inferior_io_stdin ());
> +}
> +
> +/* Set the inferior stdout file name.  */
> +
> +void
> +mi_cmd_inferior_stdout_set (char *command, char **argv, int argc)
> +{
> +  set_inferior_io_stdout (argv[0]);
> +}
> +
> +/* Print the inferior stdout file name.  */
> +
> +void
> +mi_cmd_inferior_stdout_show (char *command, char **argv, int argc)
> +{
> +  mi_cmd_inferior_std_show_helper("-inferior-stdout-show",

Space before parens.

> +                                  argv,
> +                                  argc,
> +				  "inferior_stdout",

Something odd with indentation here.

> +                                  get_inferior_io_stdout ());
> +}
> +
> +/* Set the inferior stderr file name.  */
> +
> +void
> +mi_cmd_inferior_stderr_set (char *command, char **argv, int argc)
> +{
> +  set_inferior_io_stderr (argv[0]);
> +}
> +
> +/* Print the inferior stderr file name.  */
> +
> +void
> +mi_cmd_inferior_stderr_show (char *command, char **argv, int argc)
> +{
> +  mi_cmd_inferior_std_show_helper("-inferior-stderr-show",

Space before parens.

> +                                  argv,
> +                                  argc,
> +                                  "inferior_stderr",
> +                                  get_inferior_io_stderr ());
> +}
> +
>  void 
>  _initialize_mi_cmd_env (void)
>  {
> diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
> index 2d8af2f..388b5b1 100644
> --- a/gdb/mi/mi-cmds.c
> +++ b/gdb/mi/mi-cmds.c
> @@ -123,6 +123,12 @@ static struct mi_cmd mi_cmds[] =
>    DEF_MI_CMD_CLI ("gdb-version", "show version", 0),
>    DEF_MI_CMD_MI ("inferior-tty-set", mi_cmd_inferior_tty_set),
>    DEF_MI_CMD_MI ("inferior-tty-show", mi_cmd_inferior_tty_show),
> +  DEF_MI_CMD_MI ("inferior-stdin-set", mi_cmd_inferior_stdin_set),
> +  DEF_MI_CMD_MI ("inferior-stdin-show", mi_cmd_inferior_stdin_show),
> +  DEF_MI_CMD_MI ("inferior-stdout-set", mi_cmd_inferior_stdout_set),
> +  DEF_MI_CMD_MI ("inferior-stdout-show", mi_cmd_inferior_stdout_show),
> +  DEF_MI_CMD_MI ("inferior-stderr-set", mi_cmd_inferior_stderr_set),
> +  DEF_MI_CMD_MI ("inferior-stderr-show", mi_cmd_inferior_stderr_show),
>    DEF_MI_CMD_MI ("info-ada-exceptions", mi_cmd_info_ada_exceptions),
>    DEF_MI_CMD_MI ("info-gdb-mi-command", mi_cmd_info_gdb_mi_command),
>    DEF_MI_CMD_MI ("info-os", mi_cmd_info_os),
> diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h
> index 55141f3..8ffb65f 100644
> --- a/gdb/mi/mi-cmds.h
> +++ b/gdb/mi/mi-cmds.h
> @@ -73,6 +73,12 @@ extern mi_cmd_argv_ftype mi_cmd_file_list_exec_source_files;
>  extern mi_cmd_argv_ftype mi_cmd_gdb_exit;
>  extern mi_cmd_argv_ftype mi_cmd_inferior_tty_set;
>  extern mi_cmd_argv_ftype mi_cmd_inferior_tty_show;
> +extern mi_cmd_argv_ftype mi_cmd_inferior_stdin_set;
> +extern mi_cmd_argv_ftype mi_cmd_inferior_stdin_show;
> +extern mi_cmd_argv_ftype mi_cmd_inferior_stdout_set;
> +extern mi_cmd_argv_ftype mi_cmd_inferior_stdout_show;
> +extern mi_cmd_argv_ftype mi_cmd_inferior_stderr_set;
> +extern mi_cmd_argv_ftype mi_cmd_inferior_stderr_show;
>  extern mi_cmd_argv_ftype mi_cmd_info_ada_exceptions;
>  extern mi_cmd_argv_ftype mi_cmd_info_gdb_mi_command;
>  extern mi_cmd_argv_ftype mi_cmd_info_os;
> diff --git a/gdb/testsuite/gdb.mi/mi-basics.exp b/gdb/testsuite/gdb.mi/mi-basics.exp
> index 476dbdc..89f7d68 100644
> --- a/gdb/testsuite/gdb.mi/mi-basics.exp
> +++ b/gdb/testsuite/gdb.mi/mi-basics.exp
> @@ -264,6 +264,67 @@ proc test_setshow_inferior_tty {} {
>  		"make sure show takes no arguments"
>  }
>  
> +proc test_setshow_inferior_std {} {
> +    global mi_gdb_prompt
> +
> +    # Test that the commands,
> +    #   -inferior-stdin-set
> +    #   -inferior-stdin-show
> +    #   -inferior-stdin-set
> +    #   -inferior-stdin-show
> +    #   -inferior-stdin-set
> +    #   -inferior-stdin-show
> +    # are setting/getting the same data in GDB.
> +
> +    mi_gdb_test "301-inferior-stdin-show" \
> +		"301\\\^done" \
> +		"initial stdin is not set"
> +
> +    mi_gdb_test "302-inferior-stdin-set /foo/bar" \
> +		"302\\\^done" \
> +		"set stdin to /foo/bar"
> +
> +    mi_gdb_test "303-inferior-stdin-show" \
> +		"303\\\^done,inferior_stdin=\"/foo/bar\"" \
> +		"std was set correctly"
> +
> +    mi_gdb_test "304-inferior-stdin-show should-take-no-args" \
> +		"304\\\^error,msg=\"-inferior-stdin-show: Usage: No args\"" \
> +		"make sure show takes no arguments"
> +
> +    mi_gdb_test "305-inferior-stdout-show" \
> +		"305\\\^done" \
> +		"initial stdout is not set"
> +
> +    mi_gdb_test "306-inferior-stdout-set /foo/bar" \
> +		"306\\\^done" \
> +		"set stdout to /foo/bar"
> +
> +    mi_gdb_test "307-inferior-stdout-show" \
> +		"307\\\^done,inferior_stdout=\"/foo/bar\"" \
> +		"std was set correctly"
> +
> +    mi_gdb_test "308-inferior-stdout-show should-take-no-args" \
> +		"308\\\^error,msg=\"-inferior-stdout-show: Usage: No args\"" \
> +		"make sure show takes no arguments"

Please make sure there are no duplicate test messages:

  https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Make_sure_test_messages_are_unique

> +
> +    mi_gdb_test "309-inferior-stderr-show" \
> +                "309\\\^done" \
> +                "initial stderr is not set"
> +
> +    mi_gdb_test "310-inferior-stderr-set /foo/bar" \
> +                "310\\\^done" \
> +                "set stderr to /foo/bar"
> +
> +    mi_gdb_test "311-inferior-stderr-show" \
> +                "311\\\^done,inferior_stderr=\"/foo/bar\"" \
> +                "std was set correctly"
> +
> +    mi_gdb_test "312-inferior-stderr-show should-take-no-args" \
> +                "312\\\^error,msg=\"-inferior-stderr-show: Usage: No args\"" \
> +                "make sure show takes no arguments"
> +}
> +
>  if { [test_mi_interpreter_selection]
>        && [test_exec_and_symbol_mi_operatons] } {
>    test_breakpoints_deletion
> @@ -271,6 +332,7 @@ if { [test_mi_interpreter_selection]
>    test_cwd_specification
>    test_path_specification
>    test_setshow_inferior_tty
> +  test_setshow_inferior_std
>  }
>  
>  mi_gdb_exit
> 

Looks good to me with the nits above fixed.

Thanks,
Pedro Alves