From patchwork Sun Oct 27 09:15:04 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 99674 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 821EF3858C2B for ; Sun, 27 Oct 2024 09:16:05 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTP id 531BB3858408 for ; Sun, 27 Oct 2024 09:15:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 531BB3858408 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 531BB3858408 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1730020524; cv=none; b=eAtI4meW063t0W0KGX/eUw/vGYfqDaCvAFYdIWbDQKgz6WFG8KP0df28OfVQx1TRJ7sII9ugM4emCzqmTXsXQHBOCvbwt2JyvVNHp/RaEAsHHS9QcpdekPcScqWNXjULpRTtqL0OVeEZxt18n3/evbXVs+xqZqQxzHGI+1nvF7s= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1730020524; c=relaxed/simple; bh=C4d/gDBx5Pw4m4IdTnzMHxu5V4aTkeDQt3iDsbhFbPA=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=CCyosCa4fgmDjp0BHLP8664xuZ3tTMzjkOoIePVjiMZjEIVOlXOhogcgFmWakWkyaY3NL7gY6vUInVt4ZXv7ICvMfDMjv5z0dyXRXGnwRCuo2hPKtoi5GJWchiM2RkG3vcHuQPk4BQM7kST2GGKJguNxvgAJTukB1fjruZ3AjOw= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1730020521; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=frW8wTK4UoP/jPQ8qSceWxV0rN1OjIhaVQsLS4VoG1I=; b=eCA2kigIIGRUN5774ZpwyugNGIYFHjq2oftovfAlYNBOcdcpoLggNL+7TG5p2yFRXNjejT nxuriaUHmXMkj0JFFYtsnhD8E/Voqz8i191kUeYVqtSYFnagLjzqafCl9/nHxTcLSiVyrd 9JW0+K7eTNg3WFt1uUY6Kw8etOW5Sag= Received: from mail-wm1-f69.google.com (mail-wm1-f69.google.com [209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-189-nYcZkgnAP4m0r-N5sH_LSA-1; Sun, 27 Oct 2024 05:15:13 -0400 X-MC-Unique: nYcZkgnAP4m0r-N5sH_LSA-1 Received: by mail-wm1-f69.google.com with SMTP id 5b1f17b1804b1-4316300bb15so24280505e9.2 for ; Sun, 27 Oct 2024 02:15:13 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1730020512; x=1730625312; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=frW8wTK4UoP/jPQ8qSceWxV0rN1OjIhaVQsLS4VoG1I=; b=pOs+LgcRE/7MAV4Dswwd4dhyXejckQ/7DxDAz2GKwF9GnVYeJf4rftxly/Ep3QpJEM rhM4AEshAeQJZ8DvsoG/prLLLNrGUIeRyF4vPWqmtJuoSdaD66m3HKZvCyg0eVMGocNV HonvfJCovoqAeW2nrOJNOuzmA7oo/7aVnMgZzf4ItlN74ul/LcnOf9aQmjq+UO0O/qQb ebdPwgf6afQB/KE/CrVqtXKgAfxcaq4KKjv2C+5jeUwUuzP+XFiiKx5kBXnethA8MKHe MKCgGgyr0JA8AgTBLrkouqVPmlpi412iU9xvTb6vzBKropJRoW1MTb7F07eeBJ9d1jJf S4MQ== X-Gm-Message-State: AOJu0Yx4EpIX3QD0wj9qMHHmflOCXS+NF30EaixYKdlfzagOPmqhWxl7 xpX/YLCNczcwcV4VUxh8I1MdwWWFYt1hg+toQAJ1pqUduEptDiftif3UiN4NURcOGGqZ8tf/1ZP JP1kG8BeSO31J5kJZpEm3CjGVOHseAzvnGpdT9xYsrdLW6IqA88RhQqxzNkzlnIpaXlybvaWpN6 pde7twbaB4DWGNI2hcgH8fVvXM8RDSJ2pBTymdzByQ+ws= X-Received: by 2002:a05:6000:4d:b0:37d:4d72:dca3 with SMTP id ffacd0b85a97d-380611637f8mr3909692f8f.31.1730020511625; Sun, 27 Oct 2024 02:15:11 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGJWWwTjTMJz8YuknpGdthfzzArIyfOrBgVV3cIPHjR2y8y24UmctHi3FbPaVn+mCxL4sxitA== X-Received: by 2002:a05:6000:4d:b0:37d:4d72:dca3 with SMTP id ffacd0b85a97d-380611637f8mr3909665f8f.31.1730020510935; Sun, 27 Oct 2024 02:15:10 -0700 (PDT) Received: from localhost (197.209.200.146.dyn.plus.net. [146.200.209.197]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-38058b3c7f6sm6305645f8f.41.2024.10.27.02.15.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 27 Oct 2024 02:15:10 -0700 (PDT) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCHv2 1/2] gdb: add filename option support Date: Sun, 27 Oct 2024 09:15:04 +0000 Message-Id: X-Mailer: git-send-email 2.25.4 In-Reply-To: References: MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-12.0 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~patchwork=sourceware.org@sourceware.org This commit adds support for filename options to GDB's option sub-system (see cli/cli-option.{c,h}). The new filename options support quoted and escaped filenames, and tab completion is fully supported. This commit adds the new option, and adds these options to the 'maintenance test-options' command as '-filename', along with some tests that exercise this new option. I've split the -filename testing into two. In gdb.base/options.exp we use the -filename option with some arbitrary strings. This tests that GDB can correctly extract the value from a filename option, and that GDB can complete other options after a filename option. However, these tests don't actually pass real filenames, nor do they test filename completion. In gdb.base/filename-completion.exp I have added some tests that test the -filename option with real filenames, and exercise filename tab completion. This commit doesn't include any real uses of the new filename options, that will come in the next commit. --- gdb/cli/cli-option.c | 90 +++++++++++++++++- gdb/cli/cli-option.h | 20 ++++ gdb/maint-test-options.c | 28 ++++-- .../gdb.base/filename-completion.exp | 7 ++ gdb/testsuite/gdb.base/options.exp | 95 +++++++++++++++++-- 5 files changed, 222 insertions(+), 18 deletions(-) diff --git a/gdb/cli/cli-option.c b/gdb/cli/cli-option.c index 05539285c80..9eb9ff81154 100644 --- a/gdb/cli/cli-option.c +++ b/gdb/cli/cli-option.c @@ -43,7 +43,7 @@ union option_value /* For var_enum options. */ const char *enumeration; - /* For var_string options. This is malloc-allocated. */ + /* For var_string and var_filename options. This is allocated with new. */ std::string *string; }; @@ -85,7 +85,7 @@ struct option_def_and_value { if (value.has_value ()) { - if (option.type == var_string) + if (option.type == var_string || option.type == var_filename) delete value->string; } } @@ -102,7 +102,7 @@ struct option_def_and_value { if (value.has_value ()) { - if (option.type == var_string) + if (option.type == var_string || option.type == var_filename) value->string = nullptr; } } @@ -452,6 +452,78 @@ parse_option (gdb::array_view options_group, return option_def_and_value {*match, match_ctx, val}; } + case var_filename: + { + if (check_for_argument (args, "--")) + { + /* Treat e.g., "maint test-options -filename --" as if there + was no argument after "-filename". */ + error (_("-%s requires an argument"), match->name); + } + + const char *arg_start = *args; + std::string str = extract_string_maybe_quoted (args); + + /* If we are performing completion, and extracting STR moved ARGS + to the end of the line, then the user is trying to complete the + filename value. + + If ARGS didn't make it to the end of the line then the filename + value is already complete and the user is trying to complete + something later on the line. */ + if (completion != nullptr && **args == '\0') + { + /* Preserve the current custom word point. If the call to + advance_to_filename_maybe_quoted_complete_word_point below + skips to the end of the command line then the custom word + point will have been updated even though we generate no + completions. + + However, *ARGS will also have been updated, and the general + option completion code (which we will return too) also + updates the custom word point based on the adjustment made + to *ARGS. + + And so, if we don't find any completions, we should restore + the custom word point value, this leaves the generic option + completion code free to make its own adjustments. */ + int prev_word_pt = completion->tracker.custom_word_point (); + + /* From ARG_START move forward to the start of the completion + word, this will skip over any opening quote if there is + one. + + If the word to complete is fully quoted, i.e. has an + opening and closing quote, then this will skip over the + word entirely and leave WORD pointing to the end of the + input string. */ + const char *word + = advance_to_filename_maybe_quoted_complete_word_point + (completion->tracker, arg_start); + + if (word == arg_start || *word != '\0') + { + filename_maybe_quoted_completer (nullptr, completion->tracker, + arg_start, word); + + if (completion->tracker.have_completions ()) + return {}; + } + + /* No completions. Restore the custom word point. See the + comment above for why this is needed. */ + completion->tracker.set_custom_word_point (prev_word_pt); + } + + /* Check we did manage to extract something. */ + if (*args == arg_start) + error (_("-%s requires an argument"), match->name); + + option_value val; + val.string = new std::string (std::move (str)); + return option_def_and_value {*match, match_ctx, val}; + } + default: /* Not yet. */ gdb_assert_not_reached ("option type not supported"); @@ -612,6 +684,7 @@ save_option_value_in_ctx (std::optional &ov) = ov->value->enumeration; break; case var_string: + case var_filename: *ov->option.var_address.string (ov->option, ov->ctx) = std::move (*ov->value->string); break; @@ -701,6 +774,8 @@ get_val_type_str (const option_def &opt, std::string &buffer) } case var_string: return "STRING"; + case var_filename: + return "FILENAME"; default: return nullptr; } @@ -856,6 +931,15 @@ add_setshow_cmds_for_options (command_class cmd_class, nullptr, option.show_cmd_cb, set_list, show_list); } + else if (option.type == var_filename) + { + add_setshow_filename_cmd (option.name, cmd_class, + option.var_address.string (option, data), + option.set_doc, option.show_doc, + option.help_doc, + nullptr, option.show_cmd_cb, + set_list, show_list); + } else gdb_assert_not_reached ("option type not handled"); } diff --git a/gdb/cli/cli-option.h b/gdb/cli/cli-option.h index bbe281d9721..26307a5d1e9 100644 --- a/gdb/cli/cli-option.h +++ b/gdb/cli/cli-option.h @@ -308,6 +308,26 @@ struct string_option_def : option_def } }; +/* A var_filename command line option. */ + +template +struct filename_option_def : option_def +{ + filename_option_def (const char *long_option_, + std::string *(*get_var_address_cb_) (Context *), + show_value_ftype *show_cmd_cb_, + const char *set_doc_, + const char *show_doc_ = nullptr, + const char *help_doc_ = nullptr) + : option_def (long_option_, var_filename, nullptr, + (erased_get_var_address_ftype *) get_var_address_cb_, + show_cmd_cb_, + set_doc_, show_doc_, help_doc_) + { + var_address.string = detail::get_var_address; + } +}; + /* A group of options that all share the same context pointer to pass to the options' get-current-value callbacks. */ struct option_def_group diff --git a/gdb/maint-test-options.c b/gdb/maint-test-options.c index 48b68f91084..9d768177798 100644 --- a/gdb/maint-test-options.c +++ b/gdb/maint-test-options.c @@ -57,12 +57,13 @@ readline, for proper testing of TAB completion. These maintenance commands support options of all the different - available kinds of commands (boolean, enum, flag, string, uinteger): + available kinds of commands (boolean, enum, flag, string, filename, + uinteger): (gdb) maint test-options require-delimiter -[TAB] - -bool -pinteger-unlimited -xx1 - -enum -string -xx2 - -flag -uinteger-unlimited + -bool -flag -uinteger-unlimited + -enum -pinteger-unlimited -xx1 + -filename -string -xx2 (gdb) maint test-options require-delimiter -bool o[TAB] off on @@ -77,14 +78,14 @@ Invoking the commands makes them print out the options parsed: (gdb) maint test-options unknown-is-error -flag -enum yyy cmdarg - -flag 1 -xx1 0 -xx2 0 -bool 0 -enum yyy -uint-unl 0 -pint-unl 0 -string '' -- cmdarg + -flag 1 -xx1 0 -xx2 0 -bool 0 -enum yyy -uint-unl 0 -pint-unl 0 -string '' -filename '' -- cmdarg (gdb) maint test-options require-delimiter -flag -enum yyy cmdarg - -flag 0 -xx1 0 -xx2 0 -bool 0 -enum xxx -uint-unl 0 -pint-unl 0 -string '' -- -flag -enum yyy cmdarg + -flag 0 -xx1 0 -xx2 0 -bool 0 -enum xxx -uint-unl 0 -pint-unl 0 -string '' -filename '' -- -flag -enum yyy cmdarg (gdb) maint test-options require-delimiter -flag -enum yyy cmdarg -- Unrecognized option at: cmdarg -- (gdb) maint test-options require-delimiter -flag -enum yyy -- cmdarg - -flag 1 -xx1 0 -xx2 0 -bool 0 -enum yyy -uint-unl 0 -pint-unl 0 -string '' -- cmdarg + -flag 1 -xx1 0 -xx2 0 -bool 0 -enum yyy -uint-unl 0 -pint-unl 0 -string '' -filename '' -- cmdarg The "maint show test-options-completion-result" command exists in order to do something similar for completion: @@ -135,6 +136,7 @@ struct test_options_opts unsigned int uint_unl_opt = 0; int pint_unl_opt = 0; std::string string_opt; + std::string filename_opt; test_options_opts () = default; @@ -146,7 +148,8 @@ struct test_options_opts { gdb_printf (file, _("-flag %d -xx1 %d -xx2 %d -bool %d " - "-enum %s -uint-unl %s -pint-unl %s -string '%s' -- %s\n"), + "-enum %s -uint-unl %s -pint-unl %s -string '%s' " + "-filename '%s' -- %s\n"), flag_opt, xx1_opt, xx2_opt, @@ -159,6 +162,7 @@ struct test_options_opts ? "unlimited" : plongest (pint_unl_opt)), string_opt.c_str (), + filename_opt.c_str (), args); } }; @@ -233,6 +237,14 @@ static const gdb::option::option_def test_options_option_defs[] = { nullptr, /* show_cmd_cb */ N_("A string option."), }, + + /* A filename option. */ + gdb::option::filename_option_def { + "filename", + [] (test_options_opts *opts) { return &opts->filename_opt; }, + nullptr, /* show_cmd_cb */ + N_("A filename option."), + }, }; /* Create an option_def_group for the test_options_opts options, with diff --git a/gdb/testsuite/gdb.base/filename-completion.exp b/gdb/testsuite/gdb.base/filename-completion.exp index 389e2d736c5..6de312bc6a3 100644 --- a/gdb/testsuite/gdb.base/filename-completion.exp +++ b/gdb/testsuite/gdb.base/filename-completion.exp @@ -412,6 +412,13 @@ proc run_quoting_and_escaping_tests { root } { run_mid_line_completion_tests $root $cmd } + + foreach sub_cmd { require-delimiter unknown-is-error unknown-is-operand } { + set cmd "maintenance test-options $sub_cmd -filename" + with_test_prefix "cmd=$cmd" { + run_quoting_and_escaping_tests_1 $root $cmd + } + } } # Helper for run_unquoted_tests. ROOT is the root directory as setup diff --git a/gdb/testsuite/gdb.base/options.exp b/gdb/testsuite/gdb.base/options.exp index 841e603764c..e1ad61e6470 100644 --- a/gdb/testsuite/gdb.base/options.exp +++ b/gdb/testsuite/gdb.base/options.exp @@ -99,21 +99,21 @@ proc make_cmd {variant} { # operand. proc expect_none {operand} { return "-flag 0 -xx1 0 -xx2 0 -bool 0 -enum xxx -uint-unl 0 -pint-unl 0\ - -string '' -- $operand" + -string '' -filename '' -- $operand" } # Return a string for the expected result of running "maint # test-options xxx", with -flag set. OPERAND is the expected operand. proc expect_flag {operand} { return "-flag 1 -xx1 0 -xx2 0 -bool 0 -enum xxx -uint-unl 0 -pint-unl 0\ - -string '' -- $operand" + -string '' -filename '' -- $operand" } # Return a string for the expected result of running "maint # test-options xxx", with -bool set. OPERAND is the expected operand. proc expect_bool {operand} { return "-flag 0 -xx1 0 -xx2 0 -bool 1 -enum xxx -uint-unl 0 -pint-unl 0\ - -string '' -- $operand" + -string '' -filename '' -- $operand" } # Return a string for the expected result of running "maint @@ -123,10 +123,10 @@ proc expect_bool {operand} { proc expect_integer {option val operand} { if {$option == "uinteger-unlimited"} { return "-flag 0 -xx1 0 -xx2 0 -bool 0 -enum xxx -uint-unl $val\ - -pint-unl 0 -string '' -- $operand" + -pint-unl 0 -string '' -filename '' -- $operand" } elseif {$option == "pinteger-unlimited"} { return "-flag 0 -xx1 0 -xx2 0 -bool 0 -enum xxx -uint-unl 0\ - -pint-unl $val -string '' -- $operand" + -pint-unl $val -string '' -filename '' -- $operand" } else { error "unsupported option: $option" } @@ -144,12 +144,28 @@ proc expect_string {str operand} { set str [string range $str 1 end-1] } return "-flag 0 -xx1 0 -xx2 0 -bool 0 -enum xxx -uint-unl 0 -pint-unl 0\ - -string '$str' -- $operand" + -string '$str' -filename '' -- $operand" +} + +# Return a string for the expected result of running "maint +# test-options xxx", with -filename set to $STR. OPERAND is the +# expected operand. +proc expect_filename {str operand} { + # Dequote the string in the expected output. + if { ( [string range $str 0 0] == "\"" + && [string range $str end end] == "\"") + || ([string range $str 0 0] == "'" + && [string range $str end end] == "'")} { + set str [string range $str 1 end-1] + } + return "-flag 0 -xx1 0 -xx2 0 -bool 0 -enum xxx -uint-unl 0 -pint-unl 0\ + -string '' -filename '$str' -- $operand" } set all_options { "-bool" "-enum" + "-filename" "-flag" "-pinteger-unlimited" "-string" @@ -612,7 +628,7 @@ proc_with_prefix test-flag {variant} { # Extract twice the same flag, separated by one space. gdb_test "$cmd -xx1 -xx2 -xx1 -xx2 -xx1 -- non flags args" \ "-flag 0 -xx1 1 -xx2 1 -bool 0 -enum xxx -uint-unl 0 -pint-unl 0\ - -string '' -- non flags args" + -string '' -filename '' -- non flags args" # Extract 2 known flags in front of unknown flags. gdb_test "$cmd -xx1 -xx2 -a -b -c -xx1 --" \ @@ -1031,6 +1047,70 @@ proc_with_prefix test-string {variant} { } } +# Filename option tests. These tests only focus on how GDB parses the +# filename option, and ensures that GDB can complete things after the +# filename value. The actual strings passed as filenames in this proc +# are not actual files that exist on disk. +# +# Filename options do also support completion. For testing of this +# aspect see the gdb.base/filename-completion.exp script. +proc_with_prefix test-filename {variant} { + global all_options + + set cmd [make_cmd $variant] + + # Check that "-" where a value is expected does not show the + # command's options. I.e., a filename's value is not optional. + # Check both completion and running the command. + res_test_gdb_complete_none \ + "1 [expect_none ""]" \ + "$cmd -filename -" + gdb_test "$cmd -filename --" \ + "-filename requires an argument" + if {$variant == "require-delimiter"} { + gdb_test "$cmd -filename" [expect_none "-filename"] + } else { + gdb_test "$cmd -filename" \ + "-filename requires an argument" + } + + foreach_with_prefix str { + "STR" + "\"STR\"" + "\\\"STR" + "'STR'" + "\\'STR" + "\"STR AAA\"" + "'STR BBB'" + "\"STR 'CCC' DDD\"" + "'STR \"EEE\" FFF'" + "\"STR \\\"GGG\\\" HHH\"" + "'STR \\\'III\\\' JJJ'" + } { + res_test_gdb_complete_none \ + "1 [expect_none ""]" \ + "$cmd -filename ${str}" + gdb_test "$cmd -filename ${str} --" [expect_filename "${str}" ""] + + # Completing at "-" after parsing STR should list all options. + res_test_gdb_complete_multiple \ + "1 [expect_filename "${str}" "-"]" \ + "$cmd -filename ${str} " "-" "" $all_options + + # Check that only $STR is considered part of the filename's value. + # I.e., that we stop parsing the filename at the first + # whitespace or after the closing quote of $STR. + if {$variant == "require-delimiter"} { + res_test_gdb_complete_none \ + "1 [expect_filename "${str}" "BAR"]" \ + "$cmd -filename ${str} BAR" + } else { + res_test_gdb_complete_none "0 BAR" "$cmd -filename ${str} BAR" + } + gdb_test "$cmd -filename ${str} BAR --" "Unrecognized option at: BAR --" + } +} + # Run the options framework tests first. foreach_with_prefix cmd { "require-delimiter" @@ -1045,6 +1125,7 @@ foreach_with_prefix cmd { } test-enum $cmd test-string $cmd + test-filename $cmd } # Run the print integration tests, both as "standalone", and under From patchwork Sun Oct 27 09:15:05 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 99675 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 872813858404 for ; Sun, 27 Oct 2024 09:17:15 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTP id 9DE8B3858C42 for ; Sun, 27 Oct 2024 09:15:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 9DE8B3858C42 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 9DE8B3858C42 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1730020519; cv=none; b=NY5C+H7iyDeFE6539vtgjRAGf+ABgs1iaap+GRuz4a8igQCX1jdEr+4zX6CXs+oXVQ3M8goIfKwkkzR71Cjw3I5/K/qtOyg09bwVrf5O1gAcj0xlvucNL71x/S3VUf6QvDN09QRhwnVFHga73nopgqjyLQJpofdo9NvPska0p2o= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1730020519; c=relaxed/simple; bh=PR4cZrosDHTQAusiousUgY6A41TeSmNZ79JL0iM89bM=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=QXkRD8rkmNHc1d8kjfiWoV+8cGLf6F+Twa8PdPddFtnwoqAMgmn5F/R74DqfbX4QSS+WX6IRakFe/XzWSfKjuV1VWf/l/vjmhdP8C9ulAqsRLBgJeg3si/4xQqrKGzvYeYUB23Y/znaFFHmva3zemTVo/S2w5cr5l50lS3PZl74= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1730020516; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=0rZEbNE1ZmbbN9T1kKvHKXGlYHTmvazoSnHnU/nnN58=; b=Wrka6LKX2fU4U2Q7xGIRaFh5B1d/yli1EkjKHXYNAyYEevIsZ+MOR/DQLihS0u6BL5FdZP qKWN+s700asdMB4yjISU7UIevQf6Dbt5QPyJjXzRz4E77mRr/aBdWT7F3/L7ADRRNTMBPH 1JOdV4982O7x3cEFtwNLonnmgzBnx6M= Received: from mail-wr1-f71.google.com (mail-wr1-f71.google.com [209.85.221.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-107-5UqqwoikPAiAoPJeEl8wLQ-1; Sun, 27 Oct 2024 05:15:14 -0400 X-MC-Unique: 5UqqwoikPAiAoPJeEl8wLQ-1 Received: by mail-wr1-f71.google.com with SMTP id ffacd0b85a97d-37d59ad50f3so1699589f8f.0 for ; Sun, 27 Oct 2024 02:15:14 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1730020513; x=1730625313; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=0rZEbNE1ZmbbN9T1kKvHKXGlYHTmvazoSnHnU/nnN58=; b=j+D3zhevlSBRB/CSImgt+kJIpXzTLNb0ac7NuOcT3OuhNDGUtoZ8w37YmsMb2UiM5H /VpOsV5/sYcsaVTQhe9+MQ6l36LShQGuPKTtkbeOh2du3cN78fmEVAyD62amqe5gYgSV NXJI8W2OduGLzG6zMPlhui07/MWHYLoZA/tP/49wfYDRQYalDj4D0Prap84u4nrUEv1M nAOCev6tGirA4Gu8bqKCAAwc3mziIKsOGFTzCB1iD5eM4r1IrAVkgxMHQVNY+Tcm6Osu Wk7e0Bez0tfz1hpoZgagFXdEDeo8PpJZwink/xqhrCE8YfEXkakvWwJ79JYpsD/Htdw8 WaRQ== X-Gm-Message-State: AOJu0YxzPqce3Qn/aqmrE9XkPNoGMwLVOmdDL+v6OR3mtQzsqzR5nlTg dXKVUfl4Nb9OFW+yCuVpERLoYN2CXwkqMDKrwnZd2xRW6dFL2uFwNdBM1YwrbqyFW0S9EZwyUd+ VWboU+S8tinv269PZgcIXeoSsqCb7w46sDu4FbGPi84KWB2d/8CqBUZbqEn5FJjG43DKplC7eJo RGTNwYKsakMWSzXrhx6UprHntnQSjdZur4QJmyExRddYw= X-Received: by 2002:adf:e3c9:0:b0:37d:4a80:c395 with SMTP id ffacd0b85a97d-3806113ce39mr3591232f8f.21.1730020512788; Sun, 27 Oct 2024 02:15:12 -0700 (PDT) X-Google-Smtp-Source: AGHT+IG8ZH/U9upXE+KKdN3MWvjqHhC8d569hej6T9ll6rY3kIxO3V2NjBwnJxDKrO+ntVCn1JUsaw== X-Received: by 2002:adf:e3c9:0:b0:37d:4a80:c395 with SMTP id ffacd0b85a97d-3806113ce39mr3591200f8f.21.1730020512055; Sun, 27 Oct 2024 02:15:12 -0700 (PDT) Received: from localhost (197.209.200.146.dyn.plus.net. [146.200.209.197]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4318b566f11sm99243765e9.20.2024.10.27.02.15.11 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 27 Oct 2024 02:15:11 -0700 (PDT) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCHv2 2/2] gdb: use option framework for add-inferior and clone-inferior Date: Sun, 27 Oct 2024 09:15:05 +0000 Message-Id: X-Mailer: git-send-email 2.25.4 In-Reply-To: References: MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~patchwork=sourceware.org@sourceware.org Convert the add-inferior and clone-inferior commands to make use of the option framework. This improves the tab completion for these commands. Previously the add-inferior command used a trick to simulate completion of -exec argument. The command use filename completion for everything on the command line, thus you could do: (gdb) add-inferior /path/to/some/fil and GDB would complete the file name, even though add-inferior doesn't really take a filename as an argument. This helped a little though because, if the user did this: (gdb) add-inferior -exec /path/to/some/fil then the file name would be completed. However, GDB didn't really understand the options, so couldn't offer completion of the options themselves. After this commit, the add-inferior command makes use of the recently added gdb::option::filename_option_def feature. This means that the user now has full completion of the option names, and that file names will still complete for the '-exec' option, but will no longer complete if the '-exec' option is not used. I have also converted the clone-inferior command, though this command does not use any file name options. This command does now have proper completion of the command options. --- gdb/inferior.c | 291 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 188 insertions(+), 103 deletions(-) diff --git a/gdb/inferior.c b/gdb/inferior.c index 21f37c8313c..57c8383b0f3 100644 --- a/gdb/inferior.c +++ b/gdb/inferior.c @@ -33,7 +33,7 @@ #include "arch-utils.h" #include "target-descriptions.h" #include "target-connection.h" -#include "readline/tilde.h" +#include "gdbsupport/gdb_tilde_expand.h" #include "progspace-and-thread.h" #include "gdbsupport/buildargv.h" #include "cli/cli-style.h" @@ -891,127 +891,207 @@ switch_to_inferior_and_push_target (inferior *new_inf, gdb_printf (_("Added inferior %d\n"), new_inf->num); } +/* Option values for the "add-inferior" command. */ + +struct add_inferior_opts +{ + /* When true the new inferiors are started without a connection. */ + bool no_connection = false; + + /* The number of new inferiors to add. */ + unsigned int num_copies = 1; + + /* When non-empty, this is the executable for the new inferiors. */ + std::string exec_filename; +}; + +/* Option definitions for the "add-inferior" command. */ + +static const gdb::option::option_def add_inferior_option_defs[] = { + gdb::option::flag_option_def { + "no-connection", + [] (add_inferior_opts *opts) { return &opts->no_connection; }, + N_("\ +If specified, the new inferiors begin with no target connection.\n\ +Without this flag the new inferiors inherit the current inferior's\n\ +connection."), + }, + + gdb::option::uinteger_option_def { + "copies", + [] (add_inferior_opts *opts) { return &opts->num_copies; }, + (show_value_ftype *) nullptr, /* show_cmd_cb */ + N_("\ +The number of inferiors to add. The default is 1."), + }, + + gdb::option::filename_option_def { + "exec", + [] (add_inferior_opts *opts) { return &opts->exec_filename; }, + nullptr, /* show_cmd_cb */ + N_("\ +FILENAME is the file name of the executable to use as the\n\ +main program."), + }, +}; + +/* Create the option_def_group for the "add-inferior" command. */ + +static inline gdb::option::option_def_group +make_add_inferior_options_def_group (add_inferior_opts *opts) +{ + return {{add_inferior_option_defs}, opts}; +} + +/* Completion for the "add-inferior" command. */ + +static void +add_inferior_completer (struct cmd_list_element *cmd, + completion_tracker &tracker, + const char *text, const char * /* word */) +{ + /* The only completion offered is for the command options. */ + const auto group = make_add_inferior_options_def_group (nullptr); + gdb::option::complete_options + (tracker, &text, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_ERROR, group); +} + /* add-inferior [-copies N] [-exec FILENAME] [-no-connection] */ static void add_inferior_command (const char *args, int from_tty) { - int i, copies = 1; - gdb::unique_xmalloc_ptr exec; - symfile_add_flags add_flags = 0; - bool no_connection = false; + add_inferior_opts opts; + const auto group = make_add_inferior_options_def_group (&opts); + gdb::option::process_options + (&args, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_ERROR, group); + + /* If an executable was given then perform tilde expansion. */ + if (!opts.exec_filename.empty ()) + opts.exec_filename = gdb_tilde_expand (opts.exec_filename); + symfile_add_flags add_flags = 0; if (from_tty) add_flags |= SYMFILE_VERBOSE; - if (args) - { - gdb_argv built_argv (args); - - for (char **argv = built_argv.get (); *argv != NULL; argv++) - { - if (**argv == '-') - { - if (strcmp (*argv, "-copies") == 0) - { - ++argv; - if (!*argv) - error (_("No argument to -copies")); - copies = parse_and_eval_long (*argv); - } - else if (strcmp (*argv, "-no-connection") == 0) - no_connection = true; - else if (strcmp (*argv, "-exec") == 0) - { - ++argv; - if (!*argv) - error (_("No argument to -exec")); - exec.reset (tilde_expand (*argv)); - } - } - else - error (_("Invalid argument")); - } - } - inferior *orginf = current_inferior (); scoped_restore_current_pspace_and_thread restore_pspace_thread; - for (i = 0; i < copies; ++i) + for (unsigned int i = 0; i < opts.num_copies; ++i) { inferior *inf = add_inferior_with_spaces (); - switch_to_inferior_and_push_target (inf, no_connection, orginf); + switch_to_inferior_and_push_target (inf, opts.no_connection, orginf); - if (exec != NULL) + if (!opts.exec_filename.empty ()) { - exec_file_attach (exec.get (), from_tty); - symbol_file_add_main (exec.get (), add_flags); + const char *exec = opts.exec_filename.c_str (); + exec_file_attach (exec, from_tty); + symbol_file_add_main (exec, add_flags); } } } -/* clone-inferior [-copies N] [ID] [-no-connection] */ +/* Option values for the "clone-inferior" command. */ + +struct clone_inferior_opts +{ + /* When true the new inferiors are started without a connection. */ + bool no_connection = false; + + /* The number of new inferiors to create by cloning. */ + unsigned int num_copies = 1; +}; + + +/* Option definitions for the "clone-inferior" command. */ + +static const gdb::option::option_def clone_inferior_option_defs[] = { + gdb::option::flag_option_def { + "no-connection", + [] (clone_inferior_opts *opts) { return &opts->no_connection; }, + N_("\ +If specified, the new inferiors begin with no target connection.\n\ +Without this flag the new inferiors to inherit the copied inferior's\n\ +connection."), + }, + + gdb::option::uinteger_option_def { + "copies", + [] (clone_inferior_opts *opts) { return &opts->num_copies; }, + (show_value_ftype *) nullptr, /* show_cmd_cb */ + N_("\ +The number of copies of inferior ID to create. The default is 1."), + }, +}; + +/* Create the option_def_group for the "clone-inferior" command. */ + +static inline gdb::option::option_def_group +make_clone_inferior_options_def_group (clone_inferior_opts *opts) +{ + return {{clone_inferior_option_defs}, opts}; +} + +/* Completion for the "clone-inferior" command. */ + +static void +clone_inferior_completer (struct cmd_list_element *cmd, + completion_tracker &tracker, + const char *text, const char * /* word */) +{ + /* The only completion offered is for the command options. */ + const auto group = make_clone_inferior_options_def_group (nullptr); + gdb::option::complete_options + (tracker, &text, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_OPERAND, group); +} + +/* clone-inferior [-copies N] [-no-connection] [ID] */ static void clone_inferior_command (const char *args, int from_tty) { - int i, copies = 1; - struct inferior *orginf = NULL; - bool no_connection = false; + clone_inferior_opts opts; + const auto group = make_clone_inferior_options_def_group (&opts); + gdb::option::process_options + (&args, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_OPERAND, group); - if (args) + struct inferior *orginf = NULL; + if (args != nullptr && *args != '\0') { - gdb_argv built_argv (args); + gdb_argv argv (args); - char **argv = built_argv.get (); - for (; *argv != NULL; argv++) + gdb_assert (argv.count () > 0); + + for (const char *arg : argv) { - if (**argv == '-') + if (orginf == nullptr) { - if (strcmp (*argv, "-copies") == 0) - { - ++argv; - if (!*argv) - error (_("No argument to -copies")); - copies = parse_and_eval_long (*argv); - - if (copies < 0) - error (_("Invalid copies number")); - } - else if (strcmp (*argv, "-no-connection") == 0) - no_connection = true; + /* The first non-option argument specifies the number of the + inferior to clone. */ + int num = parse_and_eval_long (arg); + orginf = find_inferior_id (num); + + if (orginf == nullptr) + error (_("Inferior ID %d not known."), num); } else - { - if (orginf == NULL) - { - int num; - - /* The first non-option (-) argument specified the - program space ID. */ - num = parse_and_eval_long (*argv); - orginf = find_inferior_id (num); - - if (orginf == NULL) - error (_("Inferior ID %d not known."), num); - continue; - } - else - error (_("Invalid argument")); - } + error (_("Unexpected argument: %s."), arg); } } + else + { + /* If no inferior id was specified, then the user wants to clone the + current inferior. */ + orginf = current_inferior (); + } - /* If no inferior id was specified, then the user wants to clone the - current inferior. */ - if (orginf == NULL) - orginf = current_inferior (); + gdb_assert (orginf != nullptr); scoped_restore_current_pspace_and_thread restore_pspace_thread; - for (i = 0; i < copies; ++i) + for (unsigned int i = 0; i < opts.num_copies; ++i) { struct program_space *pspace; struct inferior *inf; @@ -1025,7 +1105,7 @@ clone_inferior_command (const char *args, int from_tty) inf->aspace = pspace->aspace; inf->set_arch (orginf->arch ()); - switch_to_inferior_and_push_target (inf, no_connection, orginf); + switch_to_inferior_and_push_target (inf, opts.no_connection, orginf); /* If the original inferior had a user specified target description, make the clone use it too. */ @@ -1107,31 +1187,36 @@ Usage: info inferiors [ID]...\n\ If IDs are specified, the list is limited to just those inferiors.\n\ By default all inferiors are displayed.")); - c = add_com ("add-inferior", no_class, add_inferior_command, _("\ + const auto add_inf_opts = make_add_inferior_options_def_group (nullptr); + static std::string add_inferior_command_help + = gdb::option::build_help (_("\ Add a new inferior.\n\ -Usage: add-inferior [-copies N] [-exec FILENAME] [-no-connection]\n\ -N is the optional number of inferiors to add, default is 1.\n\ -FILENAME is the file name of the executable to use\n\ -as main program.\n\ -By default, the new inferior inherits the current inferior's connection.\n\ -If -no-connection is specified, the new inferior begins with\n\ -no target connection yet.")); - set_cmd_completer (c, deprecated_filename_completer); +Usage: add-inferior [-copies NUMBER] [-exec FILENAME] [-no-connection]\n\ +\n\ +Options:\n\ +%OPTIONS%"), add_inf_opts); + c = add_com ("add-inferior", no_class, add_inferior_command, + add_inferior_command_help.c_str ()); + set_cmd_completer_handle_brkchars (c, add_inferior_completer); add_com ("remove-inferiors", no_class, remove_inferior_command, _("\ Remove inferior ID (or list of IDs).\n\ Usage: remove-inferiors ID...")); - add_com ("clone-inferior", no_class, clone_inferior_command, _("\ -Clone inferior ID.\n\ -Usage: clone-inferior [-copies N] [-no-connection] [ID]\n\ -Add N copies of inferior ID. The new inferiors have the same\n\ -executable loaded as the copied inferior. If -copies is not specified,\n\ -adds 1 copy. If ID is not specified, it is the current inferior\n\ -that is cloned.\n\ -By default, the new inferiors inherit the copied inferior's connection.\n\ -If -no-connection is specified, the new inferiors begin with\n\ -no target connection yet.")); + const auto clone_inf_opts = make_clone_inferior_options_def_group (nullptr); + static std::string clone_inferior_command_help + = gdb::option::build_help (_("\ +Clone an existing inferior.\n\ +Usage: clone-inferior [-copies NUMBER] [-no-connection] [ID]\n\ +ID is the inferior number to clone, this can be found with the\n\ +'info inferiors' command. If no ID is specified, then the current\n\ +inferior is cloned.\n\ +\n\ +Options:\n\ +%OPTIONS%"), clone_inf_opts); + c = add_com ("clone-inferior", no_class, clone_inferior_command, + clone_inferior_command_help.c_str ()); + set_cmd_completer_handle_brkchars (c, clone_inferior_completer); add_cmd ("inferiors", class_run, detach_inferior_command, _("\ Detach from inferior ID (or list of IDS).\n\