public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Filename option support
@ 2024-10-01 12:49 Andrew Burgess
  2024-10-01 12:49 ` [PATCH 1/2] gdb: add filename " Andrew Burgess
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Andrew Burgess @ 2024-10-01 12:49 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess

Finally!  After the recent filename completion changes, this
mini-series adds filename option support to GDB.  I know there are
other commands which can potentially make use of the new filename
options, but for now I've just updated add-inferior as an example use
case.

---

Andrew Burgess (2):
  gdb: add filename option support
  gdb: use option framework for add-inferior and clone-inferior

 gdb/cli/cli-option.c                          |  90 +++++-
 gdb/cli/cli-option.h                          |  20 ++
 gdb/inferior.c                                | 291 +++++++++++-------
 gdb/maint-test-options.c                      |  28 +-
 .../gdb.base/filename-completion.exp          |   7 +
 gdb/testsuite/gdb.base/options.exp            |  95 +++++-
 6 files changed, 410 insertions(+), 121 deletions(-)


base-commit: 4339a3ffc39af599305bd992536dd379ac8390ca
-- 
2.25.4


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] gdb: add filename option support
  2024-10-01 12:49 [PATCH 0/2] Filename option support Andrew Burgess
@ 2024-10-01 12:49 ` Andrew Burgess
  2024-10-01 12:49 ` [PATCH 2/2] gdb: use option framework for add-inferior and clone-inferior Andrew Burgess
  2024-10-27  9:15 ` [PATCHv2 0/2] Filename option support Andrew Burgess
  2 siblings, 0 replies; 7+ messages in thread
