Handle correctly passing a bad interpreter name to new-ui

When a bad interpreter name is passed to new-ui, such as:

  (gdb)  new-ui bloop /dev/pts/10

A partially created UI is left in the UI list, with interp set to NULL.
Trying to do anything that will print on this UI (such as "start") will
cause a segmentation fault.

Changes in v2:

  - Use with_test_prefix to namespace test procedures
  - Give an explicit stable test name
  - Add a "bad terminal path" test
  - Remove useless runto_main
  - Add missing intro comments

I did not factor out the pty spawn, as there is some magic involved I
don't quite understand.  But it wouldn't bring that much anyway.

gdb/ChangeLog:

	* top.h (make_delete_ui_cleanup): New declaration.
	* top.c (delete_ui_cleanup): New function.
	(make_delete_ui_cleanup): New function.
	(new_ui_command): Create restore_ui cleanup earlier, create a
	delete_ui cleanup and discard it on success.

gdb/testsuite/ChangeLog:

	* gdb.base/new-ui.exp (do_test_invalid_args): New
	procedure.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index c346a90b0f2e9a7e89b3aa5a669f38da48b0251e..394018844d9db71ad248158a7f38bd6a66573ff4 100644 (file)
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,11 @@
+2016-07-25  Simon Marchi  <simon.marchi@ericsson.com>
+
+       * top.h (make_delete_ui_cleanup): New declaration.
+       * top.c (delete_ui_cleanup): New function.
+       (make_delete_ui_cleanup): New function.
+       (new_ui_command): Create restore_ui cleanup earlier, create a
+       delete_ui cleanup and discard it on success.
+
 2016-07-25  Pedro Alves  <palves@redhat.com>
            Jan Kratochvil  <jan.kratochvil@redhat.com>
 

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 48e2eeba565db0d2fc18ba55bbf1a00d5b99db53..609d2625fd9842f1a3e998c69428ed243cc72e1b 100644 (file)
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2016-07-25  Simon Marchi  <simon.marchi@ericsson.com>
+
+       * gdb.base/new-ui.exp (do_test_invalid_args): New
+       procedure.
+
 2016-07-25  Tim Wiederhake  <tim.wiederhake@intel.com>
 
        * gdb.btrace/reconnect.c: New file.

diff --git a/gdb/testsuite/gdb.base/new-ui.exp b/gdb/testsuite/gdb.base/new-ui.exp
index f3f66db47fe39bcb1c6eb6aef574e1b2096010ec..22e8b79151de96899ecc1036825fc6fcb4d2ba9a 100644 (file)
--- a/gdb/testsuite/gdb.base/new-ui.exp
+++ b/gdb/testsuite/gdb.base/new-ui.exp
@@ -143,4 +143,31 @@ proc do_test {} {
     }
 }
 
-do_test
+# Test missing / invalid arguments.
+
+proc do_test_invalid_args {} {
+    global testfile
+
+    clean_restart $testfile
+
+    spawn -pty
+    set extra_tty_name $spawn_out(slave,name)
+
+    # Test bad terminal path.
+    gdb_test "new-ui console /non/existent/path" \
+            "opening terminal failed: No such file or directory\." \
+            "new-ui with bad terminal path"
+
+    # Test bad interpreter name.
+    gdb_test "new-ui bloop $extra_tty_name" \
+            "Interpreter `bloop' unrecognized" \
+            "new-ui with bad interpreter name"
+
+    # Test that we can continue working normally.
+    if ![runto_main] {
+       fail "could not run to main"
+    }
+}
+
+with_test_prefix "do_test" do_test
+with_test_prefix "do_test_invalid_args" do_test_invalid_args

diff --git a/gdb/top.c b/gdb/top.c
index d56667787330e75fbfd091627e00c81c18c99427..36c300e8ce1f2f36e274f32d3d38b285bbde1605 100644 (file)
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -324,6 +324,24 @@ delete_ui (struct ui *todel)
   free_ui (ui);
 }
 
+/* Cleanup that deletes a UI.  */
+
+static void
+delete_ui_cleanup (void *void_ui)
+{
+  struct ui *ui = (struct ui *) void_ui;
+
+  delete_ui (ui);
+}
+
+/* See top.h.  */
+
+struct cleanup *
+make_delete_ui_cleanup (struct ui *ui)
+{
+  return make_cleanup (delete_ui_cleanup, ui);
+}
+
 /* Open file named NAME for read/write, making sure not to make it the
    controlling terminal.  */
 
@@ -353,13 +371,13 @@ new_ui_command (char *args, int from_tty)
   char **argv;
   const char *interpreter_name;
   const char *tty_name;
-  struct cleanup *back_to;
-  struct cleanup *streams_chain;
+  struct cleanup *success_chain;
+  struct cleanup *failure_chain;
 
   dont_repeat ();
 
   argv = gdb_buildargv (args);
-  back_to = make_cleanup_freeargv (argv);
+  success_chain = make_cleanup_freeargv (argv);
   argc = countargv (argv);
 
   if (argc < 2)
@@ -368,7 +386,9 @@ new_ui_command (char *args, int from_tty)
   interpreter_name = argv[0];
   tty_name = argv[1];
 
-  streams_chain = make_cleanup (null_cleanup, NULL);
+  make_cleanup (restore_ui_cleanup, current_ui);
+
+  failure_chain = make_cleanup (null_cleanup, NULL);
 
   /* Open specified terminal, once for each of
      stdin/stdout/stderr.  */
@@ -379,20 +399,20 @@ new_ui_command (char *args, int from_tty)
     }
 
   ui = new_ui (stream[0], stream[1], stream[2]);
-
-  discard_cleanups (streams_chain);
+  make_cleanup (delete_ui_cleanup, ui);
 
   ui->async = 1;
 
-  make_cleanup (restore_ui_cleanup, current_ui);
   current_ui = ui;
 
   set_top_level_interpreter (interpreter_name);
 
   interp_pre_command_loop (top_level_interpreter ());
 
-  /* This restores the previous UI.  */
-  do_cleanups (back_to);
+  discard_cleanups (failure_chain);
+
+  /* This restores the previous UI and frees argv.  */
+  do_cleanups (success_chain);
 
   printf_unfiltered ("New UI allocated\n");
 }

diff --git a/gdb/top.h b/gdb/top.h
index 64f7211650183a46a099dd47f4029ba6c3d39426..bdc3529ab136d0a90ed79eaef76809cb3a73d7ea 100644 (file)
--- a/gdb/top.h
+++ b/gdb/top.h
@@ -185,6 +185,9 @@ extern void switch_thru_all_uis_next (struct switch_thru_all_uis *state);
 extern struct ui *new_ui (FILE *instream, FILE *outstream, FILE *errstream);
 extern void delete_ui (struct ui *todel);
 
+/* Cleanup that deletes a UI.  */
+extern struct cleanup *make_delete_ui_cleanup (struct ui *ui);
+
 /* Cleanup that restores the current UI.  */
 extern void restore_ui_cleanup (void *data);