From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 74239 invoked by alias); 22 Jul 2016 21:21:13 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 74229 invoked by uid 89); 22 Jul 2016 21:21:13 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_PASS autolearn=ham version=3.3.2 spammy=*data, existent, H*r:Symantec, sk:switch_ X-HELO: usplmg20.ericsson.net Received: from inet2.ericy.com (HELO usplmg20.ericsson.net) (198.24.6.130) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Fri, 22 Jul 2016 21:21:11 +0000 Received: from EUSAAHC002.ericsson.se (Unknown_Domain [147.117.188.78]) by usplmg20.ericsson.net (Symantec Mail Security) with SMTP id 42.4D.09012.BD282975; Fri, 22 Jul 2016 22:32:27 +0200 (CEST) Received: from elxcz23q12-y4.dyn.mo.ca.am.ericsson.se (147.117.188.8) by smtps-am.internal.ericsson.com (147.117.188.78) with Microsoft SMTP Server (TLS) id 14.3.294.0; Fri, 22 Jul 2016 17:21:05 -0400 From: Simon Marchi To: CC: Simon Marchi Subject: [PATCH v2] Handle correctly passing a bad interpreter name to new-ui Date: Fri, 22 Jul 2016 21:21:00 -0000 Message-ID: <20160722212059.13580-1-simon.marchi@ericsson.com> MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2016-07/txt/msg00304.txt.bz2 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. --- gdb/testsuite/gdb.base/new-ui.exp | 29 ++++++++++++++++++++++++++++- gdb/top.c | 38 +++++++++++++++++++++++++++++--------- gdb/top.h | 3 +++ 3 files changed, 60 insertions(+), 10 deletions(-) diff --git a/gdb/testsuite/gdb.base/new-ui.exp b/gdb/testsuite/gdb.base/new-ui.exp index f3f66db..22e8b79 100644 --- 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 d566677..36c300e 100644 --- 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 64f7211..bdc3529 100644 --- 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); -- 2.9.2