From: Andrew Burgess @ 2024-10-01 12:49 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess

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<const option_def_group> 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<option_def_and_value> &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<typename Context>
+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<std::string, Context>;
+  }
+};
+
 /* 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<test_options_opts> {
+    "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 85fac35d7c8..a99781965e7 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
-- 
2.25.4


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/2] gdb: use option framework for add-inferior and clone-inferior
  2024-10-01 12:49 [PATCH 0/2] Filename option support Andrew Burgess
  2024-10-01 12:49 ` [PATCH 1/2] gdb: add filename " Andrew Burgess
@ 2024-10-01 12:49 ` Andrew Burgess
  2024-10-27  9:15 ` [PATCHv2 0/2] Filename option support Andrew Burgess
  2 siblings, 0 replies; 7+ messages in thread
From: Andrew Burgess @ 2024-10-01 12:49 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess

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<TAB>

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<TAB>

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<add_inferior_opts> {
+    "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<add_inferior_opts> {
+    "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<add_inferior_opts> {
+    "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<char> 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<clone_inferior_opts> {
+    "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<clone_inferior_opts> {
+    "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\
-- 
2.25.4


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCHv2 0/2] Filename option support
  2024-10-01 12:49 [PATCH 0/2] Filename option support Andrew Burgess
  2024-10-01 12:49 ` [PATCH 1/2] gdb: add filename " Andrew Burgess
  2024-10-01 12:49 ` [PATCH 2/2] gdb: use option framework for add-inferior and clone-inferior Andrew Burgess
@ 2024-10-27  9:15 ` Andrew Burgess
  2024-10-27  9:15   ` [PATCHv2 1/2] gdb: add filename " Andrew Burgess
                     ` (2 more replies)
  2 siblings, 3 replies; 7+ messages in thread
From: Andrew Burgess @ 2024-10-27  9:15 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess

In v2:

  - Rebased to current HEAD.

  - Retested.

---

Finally!  After the recent filename completion changes, this
mini-series adds filename option support to GDB.  I know there are
other commands which can potentially make use of the new filename
options, but for now I've just updated add-inferior as an example use
case.

---

Andrew Burgess (2):
  gdb: add filename option support
  gdb: use option framework for add-inferior and clone-inferior

 gdb/cli/cli-option.c                          |  90 +++++-
 gdb/cli/cli-option.h                          |  20 ++
 gdb/inferior.c                                | 291 +++++++++++-------
 gdb/maint-test-options.c                      |  28 +-
 .../gdb.base/filename-completion.exp          |   7 +
 gdb/testsuite/gdb.base/options.exp            |  95 +++++-
 6 files changed, 410 insertions(+), 121 deletions(-)


base-commit: a723c56efb07c4f8b3f6a3ed4b878a2f8f5572cc
-- 
2.25.4


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCHv2 1/2] gdb: add filename option support
  2024-10-27  9:15 ` [PATCHv2 0/2] Filename option support Andrew Burgess
@ 2024-10-27  9:15   ` Andrew Burgess
  2024-10-27  9:15   ` [PATCHv2 2/2] gdb: use option framework for add-inferior and clone-inferior Andrew Burgess
  2024-11-04 16:14   ` [PATCHv2 0/2] Filename option support Andrew Burgess
  2 siblings, 0 replies; 7+ messages in thread
From: Andrew Burgess @ 2024-10-27  9:15 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess

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<const option_def_group> 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<option_def_and_value> &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<typename Context>
+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<std::string, Context>;
+  }
+};
+
 /* 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<test_options_opts> {
+    "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
-- 
2.25.4


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCHv2 2/2] gdb: use option framework for add-inferior and clone-inferior
  2024-10-27  9:15 ` [PATCHv2 0/2] Filename option support Andrew Burgess
  2024-10-27  9:15   ` [PATCHv2 1/2] gdb: add filename " Andrew Burgess
@ 2024-10-27  9:15   ` Andrew Burgess
  2024-11-04 16:14   ` [PATCHv2 0/2] Filename option support Andrew Burgess
  2 siblings, 0 replies; 7+ messages in thread
From: Andrew Burgess @ 2024-10-27  9:15 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess

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<TAB>

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<TAB>

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<add_inferior_opts> {
+    "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<add_inferior_opts> {
+    "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<add_inferior_opts> {
+    "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<char> 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<clone_inferior_opts> {
+    "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<clone_inferior_opts> {
+    "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\
-- 
2.25.4


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCHv2 0/2] Filename option support
  2024-10-27  9:15 ` [PATCHv2 0/2] Filename option support Andrew Burgess
  2024-10-27  9:15   ` [PATCHv2 1/2] gdb: add filename " Andrew Burgess
  2024-10-27  9:15   ` [PATCHv2 2/2] gdb: use option framework for add-inferior and clone-inferior Andrew Burgess
@ 2024-11-04 16:14   ` Andrew Burgess
  2 siblings, 0 replies; 7+ messages in thread
From: Andrew Burgess @ 2024-11-04 16:14 UTC (permalink / raw)
  To: gdb-patches

Andrew Burgess <aburgess@redhat.com> writes:

> In v2:
>
>   - Rebased to current HEAD.
>
>   - Retested.
>
> ---
>
> Finally!  After the recent filename completion changes, this
> mini-series adds filename option support to GDB.  I know there are
> other commands which can potentially make use of the new filename
> options, but for now I've just updated add-inferior as an example use
> case.

I've gone ahead and checked this series in.  As always, if there's any
post-commit feedback then I'm happy to address it.

Thanks,
Andrew


>
> ---
>
> Andrew Burgess (2):
>   gdb: add filename option support
>   gdb: use option framework for add-inferior and clone-inferior
>
>  gdb/cli/cli-option.c                          |  90 +++++-
>  gdb/cli/cli-option.h                          |  20 ++
>  gdb/inferior.c                                | 291 +++++++++++-------
>  gdb/maint-test-options.c                      |  28 +-
>  .../gdb.base/filename-completion.exp          |   7 +
>  gdb/testsuite/gdb.base/options.exp            |  95 +++++-
>  6 files changed, 410 insertions(+), 121 deletions(-)
>
>
> base-commit: a723c56efb07c4f8b3f6a3ed4b878a2f8f5572cc
> -- 
> 2.25.4


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2024-11-04 16:14 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-10-01 12:49 [PATCH 0/2] Filename option support Andrew Burgess
2024-10-01 12:49 ` [PATCH 1/2] gdb: add filename " Andrew Burgess
2024-10-01 12:49 ` [PATCH 2/2] gdb: use option framework for add-inferior and clone-inferior Andrew Burgess
2024-10-27  9:15 ` [PATCHv2 0/2] Filename option support Andrew Burgess
2024-10-27  9:15   ` [PATCHv2 1/2] gdb: add filename " Andrew Burgess
2024-10-27  9:15   ` [PATCHv2 2/2] gdb: use option framework for add-inferior and clone-inferior Andrew Burgess
2024-11-04 16:14   ` [PATCHv2 0/2] Filename option support Andrew Burgess

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).