* [PATCH 2/2] gdb: Make use of gdb::option framework for some info commands
2019-07-11 13:21 [PATCH 0/2] Make use of gdb::options for info variabels|functions|args|locals Andrew Burgess
@ 2019-07-11 13:21 ` Andrew Burgess
2019-07-11 13:21 ` [PATCH 1/2] gdb: Allow quoting around string options in the gdb::option framework Andrew Burgess
2019-07-11 13:36 ` [PATCH 0/2] Make use of gdb::options for info variabels|functions|args|locals Pedro Alves
2 siblings, 0 replies; 10+ messages in thread
From: Andrew Burgess @ 2019-07-11 13:21 UTC (permalink / raw)
To: gdb-patches; +Cc: Andrew Burgess
Update the 'info variables', 'info functions', 'info locals', and
'info args' commands to make use of the gdb::options framework.
There should be no user visible changes after this commit as I have
left the help text generation using the existing mechanism, which
already tries to customise the text for each of the commands.
gdb/ChangeLog:
* cli/cli-utils.c (extract_info_print_args): Delete.
(extract_arg_maybe_quoted): Delete.
(info_print_options_defs): New variable.
(make_info_print_options_def_group): New function.
(extract_info_print_options): Define new function.
* cli/cli-utils.h (extract_info_print_args): Delete.
(struct info_print_options): New structure.
(extract_info_print_options): Declare new function.
* stack.c (info_locals_command): Update to use new
extract_info_print_options, also add a header comment.
(info_args_command): Likewise.
* symtab.c (info_variables_command): Likewise.
(info_functions_command): Likewise.
---
gdb/ChangeLog | 16 +++++++
gdb/cli/cli-utils.c | 134 +++++++++++++++-------------------------------------
gdb/cli/cli-utils.h | 38 ++++++++-------
gdb/stack.c | 38 ++++-----------
gdb/symtab.c | 44 ++++++-----------
5 files changed, 99 insertions(+), 171 deletions(-)
diff --git a/gdb/cli/cli-utils.c b/gdb/cli/cli-utils.c
index 333a86a81b9..cd3dfe65a2b 100644
--- a/gdb/cli/cli-utils.c
+++ b/gdb/cli/cli-utils.c
@@ -23,8 +23,6 @@
#include <ctype.h>
-static std::string extract_arg_maybe_quoted (const char **arg);
-
/* See documentation in cli-utils.h. */
ULONGEST
@@ -182,38 +180,6 @@ get_number (char **pp)
/* See documentation in cli-utils.h. */
-bool
-extract_info_print_args (const char **args,
- bool *quiet,
- std::string *regexp,
- std::string *t_regexp)
-{
- /* Check for NAMEREGEXP or -- NAMEREGEXP. */
- if (**args != '-' || check_for_argument (args, "--", 2))
- {
- *regexp = *args;
- *args = NULL;
- return true;
- }
-
- if (check_for_argument (args, "-t", 2))
- {
- *t_regexp = extract_arg_maybe_quoted (args);
- *args = skip_spaces (*args);
- return true;
- }
-
- if (check_for_argument (args, "-q", 2))
- {
- *quiet = true;
- return true;
- }
-
- return false;
-}
-
-/* See documentation in cli-utils.h. */
-
void
report_unrecognized_option_error (const char *command, const char *args)
{
@@ -407,69 +373,6 @@ remove_trailing_whitespace (const char *start, const char *s)
return s;
}
-/* A helper function to extract an argument from *ARG. An argument is
- delimited by whitespace, but it can also be optionally quoted.
- The quoting and special characters are handled similarly to
- the parsing done by gdb_argv.
- The return value is empty if no argument was found. */
-
-static std::string
-extract_arg_maybe_quoted (const char **arg)
-{
- bool squote = false;
- bool dquote = false;
- bool bsquote = false;
- std::string result;
- const char *p = *arg;
-
- /* Find the start of the argument. */
- p = skip_spaces (p);
-
- /* Parse p similarly to gdb_argv buildargv function. */
- while (*p != '\0')
- {
- if (isspace (*p) && !squote && !dquote && !bsquote)
- break;
- else
- {
- if (bsquote)
- {
- bsquote = false;
- result += *p;
- }
- else if (*p == '\\')
- bsquote = true;
- else if (squote)
- {
- if (*p == '\'')
- squote = false;
- else
- result += *p;
- }
- else if (dquote)
- {
- if (*p == '"')
- dquote = false;
- else
- result += *p;
- }
- else
- {
- if (*p == '\'')
- squote = true;
- else if (*p == '"')
- dquote = true;
- else
- result += *p;
- }
- p++;
- }
- }
-
- *arg = p;
- return result;
-}
-
/* See documentation in cli-utils.h. */
std::string
@@ -532,4 +435,41 @@ validate_flags_qcs (const char *which_command, qcs_flags *flags)
error (_("%s: -c and -s are mutually exclusive"), which_command);
}
+/* The options used by the 'info variables' commands and similar. */
+
+static const gdb::option::option_def info_print_options_defs[] = {
+ gdb::option::boolean_option_def<info_print_options> {
+ "q",
+ [] (info_print_options *opt) { return &opt->quiet; },
+ nullptr, /* show_cmd_cb */
+ nullptr /* set_doc */
+ },
+
+ gdb::option::string_option_def<info_print_options> {
+ "t",
+ [] (info_print_options *opt) { return &opt->type_regexp; },
+ nullptr, /* show_cmd_cb */
+ nullptr /* set_doc */
+ }
+};
+
+/* Returns the option group used by 'info variables' and similar. */
+
+static gdb::option::option_def_group
+make_info_print_options_def_group (info_print_options *opts)
+{
+ return {{info_print_options_defs}, opts};
+}
+
/* See documentation in cli-utils.h. */
+
+void
+extract_info_print_options (info_print_options *opts,
+ const char **args)
+{
+ auto grp = make_info_print_options_def_group (opts);
+ gdb::option::process_options
+ (args, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_OPERAND, grp);
+ if (*args != nullptr && **args == '\0')
+ *args = nullptr;
+}
diff --git a/gdb/cli/cli-utils.h b/gdb/cli/cli-utils.h
index c2a0f374a6e..a3826be6824 100644
--- a/gdb/cli/cli-utils.h
+++ b/gdb/cli/cli-utils.h
@@ -43,22 +43,28 @@ extern int get_number (char **);
error instead of returning 0. */
extern ULONGEST get_ulongest (const char **pp, int trailer = '\0');
-/* Extract from ARGS the arguments [-q] [-t TYPEREGEXP] [--] NAMEREGEXP.
-
- The caller is responsible to initialize *QUIET to false, *REGEXP
- and *T_REGEXP to "".
- extract_info_print_args can then be called iteratively to search
- for valid arguments, as part of a 'main parsing loop' searching for
- -q/-t/-- arguments together with other flags and options.
-
- Returns true and updates *ARGS + one of *QUIET, *REGEXP, *T_REGEXP if
- it finds a valid argument.
- Returns false if no valid argument is found at the beginning of ARGS. */
-
-extern bool extract_info_print_args (const char **args,
- bool *quiet,
- std::string *regexp,
- std::string *t_regexp);
+/* Structure to hold the values of the options used by the 'info
+ variables' command and other similar commands. These correspond to the
+ -q and -t options. */
+
+struct info_print_options
+{
+ int quiet = false;
+ char *type_regexp = nullptr;
+
+ ~info_print_options ()
+ {
+ xfree (type_regexp);
+ }
+};
+
+/* Extract options from ARGS for commands like 'info variables', placing
+ the options into OPTS. ARGS is updated to point to the first character
+ after the options, or, if there is nothing after the options, then ARGS
+ is set to nullptr. */
+
+extern void extract_info_print_options (info_print_options *opts,
+ const char **args);
/* Throws an error telling the user that ARGS starts with an option
unrecognized by COMMAND. */
diff --git a/gdb/stack.c b/gdb/stack.c
index f7df7a40d10..175f2116a5b 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -2359,24 +2359,16 @@ print_frame_local_vars (struct frame_info *frame,
}
}
+/* Implement the 'info locals' command. */
+
void
info_locals_command (const char *args, int from_tty)
{
- std::string regexp;
- std::string t_regexp;
- bool quiet = false;
-
- while (args != NULL
- && extract_info_print_args (&args, &quiet, ®exp, &t_regexp))
- ;
-
- if (args != NULL)
- report_unrecognized_option_error ("info locals", args);
+ info_print_options opts;
+ extract_info_print_options (&opts, &args);
print_frame_local_vars (get_selected_frame (_("No frame selected.")),
- quiet,
- regexp.empty () ? NULL : regexp.c_str (),
- t_regexp.empty () ? NULL : t_regexp.c_str (),
+ opts.quiet, args, opts.type_regexp,
0, gdb_stdout);
}
@@ -2474,26 +2466,16 @@ print_frame_arg_vars (struct frame_info *frame,
}
}
+/* Implement the 'info args' command. */
+
void
info_args_command (const char *args, int from_tty)
{
- std::string regexp;
- std::string t_regexp;
- bool quiet = false;
-
- while (args != NULL
- && extract_info_print_args (&args, &quiet, ®exp, &t_regexp))
- ;
-
- if (args != NULL)
- report_unrecognized_option_error ("info args", args);
-
+ info_print_options opts;
+ extract_info_print_options (&opts, &args);
print_frame_arg_vars (get_selected_frame (_("No frame selected.")),
- quiet,
- regexp.empty () ? NULL : regexp.c_str (),
- t_regexp.empty () ? NULL : t_regexp.c_str (),
- gdb_stdout);
+ opts.quiet, args, opts.type_regexp, gdb_stdout);
}
\f
/* Return the symbol-block in which the selected frame is executing.
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 01118c62555..46691122187 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4687,6 +4687,9 @@ symtab_symbol_info (bool quiet,
gdb_assert (kind <= TYPES_DOMAIN);
+ if (regexp != nullptr && *regexp == '\0')
+ regexp = nullptr;
+
/* Must make sure that if we're interrupted, symbols gets freed. */
std::vector<symbol_search> symbols = search_symbols (regexp, kind,
t_regexp, 0, NULL);
@@ -4742,47 +4745,28 @@ symtab_symbol_info (bool quiet,
}
}
+/* Implement the 'info variables' command. */
+
static void
info_variables_command (const char *args, int from_tty)
{
- std::string regexp;
- std::string t_regexp;
- bool quiet = false;
-
- while (args != NULL
- && extract_info_print_args (&args, &quiet, ®exp, &t_regexp))
- ;
+ info_print_options opts;
+ extract_info_print_options (&opts, &args);
- if (args != NULL)
- report_unrecognized_option_error ("info variables", args);
-
- symtab_symbol_info (quiet,
- regexp.empty () ? NULL : regexp.c_str (),
- VARIABLES_DOMAIN,
- t_regexp.empty () ? NULL : t_regexp.c_str (),
- from_tty);
+ symtab_symbol_info (opts.quiet, args, VARIABLES_DOMAIN,
+ opts.type_regexp, from_tty);
}
+/* Implement the 'info functions' command. */
static void
info_functions_command (const char *args, int from_tty)
{
- std::string regexp;
- std::string t_regexp;
- bool quiet = false;
-
- while (args != NULL
- && extract_info_print_args (&args, &quiet, ®exp, &t_regexp))
- ;
-
- if (args != NULL)
- report_unrecognized_option_error ("info functions", args);
+ info_print_options opts;
+ extract_info_print_options (&opts, &args);
- symtab_symbol_info (quiet,
- regexp.empty () ? NULL : regexp.c_str (),
- FUNCTIONS_DOMAIN,
- t_regexp.empty () ? NULL : t_regexp.c_str (),
- from_tty);
+ symtab_symbol_info (opts.quiet, args, FUNCTIONS_DOMAIN,
+ opts.type_regexp, from_tty);
}
--
2.14.5
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/2] gdb: Allow quoting around string options in the gdb::option framework
2019-07-11 13:21 [PATCH 0/2] Make use of gdb::options for info variabels|functions|args|locals Andrew Burgess
2019-07-11 13:21 ` [PATCH 2/2] gdb: Make use of gdb::option framework for some info commands Andrew Burgess
@ 2019-07-11 13:21 ` Andrew Burgess
2019-07-11 16:17 ` Tom Tromey
2019-07-11 13:36 ` [PATCH 0/2] Make use of gdb::options for info variabels|functions|args|locals Pedro Alves
2 siblings, 1 reply; 10+ messages in thread
From: Andrew Burgess @ 2019-07-11 13:21 UTC (permalink / raw)
To: gdb-patches; +Cc: Andrew Burgess
Currently string options must be a single string with no whitespace,
this limitation prevents the gdb::option framework being used in some
places.
After this commit, string options can be quoted in single or double
quotes, and quote characters can be escaped with a backslash if needed
to either place them within quotes, or to avoid starting a quoted
argument.
This test adds a new function extract_string_maybe_quoted which is
basically a copy of extract_arg_maybe_quoted from cli/cli-utils.c,
however, the cli-utils.c function will be deleted in the next commit.
There are tests to exercise the new quoting mechanism.
gdb/ChangeLog:
* cli/cli-option.c (parse_option): Use extract_string_maybe_quoted
to extract string arguments.
* common/common-utils.c (extract_string_maybe_quoted): New function.
* common/common-utils.h (extract_string_maybe_quoted): Declare.
gdb/testsuite/ChangeLog:
* gdb.base/options.exp (expect_string): Dequote strings in
results.
(test-string): Test strings with different quoting and reindent.
---
gdb/ChangeLog | 7 +++++
gdb/cli/cli-option.c | 5 ++--
gdb/gdbsupport/common-utils.c | 59 ++++++++++++++++++++++++++++++++++++++
gdb/gdbsupport/common-utils.h | 10 +++++++
gdb/testsuite/ChangeLog | 6 ++++
gdb/testsuite/gdb.base/options.exp | 54 +++++++++++++++++++++++-----------
6 files changed, 121 insertions(+), 20 deletions(-)
diff --git a/gdb/cli/cli-option.c b/gdb/cli/cli-option.c
index 07d552b7f5b..eb8ef79d4f3 100644
--- a/gdb/cli/cli-option.c
+++ b/gdb/cli/cli-option.c
@@ -434,13 +434,12 @@ parse_option (gdb::array_view<const option_def_group> options_group,
}
const char *arg_start = *args;
- *args = skip_to_space (*args);
-
+ std::string str = extract_string_maybe_quoted (args);
if (*args == arg_start)
error (_("-%s requires an argument"), match->name);
option_value val;
- val.string = savestring (arg_start, *args - arg_start);
+ val.string = xstrdup (str.c_str ());
return option_def_and_value {*match, match_ctx, val};
}
diff --git a/gdb/gdbsupport/common-utils.c b/gdb/gdbsupport/common-utils.c
index 384029db70a..d1059de0b33 100644
--- a/gdb/gdbsupport/common-utils.c
+++ b/gdb/gdbsupport/common-utils.c
@@ -160,6 +160,65 @@ savestring (const char *ptr, size_t len)
return p;
}
+/* See documentation in common-utils.h. */
+
+std::string
+extract_string_maybe_quoted (const char **arg)
+{
+ bool squote = false;
+ bool dquote = false;
+ bool bsquote = false;
+ std::string result;
+ const char *p = *arg;
+
+ /* Find the start of the argument. */
+ p = skip_spaces (p);
+
+ /* Parse p similarly to gdb_argv buildargv function. */
+ while (*p != '\0')
+ {
+ if (isspace (*p) && !squote && !dquote && !bsquote)
+ break;
+ else
+ {
+ if (bsquote)
+ {
+ bsquote = false;
+ result += *p;
+ }
+ else if (*p == '\\')
+ bsquote = true;
+ else if (squote)
+ {
+ if (*p == '\'')
+ squote = false;
+ else
+ result += *p;
+ }
+ else if (dquote)
+ {
+ if (*p == '"')
+ dquote = false;
+ else
+ result += *p;
+ }
+ else
+ {
+ if (*p == '\'')
+ squote = true;
+ else if (*p == '"')
+ dquote = true;
+ else
+ result += *p;
+ }
+ p++;
+ }
+ }
+
+ *arg = p;
+ return result;
+}
+
/* The bit offset of the highest byte in a ULONGEST, for overflow
checking. */
diff --git a/gdb/gdbsupport/common-utils.h b/gdb/gdbsupport/common-utils.h
index 52bf3437b1c..a5312cb0c49 100644
--- a/gdb/gdbsupport/common-utils.h
+++ b/gdb/gdbsupport/common-utils.h
@@ -94,6 +94,16 @@ void string_vappendf (std::string &dest, const char* fmt, va_list args)
char *savestring (const char *ptr, size_t len);
+/* Extract the next word from ARG. The next word is defined as either,
+ everything up to the next space, or, if the next word starts with either
+ a single or double quote, then everything up to the closing quote. The
+ enclosing quotes are not returned in the result string. The pointer in
+ ARG is updated to point to the first character after the end of the
+ word, or, for quoted words, the first character after the closing
+ quote. */
+
+std::string extract_string_maybe_quoted (const char **arg);
+
/* The strerror() function can return NULL for errno values that are
out of range. Provide a "safe" version that always returns a
printable string. */
diff --git a/gdb/testsuite/gdb.base/options.exp b/gdb/testsuite/gdb.base/options.exp
index e8f571d9ba9..3495a0142fb 100644
--- a/gdb/testsuite/gdb.base/options.exp
+++ b/gdb/testsuite/gdb.base/options.exp
@@ -128,6 +128,11 @@ proc expect_integer {option val operand} {
# test-options xxx", with -string set to $STR. OPERAND is the
# expected operand.
proc expect_string {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 0 -zuint-unl 0 -string '$str' -- $operand"
}
@@ -967,26 +972,41 @@ proc_with_prefix test-string {variant} {
"-string requires an argument"
}
- res_test_gdb_complete_none \
- "1 [expect_none ""]" \
- "$cmd -string STR"
- gdb_test "$cmd -string STR --" [expect_string "STR" ""]
+ 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 -string ${str}"
+ gdb_test "$cmd -string ${str} --" [expect_string "${str}" ""]
- # Completing at "-" after parsing STR should list all options.
- res_test_gdb_complete_multiple \
- "1 [expect_string "STR" "-"]" \
- "$cmd -string STR " "-" "" $all_options
+ # Completing at "-" after parsing STR should list all options.
+ res_test_gdb_complete_multiple \
+ "1 [expect_string "${str}" "-"]" \
+ "$cmd -string ${str} " "-" "" $all_options
- # Check that only FOO is considered part of the string's value.
- # I.e., that we stop parsing the string at the first whitespace.
- if {$variant == "require-delimiter"} {
- res_test_gdb_complete_none \
- "1 [expect_string "FOO" "BAR"]" \
- "$cmd -string FOO BAR"
- } else {
- res_test_gdb_complete_none "0 BAR" "$cmd -string FOO BAR"
+ # Check that only $STR is considered part of the string's value.
+ # I.e., that we stop parsing the string at the first
+ # whitespace or after the closing quote of $STR.
+ if {$variant == "require-delimiter"} {
+ res_test_gdb_complete_none \
+ "1 [expect_string "${str}" "BAR"]" \
+ "$cmd -string ${str} BAR"
+ } else {
+ res_test_gdb_complete_none "0 BAR" "$cmd -string ${str} BAR"
+ }
+ gdb_test "$cmd -string ${str} BAR --" "Unrecognized option at: BAR --"
}
- gdb_test "$cmd -string FOO BAR --" "Unrecognized option at: BAR --"
}
# Run the options framework tests first.
--
2.14.5
^ permalink raw reply [flat|nested] 10+ messages in thread