Teach gdb::option about string options

A following patch will make the "pipe" command use the gdb::option
framework for option processing.  However, "pipe"'s only option today
is a string option, "-d DELIM", and gdb::option does not support
string options yet.

This commit adds support for string options, mapped to var_string.
For now, a string is parsed up until the first whitespace.  I imagine
that we'll need to add support for quoting so that we could do:

 (gdb) cmd -option 'some -string'

without gdb confusing the "-string" for an option.

This doesn't seem important for pipe, so I'm leaving it for another
day.

One thing I'm not happy with, is that the string data is managed as a
raw malloc-allocated char *, which means that we need to xfree it
manually.  This is because var_string settings work that way too.
Although with var_string settings we're leaking the strings at gdb
exit, that was never really a problem.  For options though, leaking is
undesirable.

I think we should tackle that for both settings and options at the
same time, so for now I'm just managing the malloced data manually.
It's a bit ugly in option_def_and_value, but at least that's hidden
from view.

For testing, this adds a new "-string" option to "maint
test-settings", and then tweaks gdb.base/options.exp to exercise it.

gdb/ChangeLog:
2019-07-03  Pedro Alves  <palves@redhat.com>

	* cli/cli-option.c (union option_value) <string>: New field.
	(struct option_def_and_value): Add ctor, move ctor, dtor and
	use DISABLE_COPY_AND_ASSIGN.
	(option_def_and_value::clear_value): New.
	(parse_option, save_option_value_in_ctx, get_val_type_str)
	(add_setshow_cmds_for_options): Handle var_string.
	* cli-option.h (union option_def::var_address) <string>: New
	field.
	(struct string_option_def): New.
	* maint-test-options.c (struct test_options_opts): Add default
	ctor and use DISABLE_COPY_AND_ASSIGN.
	<string_opt>: New field.
	(test_options_opts::~test_options_opts): New.
	(test_options_opts::dump): Also dump "-string".
	(test_options_option_defs): Install "string.

gdb/testsuite/ChangeLog:
2019-07-03  Pedro Alves  <palves@redhat.com>

	* gdb.base/options.exp (expect_none, expect_flag, expect_bool)
	(expect_integer): Adjust to expect "-string".
	(expect_string): New.
	(all_options): Expect "-string".
	(test-flag, test-boolean): Adjust to expect "-string".
	(test-string): New proc.
	(top level): Call it.

2 files changed, 107 insertions, 0 deletions

diff --git a/gdb/cli/cli-option.c b/gdb/cli/cli-option.c
index 8f28446..07d552b 100644
--- a/gdb/cli/cli-option.c
+++ b/gdb/cli/cli-option.c
@@ -43,6 +43,9 @@ union option_value
 
   /* For var_enum options.  */
   const char *enumeration;
+
+  /* For var_string options.  This is malloc-allocated.  */
+  char *string;
 };
 
 /* Holds an options definition and its value.  */
@@ -56,6 +59,54 @@ struct option_def_and_value
 
   /* The option's value, if any.  */
   gdb::optional<option_value> value;
+
+  /* Constructor.  */
+  option_def_and_value (const option_def &option_, void *ctx_,
+			gdb::optional<option_value> &&value_ = {})
+    : option (option_),
+      ctx (ctx_),
+      value (std::move (value_))
+  {
+    clear_value (option_, value_);
+  }
+
+  /* Move constructor.  Need this because for some types the values
+     are allocated on the heap.  */
+  option_def_and_value (option_def_and_value &&rval)
+    : option (rval.option),
+      ctx (rval.ctx),
+      value (std::move (rval.value))
+  {
+    clear_value (rval.option, rval.value);
+  }
+
+  DISABLE_COPY_AND_ASSIGN (option_def_and_value);
+
+  ~option_def_and_value ()
+  {
+    if (value.has_value ())
+      {
+	if (option.type == var_string)
+	  xfree (value->string);
+      }
+  }
+
+private:
+
+  /* Clear the option_value, without releasing it.  This is used after
+     the value has been moved to some other option_def_and_value
+     instance.  This is needed because for some types the value is
+     allocated on the heap, so we must clear the pointer in the
+     source, to avoid a double free.  */
+  static void clear_value (const option_def &option,
+			   gdb::optional<option_value> &value)
+  {
+    if (value.has_value ())
+      {
+	if (option.type == var_string)
+	  value->string = nullptr;
+      }
+  }
 };
 
 static void save_option_value_in_ctx (gdb::optional<option_def_and_value> &ov);
@@ -373,6 +424,25 @@ parse_option (gdb::array_view<const option_def_group> options_group,
 	val.enumeration = parse_cli_var_enum (args, match->enums);
 	return option_def_and_value {*match, match_ctx, val};
       }
+    case var_string:
+      {
+	if (check_for_argument (args, "--"))
+	  {
+	    /* Treat e.g., "maint test-options -string --" as if there
+	       was no argument after "-string".  */
+	    error (_("-%s requires an argument"), match->name);
+	  }
+
+	const char *arg_start = *args;
+	*args = skip_to_space (*args);
+
+	if (*args == arg_start)
+	  error (_("-%s requires an argument"), match->name);
+
+	option_value val;
+	val.string = savestring (arg_start, *args - arg_start);
+	return option_def_and_value {*match, match_ctx, val};
+      }
 
     default:
       /* Not yet.  */
@@ -532,6 +602,11 @@ save_option_value_in_ctx (gdb::optional<option_def_and_value> &ov)
       *ov->option.var_address.enumeration (ov->option, ov->ctx)
 	= ov->value->enumeration;
       break;
+    case var_string:
+      *ov->option.var_address.string (ov->option, ov->ctx)
+	= ov->value->string;
+      ov->value->string = nullptr;
+      break;
     default:
       gdb_assert_not_reached ("unhandled option type");
     }
@@ -604,6 +679,8 @@ get_val_type_str (const option_def &opt, std::string &buffer)
 	  }
 	return buffer.c_str ();
       }
+    case var_string:
+      return "STRING";
     default:
       return nullptr;
     }
@@ -731,6 +808,15 @@ add_setshow_cmds_for_options (command_class cmd_class,
 				nullptr, option.show_cmd_cb,
 				set_list, show_list);
 	}
+      else if (option.type == var_string)
+	{
+	  add_setshow_string_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 1bfbfce..a26b52f 100644
--- a/gdb/cli/cli-option.h
+++ b/gdb/cli/cli-option.h
@@ -86,6 +86,7 @@ public:
       unsigned int *(*uinteger) (const option_def &, void *ctx);
       int *(*integer) (const option_def &, void *ctx);
       const char **(*enumeration) (const option_def &, void *ctx);
+      char **(*string) (const option_def &, void *ctx);
     }
   var_address;
 
@@ -261,6 +262,26 @@ struct enum_option_def : option_def
   }
 };
 
+/* A var_string command line option.  */
+
+template<typename Context>
+struct string_option_def : option_def
+{
+  string_option_def (const char *long_option_,
+		     char **(*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_string,
+		  (erased_get_var_address_ftype *) get_var_address_cb_,
+		  show_cmd_cb_,
+		  set_doc_, show_doc_, help_doc_)
+  {
+    var_address.enumeration = detail::get_var_address<const char *, 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