* [RFA 00/10] Remove some cleanups from linespec.c
@ 2018-04-01 16:35 Tom Tromey
2018-04-01 16:35 ` [RFA 07/10] Change streq to return bool Tom Tromey
` (10 more replies)
0 siblings, 11 replies; 18+ messages in thread
From: Tom Tromey @ 2018-04-01 16:35 UTC (permalink / raw)
To: gdb-patches
This series removes many (but not all) cleanups from linespec.c.
Generally the removals are done in the normal way: replacing manual
memory management with a self-managing data structure.
I've tried to make each patch relatively small to make them simpler to
review.
In a few cases the patch required changes outside of linespec.c.
A couple of the patches (at least #2 and #10) are obvious, though of
course it doesn't hurt to read them anyhow.
Regression tested by the buildbot. I've also built each patch in the
series locally and run it through the gdb.linespec tests, while I was
tracking down the failures described in patch #7 (though of course the
series has changed slightly since then).
Tom
^ permalink raw reply [flat|nested] 18+ messages in thread
* [RFA 03/10] Make copy_token_string return unique_xmalloc_ptr
2018-04-01 16:35 [RFA 00/10] Remove some cleanups from linespec.c Tom Tromey
2018-04-01 16:35 ` [RFA 07/10] Change streq to return bool Tom Tromey
@ 2018-04-01 16:35 ` Tom Tromey
2018-04-01 16:35 ` [RFA 08/10] More use of std::vector in linespec.c Tom Tromey
` (8 subsequent siblings)
10 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2018-04-01 16:35 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This changes copy_token_string to return a unique_xmalloc_ptr, which
allows the removal of some cleanups.
2018-03-31 Tom Tromey <tom@tromey.com>
* linespec.c (copy_token_string): Return a unique_xmalloc_ptr.
(unexpected_linespec_error): Update.
(linespec_parse_basic, parse_linespec): Update.
---
gdb/ChangeLog | 6 ++++
gdb/linespec.c | 96 +++++++++++++++++++---------------------------------------
2 files changed, 37 insertions(+), 65 deletions(-)
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 9273dcde88..8d1ac326d8 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -536,20 +536,18 @@ skip_quote_char (const char *string, char quote_char)
/* Make a writable copy of the string given in TOKEN, trimming
any trailing whitespace. */
-static char *
+static gdb::unique_xmalloc_ptr<char>
copy_token_string (linespec_token token)
{
- char *str, *s;
+ const char *str, *s;
if (token.type == LSTOKEN_KEYWORD)
- return xstrdup (LS_TOKEN_KEYWORD (token));
+ return gdb::unique_xmalloc_ptr<char> (xstrdup (LS_TOKEN_KEYWORD (token)));
- str = savestring (LS_TOKEN_STOKEN (token).ptr,
- LS_TOKEN_STOKEN (token).length);
+ str = LS_TOKEN_STOKEN (token).ptr;
s = remove_trailing_whitespace (str, str + LS_TOKEN_STOKEN (token).length);
- *s = '\0';
- return str;
+ return gdb::unique_xmalloc_ptr<char> (savestring (str, s - str));
}
/* Does P represent the end of a quote-enclosed linespec? */
@@ -1692,13 +1690,10 @@ unexpected_linespec_error (linespec_parser *parser)
if (token.type == LSTOKEN_STRING || token.type == LSTOKEN_NUMBER
|| token.type == LSTOKEN_KEYWORD)
{
- char *string;
-
- string = copy_token_string (token);
- make_cleanup (xfree, string);
+ gdb::unique_xmalloc_ptr<char> string = copy_token_string (token);
throw_error (GENERIC_ERROR,
_("malformed linespec error: unexpected %s, \"%s\""),
- token_type_strings[token.type], string);
+ token_type_strings[token.type], string.get ());
}
else
throw_error (GENERIC_ERROR,
@@ -1792,11 +1787,10 @@ set_completion_after_number (linespec_parser *parser,
static void
linespec_parse_basic (linespec_parser *parser)
{
- char *name;
+ gdb::unique_xmalloc_ptr<char> name;
linespec_token token;
VEC (symbolp) *symbols, *labels;
VEC (bound_minimal_symbol_d) *minimal_symbols;
- struct cleanup *cleanup;
/* Get the next token. */
token = linespec_lexer_lex_one (parser);
@@ -1818,9 +1812,8 @@ linespec_parse_basic (linespec_parser *parser)
/* Record the line offset and get the next token. */
name = copy_token_string (token);
- cleanup = make_cleanup (xfree, name);
- PARSER_EXPLICIT (parser)->line_offset = linespec_parse_line_offset (name);
- do_cleanups (cleanup);
+ PARSER_EXPLICIT (parser)->line_offset
+ = linespec_parse_line_offset (name.get ());
/* Get the next token. */
token = linespec_lexer_consume_token (parser);
@@ -1851,7 +1844,6 @@ linespec_parse_basic (linespec_parser *parser)
/* The current token will contain the name of a function, method,
or label. */
name = copy_token_string (token);
- cleanup = make_cleanup (free_current_contents, &name);
if (parser->completion_tracker != NULL)
{
@@ -1885,21 +1877,19 @@ linespec_parse_basic (linespec_parser *parser)
PARSER_STREAM (parser)++;
LS_TOKEN_STOKEN (token).length++;
- xfree (name);
- name = savestring (parser->completion_word,
- (PARSER_STREAM (parser)
- - parser->completion_word));
+ name.reset (savestring (parser->completion_word,
+ (PARSER_STREAM (parser)
+ - parser->completion_word)));
}
}
- PARSER_EXPLICIT (parser)->function_name = name;
- discard_cleanups (cleanup);
+ PARSER_EXPLICIT (parser)->function_name = name.release ();
}
else
{
/* Try looking it up as a function/method. */
find_linespec_symbols (PARSER_STATE (parser),
- PARSER_RESULT (parser)->file_symtabs, name,
+ PARSER_RESULT (parser)->file_symtabs, name.get (),
PARSER_EXPLICIT (parser)->func_name_match_type,
&symbols, &minimal_symbols);
@@ -1907,43 +1897,36 @@ linespec_parse_basic (linespec_parser *parser)
{
PARSER_RESULT (parser)->function_symbols = symbols;
PARSER_RESULT (parser)->minimal_symbols = minimal_symbols;
- PARSER_EXPLICIT (parser)->function_name = name;
+ PARSER_EXPLICIT (parser)->function_name = name.release ();
symbols = NULL;
- discard_cleanups (cleanup);
}
else
{
/* NAME was not a function or a method. So it must be a label
name or user specified variable like "break foo.c:$zippo". */
labels = find_label_symbols (PARSER_STATE (parser), NULL,
- &symbols, name);
+ &symbols, name.get ());
if (labels != NULL)
{
PARSER_RESULT (parser)->labels.label_symbols = labels;
PARSER_RESULT (parser)->labels.function_symbols = symbols;
- PARSER_EXPLICIT (parser)->label_name = name;
+ PARSER_EXPLICIT (parser)->label_name = name.release ();
symbols = NULL;
- discard_cleanups (cleanup);
}
else if (token.type == LSTOKEN_STRING
&& *LS_TOKEN_STOKEN (token).ptr == '$')
{
/* User specified a convenience variable or history value. */
PARSER_EXPLICIT (parser)->line_offset
- = linespec_parse_variable (PARSER_STATE (parser), name);
+ = linespec_parse_variable (PARSER_STATE (parser), name.get ());
if (PARSER_EXPLICIT (parser)->line_offset.sign == LINE_OFFSET_UNKNOWN)
{
/* The user-specified variable was not valid. Do not
throw an error here. parse_linespec will do it for us. */
- PARSER_EXPLICIT (parser)->function_name = name;
- discard_cleanups (cleanup);
+ PARSER_EXPLICIT (parser)->function_name = name.release ();
return;
}
-
- /* The convenience variable/history value parsed correctly.
- NAME is no longer needed. */
- do_cleanups (cleanup);
}
else
{
@@ -1951,8 +1934,7 @@ linespec_parse_basic (linespec_parser *parser)
an error here. parse_linespec will do it for us. */
/* Save a copy of the name we were trying to lookup. */
- PARSER_EXPLICIT (parser)->function_name = name;
- discard_cleanups (cleanup);
+ PARSER_EXPLICIT (parser)->function_name = name.release ();
return;
}
}
@@ -1980,10 +1962,8 @@ linespec_parse_basic (linespec_parser *parser)
set_completion_after_number (parser, linespec_complete_what::KEYWORD);
name = copy_token_string (token);
- cleanup = make_cleanup (xfree, name);
PARSER_EXPLICIT (parser)->line_offset
- = linespec_parse_line_offset (name);
- do_cleanups (cleanup);
+ = linespec_parse_line_offset (name.get ());
/* Get the next token. */
token = linespec_lexer_consume_token (parser);
@@ -2027,24 +2007,22 @@ linespec_parse_basic (linespec_parser *parser)
{
/* Grab a copy of the label's name and look it up. */
name = copy_token_string (token);
- cleanup = make_cleanup (xfree, name);
labels = find_label_symbols (PARSER_STATE (parser),
PARSER_RESULT (parser)->function_symbols,
- &symbols, name);
+ &symbols, name.get ());
if (labels != NULL)
{
PARSER_RESULT (parser)->labels.label_symbols = labels;
PARSER_RESULT (parser)->labels.function_symbols = symbols;
- PARSER_EXPLICIT (parser)->label_name = name;
+ PARSER_EXPLICIT (parser)->label_name = name.release ();
symbols = NULL;
- discard_cleanups (cleanup);
}
else
{
/* We don't know what it was, but it isn't a label. */
undefined_label_error (PARSER_EXPLICIT (parser)->function_name,
- name);
+ name.get ());
}
}
@@ -2062,11 +2040,9 @@ linespec_parse_basic (linespec_parser *parser)
/* Record the line offset and get the next token. */
name = copy_token_string (token);
- cleanup = make_cleanup (xfree, name);
PARSER_EXPLICIT (parser)->line_offset
- = linespec_parse_line_offset (name);
- do_cleanups (cleanup);
+ = linespec_parse_line_offset (name.get ());
/* Get the next token. */
token = linespec_lexer_consume_token (parser);
@@ -2523,7 +2499,6 @@ parse_linespec (linespec_parser *parser, const char *arg,
{
linespec_token token;
struct gdb_exception file_exception = exception_none;
- struct cleanup *cleanup;
/* A special case to start. It has become quite popular for
IDEs to work around bugs in the previous parser by quoting
@@ -2581,18 +2556,14 @@ parse_linespec (linespec_parser *parser, const char *arg,
/* It must be either LSTOKEN_STRING or LSTOKEN_NUMBER. */
if (token.type == LSTOKEN_STRING && *LS_TOKEN_STOKEN (token).ptr == '$')
{
- char *var;
-
/* A NULL entry means to use GLOBAL_DEFAULT_SYMTAB. */
if (parser->completion_tracker == NULL)
VEC_safe_push (symtab_ptr, PARSER_RESULT (parser)->file_symtabs, NULL);
/* User specified a convenience variable or history value. */
- var = copy_token_string (token);
- cleanup = make_cleanup (xfree, var);
+ gdb::unique_xmalloc_ptr<char> var = copy_token_string (token);
PARSER_EXPLICIT (parser)->line_offset
- = linespec_parse_variable (PARSER_STATE (parser), var);
- do_cleanups (cleanup);
+ = linespec_parse_variable (PARSER_STATE (parser), var.get ());
/* If a line_offset wasn't found (VAR is the name of a user
variable/function), then skip to normal symbol processing. */
@@ -2621,17 +2592,15 @@ parse_linespec (linespec_parser *parser, const char *arg,
if (token.type == LSTOKEN_COLON)
{
- char *user_filename;
-
/* Get the current token again and extract the filename. */
token = linespec_lexer_lex_one (parser);
- user_filename = copy_token_string (token);
+ gdb::unique_xmalloc_ptr<char> user_filename = copy_token_string (token);
/* Check if the input is a filename. */
TRY
{
PARSER_RESULT (parser)->file_symtabs
- = symtabs_from_filename (user_filename,
+ = symtabs_from_filename (user_filename.get (),
PARSER_STATE (parser)->search_pspace);
}
CATCH (ex, RETURN_MASK_ERROR)
@@ -2643,7 +2612,7 @@ parse_linespec (linespec_parser *parser, const char *arg,
if (file_exception.reason >= 0)
{
/* Symtabs were found for the file. Record the filename. */
- PARSER_EXPLICIT (parser)->source_filename = user_filename;
+ PARSER_EXPLICIT (parser)->source_filename = user_filename.release ();
/* Get the next token. */
token = linespec_lexer_consume_token (parser);
@@ -2653,9 +2622,6 @@ parse_linespec (linespec_parser *parser, const char *arg,
}
else
{
- /* No symtabs found -- discard user_filename. */
- xfree (user_filename);
-
/* A NULL entry means to use GLOBAL_DEFAULT_SYMTAB. */
VEC_safe_push (symtab_ptr, PARSER_RESULT (parser)->file_symtabs, NULL);
}
--
2.13.6
^ permalink raw reply [flat|nested] 18+ messages in thread
* [RFA 05/10] Have filter_results take a std::vector
2018-04-01 16:35 [RFA 00/10] Remove some cleanups from linespec.c Tom Tromey
` (4 preceding siblings ...)
2018-04-01 16:35 ` [RFA 04/10] Return std::string from canonical_to_fullform Tom Tromey
@ 2018-04-01 16:35 ` Tom Tromey
2018-04-01 16:35 ` [RFA 01/10] Remove some cleanups from search_minsyms_for_name Tom Tromey
` (4 subsequent siblings)
10 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2018-04-01 16:35 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This chagnes filter_results to take a std::vector, allowing the
removal of some cleanups in its callers.
2018-03-31 Tom Tromey <tom@tromey.com>
* linespec.c (filter_results): Use std::vector.
(decode_line_2, decode_line_full): Update.
---
gdb/ChangeLog | 5 +++++
gdb/linespec.c | 22 ++++++----------------
2 files changed, 11 insertions(+), 16 deletions(-)
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 02466759c0..ca916a2a1d 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -1402,12 +1402,9 @@ canonical_to_fullform (const struct linespec_canonical_name *canonical)
static void
filter_results (struct linespec_state *self,
std::vector<symtab_and_line> *result,
- VEC (const_char_ptr) *filters)
+ const std::vector<const char *> &filters)
{
- int i;
- const char *name;
-
- for (i = 0; VEC_iterate (const_char_ptr, filters, i, name); ++i)
+ for (const char *name : filters)
{
linespec_sals lsal;
@@ -1501,16 +1498,13 @@ decode_line_2 (struct linespec_state *self,
char *args;
const char *prompt;
int i;
- struct cleanup *old_chain;
- VEC (const_char_ptr) *filters = NULL;
+ std::vector<const char *> filters;
std::vector<struct decode_line_2_item> items;
gdb_assert (select_mode != multiple_symbols_all);
gdb_assert (self->canonical != NULL);
gdb_assert (!result->empty ());
- old_chain = make_cleanup (VEC_cleanup (const_char_ptr), &filters);
-
/* Prepare ITEMS array. */
for (i = 0; i < result->size (); ++i)
{
@@ -1557,7 +1551,6 @@ decode_line_2 (struct linespec_state *self,
if (select_mode == multiple_symbols_all || items.size () == 1)
{
- do_cleanups (old_chain);
convert_results_to_lsals (self, result);
return;
}
@@ -1591,7 +1584,6 @@ decode_line_2 (struct linespec_state *self,
multiple_symbols_all behavior even with the 'ask'
setting; and he can get separate breakpoints by entering
"2-57" at the query. */
- do_cleanups (old_chain);
convert_results_to_lsals (self, result);
return;
}
@@ -1605,7 +1597,7 @@ decode_line_2 (struct linespec_state *self,
if (!item->selected)
{
- VEC_safe_push (const_char_ptr, filters, item->fullform.c_str ());
+ filters.push_back (item->fullform.c_str ());
item->selected = 1;
}
else
@@ -1617,7 +1609,6 @@ decode_line_2 (struct linespec_state *self,
}
filter_results (self, result, filters);
- do_cleanups (old_chain);
}
\f
@@ -3228,7 +3219,7 @@ decode_line_full (const struct event_location *location, int flags,
const char *filter)
{
struct cleanup *cleanups;
- VEC (const_char_ptr) *filters = NULL;
+ std::vector<const char *> filters;
linespec_parser parser;
struct linespec_state *state;
@@ -3280,8 +3271,7 @@ decode_line_full (const struct event_location *location, int flags,
{
if (filter != NULL)
{
- make_cleanup (VEC_cleanup (const_char_ptr), &filters);
- VEC_safe_push (const_char_ptr, filters, filter);
+ filters.push_back (filter);
filter_results (state, &result, filters);
}
else
--
2.13.6
^ permalink raw reply [flat|nested] 18+ messages in thread
* [RFA 06/10] Remove a string copy from event_location_to_sals
2018-04-01 16:35 [RFA 00/10] Remove some cleanups from linespec.c Tom Tromey
` (8 preceding siblings ...)
2018-04-01 16:35 ` [RFA 02/10] Fix some indentation in linespec.c Tom Tromey
@ 2018-04-01 16:35 ` Tom Tromey
2018-04-02 2:44 ` [RFA 00/10] Remove some cleanups from linespec.c Simon Marchi
10 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2018-04-01 16:35 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
The use of "const" showed that a string copy in event_location_to_sals
was unnecessary. This patch removes it.
2018-03-31 Tom Tromey <tom@tromey.com>
* linespec.c (event_location_to_sals) <case ADDRESS_LOCATION>:
Remove a string copy.
---
gdb/ChangeLog | 5 +++++
gdb/linespec.c | 8 +-------
2 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/gdb/linespec.c b/gdb/linespec.c
index ca916a2a1d..48168eaaa6 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -3168,16 +3168,10 @@ event_location_to_sals (linespec_parser *parser,
if (addr_string != NULL)
{
- char *expr = xstrdup (addr_string);
- const char *const_expr = expr;
- struct cleanup *cleanup = make_cleanup (xfree, expr);
-
- addr = linespec_expression_to_pc (&const_expr);
+ addr = linespec_expression_to_pc (&addr_string);
if (PARSER_STATE (parser)->canonical != NULL)
PARSER_STATE (parser)->canonical->location
= copy_event_location (location);
-
- do_cleanups (cleanup);
}
result = convert_address_location_to_sals (PARSER_STATE (parser),
--
2.13.6
^ permalink raw reply [flat|nested] 18+ messages in thread
* [RFA 08/10] More use of std::vector in linespec.c
2018-04-01 16:35 [RFA 00/10] Remove some cleanups from linespec.c Tom Tromey
2018-04-01 16:35 ` [RFA 07/10] Change streq to return bool Tom Tromey
2018-04-01 16:35 ` [RFA 03/10] Make copy_token_string return unique_xmalloc_ptr Tom Tromey
@ 2018-04-01 16:35 ` Tom Tromey
2018-04-02 2:35 ` Simon Marchi
2018-04-01 16:35 ` [RFA 10/10] Remove unnecessary include from linespec.h Tom Tromey
` (7 subsequent siblings)
10 siblings, 1 reply; 18+ messages in thread
From: Tom Tromey @ 2018-04-01 16:35 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This changes some spots in linespec.c to take a std::vector. This
patch spilled out to objc-lang.c a bit as well. This change allows
for the removal of some cleanups.
2018-03-31 Tom Tromey <tom@tromey.com>
* utils.c (compare_strings): Remove.
* utils.h (compare_strings): Remove.
* objc-lang.h (find_imps): Update.
* objc-lang.c (find_methods): Take a std::vector.
(uniquify_strings, find_imps): Likewise.
* linespec.c (find_methods): Take a std::vector.
(decode_objc): Use std::vector.
(add_all_symbol_names_from_pspace, find_superclass_methods): Take
a std::vector.
(find_method, find_function_symbols): Use std::vector.
---
gdb/ChangeLog | 13 +++++++++++++
gdb/linespec.c | 47 ++++++++++++++++++-----------------------------
gdb/objc-lang.c | 44 ++++++++++++--------------------------------
gdb/objc-lang.h | 4 ++--
gdb/utils.c | 11 -----------
gdb/utils.h | 1 -
6 files changed, 45 insertions(+), 75 deletions(-)
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 48168eaaa6..96a3117293 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -379,7 +379,7 @@ static void add_matching_symbols_to_info (const char *name,
static void add_all_symbol_names_from_pspace (struct collect_info *info,
struct program_space *pspace,
- VEC (const_char_ptr) *names,
+ const std::vector<const char *> &names,
enum search_domain search_domain);
static VEC (symtab_ptr) *
@@ -1211,7 +1211,7 @@ iterate_over_file_blocks
static void
find_methods (struct type *t, enum language t_lang, const char *name,
- VEC (const_char_ptr) **result_names,
+ std::vector<const char *> *result_names,
VEC (typep) **superclasses)
{
int ibase;
@@ -1266,7 +1266,7 @@ find_methods (struct type *t, enum language t_lang, const char *name,
if (TYPE_FN_FIELD_STUB (f, field_counter))
continue;
phys_name = TYPE_FN_FIELD_PHYSNAME (f, field_counter);
- VEC_safe_push (const_char_ptr, *result_names, phys_name);
+ result_names->push_back (phys_name);
}
}
}
@@ -3401,20 +3401,19 @@ static std::vector<symtab_and_line>
decode_objc (struct linespec_state *self, linespec_p ls, const char *arg)
{
struct collect_info info;
- VEC (const_char_ptr) *symbol_names = NULL;
+ std::vector<const char *> symbol_names;
const char *new_argptr;
- struct cleanup *cleanup = make_cleanup (VEC_cleanup (const_char_ptr),
- &symbol_names);
info.state = self;
info.file_symtabs = NULL;
VEC_safe_push (symtab_ptr, info.file_symtabs, NULL);
- make_cleanup (VEC_cleanup (symtab_ptr), &info.file_symtabs);
+ struct cleanup *cleanup = make_cleanup (VEC_cleanup (symtab_ptr),
+ &info.file_symtabs);
info.result.symbols = NULL;
info.result.minimal_symbols = NULL;
new_argptr = find_imps (arg, &symbol_names);
- if (VEC_empty (const_char_ptr, symbol_names))
+ if (symbol_names.empty ())
{
do_cleanups (cleanup);
return {};
@@ -3640,13 +3639,10 @@ compare_msymbols (const void *a, const void *b)
static void
add_all_symbol_names_from_pspace (struct collect_info *info,
struct program_space *pspace,
- VEC (const_char_ptr) *names,
+ const std::vector<const char *> &names,
enum search_domain search_domain)
{
- int ix;
- const char *iter;
-
- for (ix = 0; VEC_iterate (const_char_ptr, names, ix, iter); ++ix)
+ for (const char *iter : names)
add_matching_symbols_to_info (iter,
symbol_name_match_type::FULL,
search_domain, info, pspace);
@@ -3655,9 +3651,9 @@ add_all_symbol_names_from_pspace (struct collect_info *info,
static void
find_superclass_methods (VEC (typep) *superclasses,
const char *name, enum language name_lang,
- VEC (const_char_ptr) **result_names)
+ std::vector<const char *> *result_names)
{
- int old_len = VEC_length (const_char_ptr, *result_names);
+ size_t old_len = result_names->size ();
VEC (typep) *iter_classes;
struct cleanup *cleanup = make_cleanup (null_cleanup, NULL);
@@ -3672,8 +3668,7 @@ find_superclass_methods (VEC (typep) *superclasses,
for (ix = 0; VEC_iterate (typep, iter_classes, ix, t); ++ix)
find_methods (t, name_lang, name, result_names, &new_supers);
- if (VEC_length (const_char_ptr, *result_names) != old_len
- || VEC_empty (typep, new_supers))
+ if (result_names->size () != old_len || VEC_empty (typep, new_supers))
break;
iter_classes = new_supers;
@@ -3695,9 +3690,9 @@ find_method (struct linespec_state *self, VEC (symtab_ptr) *file_symtabs,
struct symbol *sym;
struct cleanup *cleanup = make_cleanup (null_cleanup, NULL);
int ix;
- int last_result_len;
+ size_t last_result_len;
VEC (typep) *superclass_vec;
- VEC (const_char_ptr) *result_names;
+ std::vector<const char *> result_names;
struct collect_info info;
/* Sort symbols so that symbols with the same program space are next
@@ -3723,8 +3718,6 @@ find_method (struct linespec_state *self, VEC (symtab_ptr) *file_symtabs,
what to do. */
superclass_vec = NULL;
make_cleanup (VEC_cleanup (typep), &superclass_vec);
- result_names = NULL;
- make_cleanup (VEC_cleanup (const_char_ptr), &result_names);
last_result_len = 0;
for (ix = 0; VEC_iterate (symbolp, sym_classes, ix, sym); ++ix)
{
@@ -3749,7 +3742,7 @@ find_method (struct linespec_state *self, VEC (symtab_ptr) *file_symtabs,
{
/* If we did not find a direct implementation anywhere in
this program space, consider superclasses. */
- if (VEC_length (const_char_ptr, result_names) == last_result_len)
+ if (result_names.size () == last_result_len)
find_superclass_methods (superclass_vec, method_name,
SYMBOL_LANGUAGE (sym), &result_names);
@@ -3760,7 +3753,7 @@ find_method (struct linespec_state *self, VEC (symtab_ptr) *file_symtabs,
FUNCTIONS_DOMAIN);
VEC_truncate (typep, superclass_vec, 0);
- last_result_len = VEC_length (const_char_ptr, result_names);
+ last_result_len = result_names.size ();
}
}
@@ -3905,9 +3898,7 @@ find_function_symbols (struct linespec_state *state,
VEC (bound_minimal_symbol_d) **minsyms)
{
struct collect_info info;
- VEC (const_char_ptr) *symbol_names = NULL;
- struct cleanup *cleanup = make_cleanup (VEC_cleanup (const_char_ptr),
- &symbol_names);
+ std::vector<const char *> symbol_names;
info.state = state;
info.result.symbols = NULL;
@@ -3916,15 +3907,13 @@ find_function_symbols (struct linespec_state *state,
/* Try NAME as an Objective-C selector. */
find_imps (name, &symbol_names);
- if (!VEC_empty (const_char_ptr, symbol_names))
+ if (!symbol_names.empty ())
add_all_symbol_names_from_pspace (&info, state->search_pspace,
symbol_names, FUNCTIONS_DOMAIN);
else
add_matching_symbols_to_info (name, name_match_type, FUNCTIONS_DOMAIN,
&info, state->search_pspace);
- do_cleanups (cleanup);
-
if (VEC_empty (symbolp, info.result.symbols))
{
VEC_free (symbolp, info.result.symbols);
diff --git a/gdb/objc-lang.c b/gdb/objc-lang.c
index 4f7dc36660..8c24fde3ee 100644
--- a/gdb/objc-lang.c
+++ b/gdb/objc-lang.c
@@ -45,6 +45,7 @@
#include "cli/cli-utils.h"
#include <ctype.h>
+#include <algorithm>
struct objc_object {
CORE_ADDR isa;
@@ -974,7 +975,7 @@ parse_method (char *method, char *type, char **theclass,
static void
find_methods (char type, const char *theclass, const char *category,
const char *selector,
- VEC (const_char_ptr) **symbol_names)
+ std::vector<const char *> *symbol_names)
{
struct objfile *objfile = NULL;
@@ -1050,7 +1051,7 @@ find_methods (char type, const char *theclass, const char *category,
((nselector == NULL) || (strcmp (selector, nselector) != 0)))
continue;
- VEC_safe_push (const_char_ptr, *symbol_names, symname);
+ symbol_names->push_back (symname);
}
if (objc_csym == NULL)
@@ -1068,33 +1069,14 @@ find_methods (char type, const char *theclass, const char *category,
/* Uniquify a VEC of strings. */
static void
-uniquify_strings (VEC (const_char_ptr) **strings)
+uniquify_strings (std::vector<const char *> *strings)
{
- int ix;
- const char *elem, *last = NULL;
- int out;
-
- /* If the vector is empty, there's nothing to do. This explicit
- check is needed to avoid invoking qsort with NULL. */
- if (VEC_empty (const_char_ptr, *strings))
+ if (strings->empty ())
return;
- qsort (VEC_address (const_char_ptr, *strings),
- VEC_length (const_char_ptr, *strings),
- sizeof (const_char_ptr),
- compare_strings);
- out = 0;
- for (ix = 0; VEC_iterate (const_char_ptr, *strings, ix, elem); ++ix)
- {
- if (last == NULL || strcmp (last, elem) != 0)
- {
- /* Keep ELEM. */
- VEC_replace (const_char_ptr, *strings, out, elem);
- ++out;
- }
- last = elem;
- }
- VEC_truncate (const_char_ptr, *strings, out);
+ std::sort (strings->begin (), strings->end (), compare_cstrings);
+ strings->erase (std::unique (strings->begin (), strings->end (), streq),
+ strings->end ());
}
/*
@@ -1128,7 +1110,7 @@ uniquify_strings (VEC (const_char_ptr) **strings)
*/
const char *
-find_imps (const char *method, VEC (const_char_ptr) **symbol_names)
+find_imps (const char *method, std::vector<const char *> *symbol_names)
{
char type = '\0';
char *theclass = NULL;
@@ -1161,22 +1143,20 @@ find_imps (const char *method, VEC (const_char_ptr) **symbol_names)
/* If we hit the "selector" case, and we found some methods, then
add the selector itself as a symbol, if it exists. */
- if (selector_case && !VEC_empty (const_char_ptr, *symbol_names))
+ if (selector_case && !symbol_names->empty ())
{
struct symbol *sym = lookup_symbol (selector, NULL, VAR_DOMAIN,
0).symbol;
if (sym != NULL)
- VEC_safe_push (const_char_ptr, *symbol_names,
- SYMBOL_NATURAL_NAME (sym));
+ symbol_names->push_back (SYMBOL_NATURAL_NAME (sym));
else
{
struct bound_minimal_symbol msym
= lookup_minimal_symbol (selector, 0, 0);
if (msym.minsym != NULL)
- VEC_safe_push (const_char_ptr, *symbol_names,
- MSYMBOL_NATURAL_NAME (msym.minsym));
+ symbol_names->push_back (MSYMBOL_NATURAL_NAME (msym.minsym));
}
}
diff --git a/gdb/objc-lang.h b/gdb/objc-lang.h
index 0727dfb3f0..d3ed1d5fa6 100644
--- a/gdb/objc-lang.h
+++ b/gdb/objc-lang.h
@@ -37,8 +37,8 @@ extern char *objc_demangle (const char *mangled, int options);
extern int find_objc_msgcall (CORE_ADDR pc, CORE_ADDR *new_pc);
-extern const char *
- find_imps (const char *method, VEC (const_char_ptr) **symbol_names);
+extern const char *find_imps (const char *method,
+ std::vector<const char *> *symbol_names);
extern struct value *value_nsstring (struct gdbarch *gdbarch,
char *ptr, int len);
diff --git a/gdb/utils.c b/gdb/utils.c
index bd7553e5f4..df0a2cecc4 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -2937,17 +2937,6 @@ compare_positive_ints (const void *ap, const void *bp)
return * (int *) ap - * (int *) bp;
}
-/* String compare function for qsort. */
-
-int
-compare_strings (const void *arg1, const void *arg2)
-{
- const char **s1 = (const char **) arg1;
- const char **s2 = (const char **) arg2;
-
- return strcmp (*s1, *s2);
-}
-
#define AMBIGUOUS_MESS1 ".\nMatching formats:"
#define AMBIGUOUS_MESS2 \
".\nUse \"set gnutarget format-name\" to specify the format."
diff --git a/gdb/utils.h b/gdb/utils.h
index 7c211391e0..ee69827558 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -102,7 +102,6 @@ extern int streq_hash (const void *, const void *);
extern int subset_compare (const char *, const char *);
int compare_positive_ints (const void *ap, const void *bp);
-int compare_strings (const void *ap, const void *bp);
/* Compare C strings for std::sort. */
--
2.13.6
^ permalink raw reply [flat|nested] 18+ messages in thread
* [RFA 07/10] Change streq to return bool
2018-04-01 16:35 [RFA 00/10] Remove some cleanups from linespec.c Tom Tromey
@ 2018-04-01 16:35 ` Tom Tromey
2018-04-02 2:28 ` Simon Marchi
2018-04-01 16:35 ` [RFA 03/10] Make copy_token_string return unique_xmalloc_ptr Tom Tromey
` (9 subsequent siblings)
10 siblings, 1 reply; 18+ messages in thread
From: Tom Tromey @ 2018-04-01 16:35 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
I wanted to use streq with std::unique in another (upcoming) patch in
this seres, so I changed it to return bool. To my surprise, this lead
to regressions. The cause turned out to be that streq was used as an
htab callback -- by casting it to the correct function type. This
sort of cast is invalid, so this patch adds a variant which is
directly suitable for use by htab. (Note that I did not add an
overload, as I could not get that to work with template deduction in
the other patch.)
gdb/ChangeLog
2018-04-01 Tom Tromey <tom@tromey.com>
* completer.c (completion_tracker::completion_tracker): Remove
cast.
(completion_tracker::discard_completions): Likewise.
* breakpoint.c (ambiguous_names_p): Remove cast.
* ada-lang.c (_initialize_ada_language): Remove cast.
* utils.h (streq): Update.
(streq_hash): Add new declaration.
* utils.c (streq): Return bool.
(streq_hash): New function.
---
gdb/ChangeLog | 12 ++++++++++++
gdb/ada-lang.c | 5 ++---
gdb/breakpoint.c | 6 ++----
gdb/completer.c | 4 ++--
gdb/utils.c | 13 +++++++++++--
gdb/utils.h | 10 +++++++++-
6 files changed, 38 insertions(+), 12 deletions(-)
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 11939d7798..de20c43bed 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -14722,9 +14722,8 @@ When enabled, the debugger will stop using the DW_AT_GNAT_descriptive_type\n\
DWARF attribute."),
NULL, NULL, &maint_set_ada_cmdlist, &maint_show_ada_cmdlist);
- decoded_names_store = htab_create_alloc
- (256, htab_hash_string, (int (*)(const void *, const void *)) streq,
- NULL, xcalloc, xfree);
+ decoded_names_store = htab_create_alloc (256, htab_hash_string, streq_hash,
+ NULL, xcalloc, xfree);
/* The ada-lang observers. */
gdb::observers::new_objfile.attach (ada_new_objfile_observer);
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 991c29c1e2..f84fef2bea 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -13384,10 +13384,8 @@ static int
ambiguous_names_p (struct bp_location *loc)
{
struct bp_location *l;
- htab_t htab = htab_create_alloc (13, htab_hash_string,
- (int (*) (const void *,
- const void *)) streq,
- NULL, xcalloc, xfree);
+ htab_t htab = htab_create_alloc (13, htab_hash_string, streq_hash, NULL,
+ xcalloc, xfree);
for (l = loc; l != NULL; l = l->next)
{
diff --git a/gdb/completer.c b/gdb/completer.c
index 4de1bcff32..769dcf0eb6 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -1469,7 +1469,7 @@ int max_completions = 200;
completion_tracker::completion_tracker ()
{
m_entries_hash = htab_create_alloc (INITIAL_COMPLETION_HTAB_SIZE,
- htab_hash_string, (htab_eq) streq,
+ htab_hash_string, streq_hash,
NULL, xcalloc, xfree);
}
@@ -1487,7 +1487,7 @@ completion_tracker::discard_completions ()
htab_delete (m_entries_hash);
m_entries_hash = htab_create_alloc (INITIAL_COMPLETION_HTAB_SIZE,
- htab_hash_string, (htab_eq) streq,
+ htab_hash_string, streq_hash,
NULL, xcalloc, xfree);
}
diff --git a/gdb/utils.c b/gdb/utils.c
index ee19fed172..bd7553e5f4 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -2601,13 +2601,22 @@ strcmp_iw_ordered (const char *string1, const char *string2)
}
}
-/* A simple comparison function with opposite semantics to strcmp. */
+/* See utils.h. */
-int
+bool
streq (const char *lhs, const char *rhs)
{
return !strcmp (lhs, rhs);
}
+
+/* See utils.h. */
+
+int
+streq_hash (const void *lhs, const void *rhs)
+{
+ return streq ((const char *) lhs, (const char *) rhs);
+}
+
\f
/*
diff --git a/gdb/utils.h b/gdb/utils.h
index 0de0fe2baa..7c211391e0 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -89,7 +89,15 @@ extern int strcmp_iw (const char *string1, const char *string2);
extern int strcmp_iw_ordered (const char *, const char *);
-extern int streq (const char *, const char *);
+/* Return true if the strings are equal. This uses strcmp but inverts
+ the result. */
+
+extern bool streq (const char *, const char *);
+
+/* A variant of streq that is suitable for use as an htab
+ callback. */
+
+extern int streq_hash (const void *, const void *);
extern int subset_compare (const char *, const char *);
--
2.13.6
^ permalink raw reply [flat|nested] 18+ messages in thread
* [RFA 01/10] Remove some cleanups from search_minsyms_for_name
2018-04-01 16:35 [RFA 00/10] Remove some cleanups from linespec.c Tom Tromey
` (5 preceding siblings ...)
2018-04-01 16:35 ` [RFA 05/10] Have filter_results take a std::vector Tom Tromey
@ 2018-04-01 16:35 ` Tom Tromey
2018-04-02 1:39 ` Simon Marchi
2018-04-01 16:35 ` [RFA 09/10] Remove typep and VEC(typep) from linespec.c Tom Tromey
` (3 subsequent siblings)
10 siblings, 1 reply; 18+ messages in thread
From: Tom Tromey @ 2018-04-01 16:35 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This changes struct collect_minsyms to use a std::vector, which
enables the removal of a cleanup from search_minsyms_for_name. This
also changes iterate_over_minimal_symbols to take a
gdb::function_view, which makes a function in linespec.c more
type-safe.
gdb/ChangeLog
2018-03-31 Tom Tromey <tom@tromey.com>
* minsyms.h (iterate_over_minimal_symbols): Update.
* minsyms.c (iterate_over_minimal_symbols): Take a
gdb::function_view.
* linespec.c (struct collect_minsyms): Add constructor and
initializers.
<msyms>: Now a std::vector.
(compare_msyms): Now a std::sort comparator.
(add_minsym): Change type of second parameter.
(search_minsyms_for_name): Update.
---
gdb/ChangeLog | 12 +++++++
gdb/linespec.c | 112 ++++++++++++++++++++++++++++-----------------------------
gdb/minsyms.c | 9 ++---
gdb/minsyms.h | 8 ++---
4 files changed, 72 insertions(+), 69 deletions(-)
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 1236b3f475..bd09f57b05 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -46,6 +46,7 @@
#include "location.h"
#include "common/function-view.h"
#include "common/def-vector.h"
+#include <algorithm>
/* An enumeration of the various things a user might attempt to
complete for a linespec location. */
@@ -4375,8 +4376,15 @@ minsym_found (struct linespec_state *self, struct objfile *objfile,
struct collect_minsyms
{
+ collect_minsyms (int funfirstline_, int list_mode_, struct symtab *symtab_)
+ : symtab (symtab_),
+ funfirstline (funfirstline_),
+ list_mode (list_mode_)
+ {
+ }
+
/* The objfile we're examining. */
- struct objfile *objfile;
+ struct objfile *objfile = nullptr;
/* Only search the given symtab, or NULL to search for all symbols. */
struct symtab *symtab;
@@ -4388,7 +4396,7 @@ struct collect_minsyms
int list_mode;
/* The resulting symbols. */
- VEC (bound_minimal_symbol_d) *msyms;
+ std::vector<struct bound_minimal_symbol> msyms;
};
/* A helper function to classify a minimal_symbol_type according to
@@ -4415,47 +4423,43 @@ classify_mtype (enum minimal_symbol_type t)
}
}
-/* Callback for qsort that sorts symbols by priority. */
+/* Callback for std::sort that sorts symbols by priority. */
-static int
-compare_msyms (const void *a, const void *b)
+static bool
+compare_msyms (const bound_minimal_symbol &a, const bound_minimal_symbol &b)
{
- const bound_minimal_symbol_d *moa = (const bound_minimal_symbol_d *) a;
- const bound_minimal_symbol_d *mob = (const bound_minimal_symbol_d *) b;
- enum minimal_symbol_type ta = MSYMBOL_TYPE (moa->minsym);
- enum minimal_symbol_type tb = MSYMBOL_TYPE (mob->minsym);
+ enum minimal_symbol_type ta = MSYMBOL_TYPE (a.minsym);
+ enum minimal_symbol_type tb = MSYMBOL_TYPE (b.minsym);
- return classify_mtype (ta) - classify_mtype (tb);
+ return classify_mtype (ta) < classify_mtype (tb);
}
/* Callback for iterate_over_minimal_symbols that adds the symbol to
the result. */
static void
-add_minsym (struct minimal_symbol *minsym, void *d)
+add_minsym (struct minimal_symbol *minsym, struct collect_minsyms &info)
{
- struct collect_minsyms *info = (struct collect_minsyms *) d;
-
- if (info->symtab != NULL)
+ if (info.symtab != NULL)
{
/* We're looking for a label for which we don't have debug
info. */
CORE_ADDR func_addr;
- if (msymbol_is_function (info->objfile, minsym, &func_addr))
+ if (msymbol_is_function (info.objfile, minsym, &func_addr))
{
symtab_and_line sal = find_pc_sect_line (func_addr, NULL, 0);
- if (info->symtab != sal.symtab)
+ if (info.symtab != sal.symtab)
return;
}
}
/* Exclude data symbols when looking for breakpoint locations. */
- if (!info->list_mode && !msymbol_is_function (info->objfile, minsym))
+ if (!info.list_mode && !msymbol_is_function (info.objfile, minsym))
return;
- bound_minimal_symbol_d mo = {minsym, info->objfile};
- VEC_safe_push (bound_minimal_symbol_d, info->msyms, &mo);
+ struct bound_minimal_symbol mo = {minsym, info.objfile};
+ info.msyms.push_back (mo);
}
/* Search for minimal symbols called NAME. If SEARCH_PSPACE
@@ -4471,15 +4475,9 @@ search_minsyms_for_name (struct collect_info *info,
struct program_space *search_pspace,
struct symtab *symtab)
{
- struct collect_minsyms local;
- struct cleanup *cleanup;
-
- memset (&local, 0, sizeof (local));
- local.funfirstline = info->state->funfirstline;
- local.list_mode = info->state->list_mode;
- local.symtab = symtab;
-
- cleanup = make_cleanup (VEC_cleanup (bound_minimal_symbol_d), &local.msyms);
+ struct collect_minsyms local (info->state->funfirstline,
+ info->state->list_mode,
+ symtab);
if (symtab == NULL)
{
@@ -4499,7 +4497,11 @@ search_minsyms_for_name (struct collect_info *info,
ALL_OBJFILES (objfile)
{
local.objfile = objfile;
- iterate_over_minimal_symbols (objfile, name, add_minsym, &local);
+ iterate_over_minimal_symbols (objfile, name,
+ [&] (struct minimal_symbol *msym)
+ {
+ add_minsym (msym, local);
+ });
}
}
}
@@ -4509,40 +4511,34 @@ search_minsyms_for_name (struct collect_info *info,
{
set_current_program_space (SYMTAB_PSPACE (symtab));
local.objfile = SYMTAB_OBJFILE(symtab);
- iterate_over_minimal_symbols (local.objfile, name, add_minsym, &local);
+ iterate_over_minimal_symbols (local.objfile, name,
+ [&] (struct minimal_symbol *msym)
+ {
+ add_minsym (msym, local);
+ });
}
}
- if (!VEC_empty (bound_minimal_symbol_d, local.msyms))
- {
- int classification;
- int ix;
- bound_minimal_symbol_d *item;
-
- qsort (VEC_address (bound_minimal_symbol_d, local.msyms),
- VEC_length (bound_minimal_symbol_d, local.msyms),
- sizeof (bound_minimal_symbol_d),
- compare_msyms);
-
- /* Now the minsyms are in classification order. So, we walk
- over them and process just the minsyms with the same
- classification as the very first minsym in the list. */
- item = VEC_index (bound_minimal_symbol_d, local.msyms, 0);
- classification = classify_mtype (MSYMBOL_TYPE (item->minsym));
-
- for (ix = 0;
- VEC_iterate (bound_minimal_symbol_d, local.msyms, ix, item);
- ++ix)
- {
- if (classify_mtype (MSYMBOL_TYPE (item->minsym)) != classification)
- break;
+ if (!local.msyms.empty ())
+ {
+ int classification;
- VEC_safe_push (bound_minimal_symbol_d,
- info->result.minimal_symbols, item);
- }
- }
+ std::sort (local.msyms.begin (), local.msyms.end (), compare_msyms);
- do_cleanups (cleanup);
+ /* Now the minsyms are in classification order. So, we walk
+ over them and process just the minsyms with the same
+ classification as the very first minsym in the list. */
+ classification = classify_mtype (MSYMBOL_TYPE (local.msyms[0].minsym));
+
+ for (const struct bound_minimal_symbol &item : local.msyms)
+ {
+ if (classify_mtype (MSYMBOL_TYPE (item.minsym)) != classification)
+ break;
+
+ VEC_safe_push (bound_minimal_symbol_d,
+ info->result.minimal_symbols, &item);
+ }
+ }
}
/* A helper function to add all symbols matching NAME to INFO. If
diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index 72969b7778..08efa1dc7e 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -471,11 +471,8 @@ linkage_name_str (const lookup_name_info &lookup_name)
void
iterate_over_minimal_symbols (struct objfile *objf,
const lookup_name_info &lookup_name,
- void (*callback) (struct minimal_symbol *,
- void *),
- void *user_data)
+ gdb::function_view<void (struct minimal_symbol *)> callback)
{
-
/* The first pass is over the ordinary hash table. */
{
const char *name = linkage_name_str (lookup_name);
@@ -490,7 +487,7 @@ iterate_over_minimal_symbols (struct objfile *objf,
iter = iter->hash_next)
{
if (mangled_cmp (MSYMBOL_LINKAGE_NAME (iter), name) == 0)
- (*callback) (iter, user_data);
+ callback (iter);
}
}
@@ -509,7 +506,7 @@ iterate_over_minimal_symbols (struct objfile *objf,
iter != NULL;
iter = iter->demangled_hash_next)
if (name_match (MSYMBOL_SEARCH_NAME (iter), lookup_name, NULL))
- (*callback) (iter, user_data);
+ callback (iter);
}
}
diff --git a/gdb/minsyms.h b/gdb/minsyms.h
index 11a202025d..b05f717575 100644
--- a/gdb/minsyms.h
+++ b/gdb/minsyms.h
@@ -268,11 +268,9 @@ struct bound_minimal_symbol lookup_minimal_symbol_by_pc (CORE_ADDR);
For each matching symbol, CALLBACK is called with the symbol and
USER_DATA as arguments. */
-void iterate_over_minimal_symbols (struct objfile *objf,
- const lookup_name_info &name,
- void (*callback) (struct minimal_symbol *,
- void *),
- void *user_data);
+void iterate_over_minimal_symbols
+ (struct objfile *objf, const lookup_name_info &name,
+ gdb::function_view<void (struct minimal_symbol *)> callback);
/* Compute the upper bound of MINSYM. The upper bound is the last
address thought to be part of the symbol. If the symbol has a
--
2.13.6
^ permalink raw reply [flat|nested] 18+ messages in thread
* [RFA 04/10] Return std::string from canonical_to_fullform
2018-04-01 16:35 [RFA 00/10] Remove some cleanups from linespec.c Tom Tromey
` (3 preceding siblings ...)
2018-04-01 16:35 ` [RFA 10/10] Remove unnecessary include from linespec.h Tom Tromey
@ 2018-04-01 16:35 ` Tom Tromey
2018-04-02 2:15 ` Simon Marchi
2018-04-01 16:35 ` [RFA 05/10] Have filter_results take a std::vector Tom Tromey
` (5 subsequent siblings)
10 siblings, 1 reply; 18+ messages in thread
From: Tom Tromey @ 2018-04-01 16:35 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This changes canonical_to_fullform to return a std::string, and
changes decode_line_2 to use std::vector. This allows for the removal
of some cleanups.
2018-03-31 Tom Tromey <tom@tromey.com>
* linespec.c (canonical_to_fullform): Return std::string.
(filter_results): Update.
(struct decode_line_2_item): Add constructor.
<fullform, displayform>: Now std::string.
(decode_line_2_compare_items): Now a std::sort comparator.
(decode_line_2): Update.
---
gdb/ChangeLog | 9 +++++
gdb/linespec.c | 115 +++++++++++++++++++++++++++------------------------------
2 files changed, 63 insertions(+), 61 deletions(-)
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 8d1ac326d8..02466759c0 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -1384,16 +1384,16 @@ find_toplevel_string (const char *haystack, const char *needle)
}
/* Convert CANONICAL to its string representation using
- symtab_to_fullname for SYMTAB. The caller must xfree the result. */
+ symtab_to_fullname for SYMTAB. */
-static char *
+static std::string
canonical_to_fullform (const struct linespec_canonical_name *canonical)
{
if (canonical->symtab == NULL)
- return xstrdup (canonical->suffix);
+ return canonical->suffix;
else
- return xstrprintf ("%s:%s", symtab_to_fullname (canonical->symtab),
- canonical->suffix);
+ return string_printf ("%s:%s", symtab_to_fullname (canonical->symtab),
+ canonical->suffix);
}
/* Given FILTERS, a list of canonical names, filter the sals in RESULT
@@ -1414,17 +1414,12 @@ filter_results (struct linespec_state *self,
for (size_t j = 0; j < result->size (); ++j)
{
const struct linespec_canonical_name *canonical;
- char *fullform;
- struct cleanup *cleanup;
canonical = &self->canonical_names[j];
- fullform = canonical_to_fullform (canonical);
- cleanup = make_cleanup (xfree, fullform);
+ std::string fullform = canonical_to_fullform (canonical);
- if (strcmp (name, fullform) == 0)
+ if (name == fullform)
lsal.sals.push_back ((*result)[j]);
-
- do_cleanups (cleanup);
}
if (!lsal.sals.empty ())
@@ -1458,34 +1453,39 @@ convert_results_to_lsals (struct linespec_state *self,
struct decode_line_2_item
{
- /* The form using symtab_to_fullname.
- It must be xfree'ed after use. */
- char *fullform;
+ decode_line_2_item (std::string &&fullform_, std::string &&displayform_,
+ bool selected_)
+ : fullform (std::move (fullform_)),
+ displayform (std::move (displayform_)),
+ selected (selected_)
+ {
+ }
+
+ /* The form using symtab_to_fullname. */
+ std::string fullform;
- /* The form using symtab_to_filename_for_display.
- It must be xfree'ed after use. */
- char *displayform;
+ /* The form using symtab_to_filename_for_display. */
+ std::string displayform;
/* Field is initialized to zero and it is set to one if the user
requested breakpoint for this entry. */
unsigned int selected : 1;
};
-/* Helper for qsort to sort decode_line_2_item entries by DISPLAYFORM and
- secondarily by FULLFORM. */
+/* Helper for std::sort to sort decode_line_2_item entries by
+ DISPLAYFORM and secondarily by FULLFORM. */
-static int
-decode_line_2_compare_items (const void *ap, const void *bp)
+static bool
+decode_line_2_compare_items (const decode_line_2_item &a,
+ const decode_line_2_item &b)
{
- const struct decode_line_2_item *a = (const struct decode_line_2_item *) ap;
- const struct decode_line_2_item *b = (const struct decode_line_2_item *) bp;
int retval;
- retval = strcmp (a->displayform, b->displayform);
- if (retval != 0)
- return retval;
-
- return strcmp (a->fullform, b->fullform);
+ if (a.displayform < b.displayform)
+ return true;
+ if (a.displayform == b.displayform)
+ return a.fullform < b.fullform;
+ return false;
}
/* Handle multiple results in RESULT depending on SELECT_MODE. This
@@ -1503,8 +1503,7 @@ decode_line_2 (struct linespec_state *self,
int i;
struct cleanup *old_chain;
VEC (const_char_ptr) *filters = NULL;
- struct decode_line_2_item *items;
- int items_count;
+ std::vector<struct decode_line_2_item> items;
gdb_assert (select_mode != multiple_symbols_all);
gdb_assert (self->canonical != NULL);
@@ -1513,56 +1512,50 @@ decode_line_2 (struct linespec_state *self,
old_chain = make_cleanup (VEC_cleanup (const_char_ptr), &filters);
/* Prepare ITEMS array. */
- items_count = result->size ();
- items = XNEWVEC (struct decode_line_2_item, items_count);
- make_cleanup (xfree, items);
- for (i = 0; i < items_count; ++i)
+ for (i = 0; i < result->size (); ++i)
{
const struct linespec_canonical_name *canonical;
struct decode_line_2_item *item;
+ std::string displayform;
+
canonical = &self->canonical_names[i];
gdb_assert (canonical->suffix != NULL);
- item = &items[i];
- item->fullform = canonical_to_fullform (canonical);
- make_cleanup (xfree, item->fullform);
+ std::string fullform = canonical_to_fullform (canonical);
if (canonical->symtab == NULL)
- item->displayform = canonical->suffix;
+ displayform = canonical->suffix;
else
{
const char *fn_for_display;
fn_for_display = symtab_to_filename_for_display (canonical->symtab);
- item->displayform = xstrprintf ("%s:%s", fn_for_display,
- canonical->suffix);
- make_cleanup (xfree, item->displayform);
+ displayform = string_printf ("%s:%s", fn_for_display,
+ canonical->suffix);
}
- item->selected = 0;
+ items.emplace_back (std::move (fullform), std::move (displayform),
+ false);
}
/* Sort the list of method names. */
- qsort (items, items_count, sizeof (*items), decode_line_2_compare_items);
+ std::sort (items.begin (), items.end (), decode_line_2_compare_items);
/* Remove entries with the same FULLFORM. */
- if (items_count >= 2)
- {
- struct decode_line_2_item *dst, *src;
-
- dst = items;
- for (src = &items[1]; src < &items[items_count]; src++)
- if (strcmp (src->fullform, dst->fullform) != 0)
- *++dst = *src;
- items_count = dst + 1 - items;
- }
-
- if (select_mode == multiple_symbols_cancel && items_count > 1)
+ items.erase (std::unique (items.begin (), items.end (),
+ [] (const struct decode_line_2_item &a,
+ const struct decode_line_2_item &b)
+ {
+ return a.fullform == b.fullform;
+ }),
+ items.end ());
+
+ if (select_mode == multiple_symbols_cancel && items.size () > 1)
error (_("canceled because the command is ambiguous\n"
"See set/show multiple-symbol."));
- if (select_mode == multiple_symbols_all || items_count == 1)
+ if (select_mode == multiple_symbols_all || items.size () == 1)
{
do_cleanups (old_chain);
convert_results_to_lsals (self, result);
@@ -1570,8 +1563,8 @@ decode_line_2 (struct linespec_state *self,
}
printf_unfiltered (_("[0] cancel\n[1] all\n"));
- for (i = 0; i < items_count; i++)
- printf_unfiltered ("[%d] %s\n", i + 2, items[i].displayform);
+ for (i = 0; i < items.size (); i++)
+ printf_unfiltered ("[%d] %s\n", i + 2, items[i].displayform.c_str ());
prompt = getenv ("PS2");
if (prompt == NULL)
@@ -1604,7 +1597,7 @@ decode_line_2 (struct linespec_state *self,
}
num -= 2;
- if (num >= items_count)
+ if (num >= items.size ())
printf_unfiltered (_("No choice number %d.\n"), num);
else
{
@@ -1612,7 +1605,7 @@ decode_line_2 (struct linespec_state *self,
if (!item->selected)
{
- VEC_safe_push (const_char_ptr, filters, item->fullform);
+ VEC_safe_push (const_char_ptr, filters, item->fullform.c_str ());
item->selected = 1;
}
else
--
2.13.6
^ permalink raw reply [flat|nested] 18+ messages in thread
* [RFA 02/10] Fix some indentation in linespec.c
2018-04-01 16:35 [RFA 00/10] Remove some cleanups from linespec.c Tom Tromey
` (7 preceding siblings ...)
2018-04-01 16:35 ` [RFA 09/10] Remove typep and VEC(typep) from linespec.c Tom Tromey
@ 2018-04-01 16:35 ` Tom Tromey
2018-04-02 1:49 ` Simon Marchi
2018-04-01 16:35 ` [RFA 06/10] Remove a string copy from event_location_to_sals Tom Tromey
2018-04-02 2:44 ` [RFA 00/10] Remove some cleanups from linespec.c Simon Marchi
10 siblings, 1 reply; 18+ messages in thread
From: Tom Tromey @ 2018-04-01 16:35 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This removes some leftover comments and fixes the indentation in a
couple of spots in linespec.c.
gdb/ChangeLog
2018-03-31 Tom Tromey <tom@tromey.com>
* linespec.c (linespec_parse_basic): Reindent.
---
gdb/ChangeLog | 4 ++
gdb/linespec.c | 136 ++++++++++++++++++++++++++++-----------------------------
2 files changed, 70 insertions(+), 70 deletions(-)
diff --git a/gdb/linespec.c b/gdb/linespec.c
index bd09f57b05..9273dcde88 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -1897,68 +1897,66 @@ linespec_parse_basic (linespec_parser *parser)
}
else
{
- /* XXX Reindent before pushing. */
-
- /* Try looking it up as a function/method. */
- find_linespec_symbols (PARSER_STATE (parser),
- PARSER_RESULT (parser)->file_symtabs, name,
- PARSER_EXPLICIT (parser)->func_name_match_type,
- &symbols, &minimal_symbols);
+ /* Try looking it up as a function/method. */
+ find_linespec_symbols (PARSER_STATE (parser),
+ PARSER_RESULT (parser)->file_symtabs, name,
+ PARSER_EXPLICIT (parser)->func_name_match_type,
+ &symbols, &minimal_symbols);
- if (symbols != NULL || minimal_symbols != NULL)
- {
- PARSER_RESULT (parser)->function_symbols = symbols;
- PARSER_RESULT (parser)->minimal_symbols = minimal_symbols;
- PARSER_EXPLICIT (parser)->function_name = name;
- symbols = NULL;
- discard_cleanups (cleanup);
- }
- else
- {
- /* NAME was not a function or a method. So it must be a label
- name or user specified variable like "break foo.c:$zippo". */
- labels = find_label_symbols (PARSER_STATE (parser), NULL,
- &symbols, name);
- if (labels != NULL)
+ if (symbols != NULL || minimal_symbols != NULL)
{
- PARSER_RESULT (parser)->labels.label_symbols = labels;
- PARSER_RESULT (parser)->labels.function_symbols = symbols;
- PARSER_EXPLICIT (parser)->label_name = name;
+ PARSER_RESULT (parser)->function_symbols = symbols;
+ PARSER_RESULT (parser)->minimal_symbols = minimal_symbols;
+ PARSER_EXPLICIT (parser)->function_name = name;
symbols = NULL;
discard_cleanups (cleanup);
}
- else if (token.type == LSTOKEN_STRING
- && *LS_TOKEN_STOKEN (token).ptr == '$')
+ else
{
- /* User specified a convenience variable or history value. */
- PARSER_EXPLICIT (parser)->line_offset
- = linespec_parse_variable (PARSER_STATE (parser), name);
+ /* NAME was not a function or a method. So it must be a label
+ name or user specified variable like "break foo.c:$zippo". */
+ labels = find_label_symbols (PARSER_STATE (parser), NULL,
+ &symbols, name);
+ if (labels != NULL)
+ {
+ PARSER_RESULT (parser)->labels.label_symbols = labels;
+ PARSER_RESULT (parser)->labels.function_symbols = symbols;
+ PARSER_EXPLICIT (parser)->label_name = name;
+ symbols = NULL;
+ discard_cleanups (cleanup);
+ }
+ else if (token.type == LSTOKEN_STRING
+ && *LS_TOKEN_STOKEN (token).ptr == '$')
+ {
+ /* User specified a convenience variable or history value. */
+ PARSER_EXPLICIT (parser)->line_offset
+ = linespec_parse_variable (PARSER_STATE (parser), name);
- if (PARSER_EXPLICIT (parser)->line_offset.sign == LINE_OFFSET_UNKNOWN)
+ if (PARSER_EXPLICIT (parser)->line_offset.sign == LINE_OFFSET_UNKNOWN)
+ {
+ /* The user-specified variable was not valid. Do not
+ throw an error here. parse_linespec will do it for us. */
+ PARSER_EXPLICIT (parser)->function_name = name;
+ discard_cleanups (cleanup);
+ return;
+ }
+
+ /* The convenience variable/history value parsed correctly.
+ NAME is no longer needed. */
+ do_cleanups (cleanup);
+ }
+ else
{
- /* The user-specified variable was not valid. Do not
- throw an error here. parse_linespec will do it for us. */
+ /* The name is also not a label. Abort parsing. Do not throw
+ an error here. parse_linespec will do it for us. */
+
+ /* Save a copy of the name we were trying to lookup. */
PARSER_EXPLICIT (parser)->function_name = name;
discard_cleanups (cleanup);
return;
}
-
- /* The convenience variable/history value parsed correctly.
- NAME is no longer needed. */
- do_cleanups (cleanup);
- }
- else
- {
- /* The name is also not a label. Abort parsing. Do not throw
- an error here. parse_linespec will do it for us. */
-
- /* Save a copy of the name we were trying to lookup. */
- PARSER_EXPLICIT (parser)->function_name = name;
- discard_cleanups (cleanup);
- return;
}
}
- }
int previous_qc = parser->completion_quote_char;
@@ -2027,29 +2025,27 @@ linespec_parse_basic (linespec_parser *parser)
}
else
{
- /* XXX Reindent before pushing. */
-
- /* Grab a copy of the label's name and look it up. */
- name = copy_token_string (token);
- cleanup = make_cleanup (xfree, name);
- labels = find_label_symbols (PARSER_STATE (parser),
- PARSER_RESULT (parser)->function_symbols,
- &symbols, name);
+ /* Grab a copy of the label's name and look it up. */
+ name = copy_token_string (token);
+ cleanup = make_cleanup (xfree, name);
+ labels = find_label_symbols (PARSER_STATE (parser),
+ PARSER_RESULT (parser)->function_symbols,
+ &symbols, name);
- if (labels != NULL)
- {
- PARSER_RESULT (parser)->labels.label_symbols = labels;
- PARSER_RESULT (parser)->labels.function_symbols = symbols;
- PARSER_EXPLICIT (parser)->label_name = name;
- symbols = NULL;
- discard_cleanups (cleanup);
- }
- else
- {
- /* We don't know what it was, but it isn't a label. */
- undefined_label_error (PARSER_EXPLICIT (parser)->function_name,
- name);
- }
+ if (labels != NULL)
+ {
+ PARSER_RESULT (parser)->labels.label_symbols = labels;
+ PARSER_RESULT (parser)->labels.function_symbols = symbols;
+ PARSER_EXPLICIT (parser)->label_name = name;
+ symbols = NULL;
+ discard_cleanups (cleanup);
+ }
+ else
+ {
+ /* We don't know what it was, but it isn't a label. */
+ undefined_label_error (PARSER_EXPLICIT (parser)->function_name,
+ name);
+ }
}
--
2.13.6
^ permalink raw reply [flat|nested] 18+ messages in thread
* [RFA 10/10] Remove unnecessary include from linespec.h
2018-04-01 16:35 [RFA 00/10] Remove some cleanups from linespec.c Tom Tromey
` (2 preceding siblings ...)
2018-04-01 16:35 ` [RFA 08/10] More use of std::vector in linespec.c Tom Tromey
@ 2018-04-01 16:35 ` Tom Tromey
2018-04-01 16:35 ` [RFA 04/10] Return std::string from canonical_to_fullform Tom Tromey
` (6 subsequent siblings)
10 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2018-04-01 16:35 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
linespec.h was inculding vec.h, but doesn't expose any VECs.
So, this include can be removed.
2018-03-31 Tom Tromey <tom@tromey.com>
* linespec.h: Remove include of "vec.h".
---
gdb/ChangeLog | 4 ++++
gdb/linespec.h | 1 -
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/gdb/linespec.h b/gdb/linespec.h
index eced085e3e..7251a963af 100644
--- a/gdb/linespec.h
+++ b/gdb/linespec.h
@@ -20,7 +20,6 @@
struct symtab;
#include "location.h"
-#include "vec.h"
/* Flags to pass to decode_line_1 and decode_line_full. */
--
2.13.6
^ permalink raw reply [flat|nested] 18+ messages in thread
* [RFA 09/10] Remove typep and VEC(typep) from linespec.c
2018-04-01 16:35 [RFA 00/10] Remove some cleanups from linespec.c Tom Tromey
` (6 preceding siblings ...)
2018-04-01 16:35 ` [RFA 01/10] Remove some cleanups from search_minsyms_for_name Tom Tromey
@ 2018-04-01 16:35 ` Tom Tromey
2018-04-01 16:35 ` [RFA 02/10] Fix some indentation in linespec.c Tom Tromey
` (2 subsequent siblings)
10 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2018-04-01 16:35 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This removes VEC(typep) from linespec.c in favor of std::vector. It
also removes the "typep" typedef. This change allowed the removal of
some cleanups.
I believe the previous cleanup code in find_superclass_methods could
result in a memory leak, so this patch is an improvement in that way
as well.
2018-03-31 Tom Tromey <tom@tromey.com>
* linespec.c (typep): Remove typedef.
(find_methods, find_superclass_methods): Take a std::vector.
(find_method): Use std::vector.
---
gdb/ChangeLog | 6 ++++++
gdb/linespec.c | 35 ++++++++++-------------------------
2 files changed, 16 insertions(+), 25 deletions(-)
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 96a3117293..1cd79d2da9 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -80,9 +80,6 @@ enum class linespec_complete_what
typedef struct symbol *symbolp;
DEF_VEC_P (symbolp);
-typedef struct type *typep;
-DEF_VEC_P (typep);
-
/* An address entry is used to ensure that any given location is only
added to the result a single time. It holds an address and the
program space from which the address came. */
@@ -1212,7 +1209,7 @@ iterate_over_file_blocks
static void
find_methods (struct type *t, enum language t_lang, const char *name,
std::vector<const char *> *result_names,
- VEC (typep) **superclasses)
+ std::vector<struct type *> *superclasses)
{
int ibase;
const char *class_name = type_name_no_tag (t);
@@ -1273,7 +1270,7 @@ find_methods (struct type *t, enum language t_lang, const char *name,
}
for (ibase = 0; ibase < TYPE_N_BASECLASSES (t); ibase++)
- VEC_safe_push (typep, *superclasses, TYPE_BASECLASS (t, ibase));
+ superclasses->push_back (TYPE_BASECLASS (t, ibase));
}
/* Find an instance of the character C in the string S that is outside
@@ -3649,32 +3646,24 @@ add_all_symbol_names_from_pspace (struct collect_info *info,
}
static void
-find_superclass_methods (VEC (typep) *superclasses,
+find_superclass_methods (std::vector<struct type *> &&superclasses,
const char *name, enum language name_lang,
std::vector<const char *> *result_names)
{
size_t old_len = result_names->size ();
- VEC (typep) *iter_classes;
- struct cleanup *cleanup = make_cleanup (null_cleanup, NULL);
- iter_classes = superclasses;
while (1)
{
- VEC (typep) *new_supers = NULL;
- int ix;
- struct type *t;
+ std::vector<struct type *> new_supers;
- make_cleanup (VEC_cleanup (typep), &new_supers);
- for (ix = 0; VEC_iterate (typep, iter_classes, ix, t); ++ix)
+ for (struct type *t : superclasses)
find_methods (t, name_lang, name, result_names, &new_supers);
- if (result_names->size () != old_len || VEC_empty (typep, new_supers))
+ if (result_names->size () != old_len || new_supers.empty ())
break;
- iter_classes = new_supers;
+ superclasses = std::move (new_supers);
}
-
- do_cleanups (cleanup);
}
/* This finds the method METHOD_NAME in the class CLASS_NAME whose type is
@@ -3688,10 +3677,9 @@ find_method (struct linespec_state *self, VEC (symtab_ptr) *file_symtabs,
VEC (bound_minimal_symbol_d) **minsyms)
{
struct symbol *sym;
- struct cleanup *cleanup = make_cleanup (null_cleanup, NULL);
int ix;
size_t last_result_len;
- VEC (typep) *superclass_vec;
+ std::vector<struct type *> superclass_vec;
std::vector<const char *> result_names;
struct collect_info info;
@@ -3716,8 +3704,6 @@ find_method (struct linespec_state *self, VEC (symtab_ptr) *file_symtabs,
those names. This loop is written in a somewhat funny way
because we collect data across the program space before deciding
what to do. */
- superclass_vec = NULL;
- make_cleanup (VEC_cleanup (typep), &superclass_vec);
last_result_len = 0;
for (ix = 0; VEC_iterate (symbolp, sym_classes, ix, sym); ++ix)
{
@@ -3743,7 +3729,7 @@ find_method (struct linespec_state *self, VEC (symtab_ptr) *file_symtabs,
/* If we did not find a direct implementation anywhere in
this program space, consider superclasses. */
if (result_names.size () == last_result_len)
- find_superclass_methods (superclass_vec, method_name,
+ find_superclass_methods (std::move (superclass_vec), method_name,
SYMBOL_LANGUAGE (sym), &result_names);
/* We have a list of candidate symbol names, so now we
@@ -3752,7 +3738,7 @@ find_method (struct linespec_state *self, VEC (symtab_ptr) *file_symtabs,
add_all_symbol_names_from_pspace (&info, pspace, result_names,
FUNCTIONS_DOMAIN);
- VEC_truncate (typep, superclass_vec, 0);
+ superclass_vec.clear ();
last_result_len = result_names.size ();
}
}
@@ -3762,7 +3748,6 @@ find_method (struct linespec_state *self, VEC (symtab_ptr) *file_symtabs,
{
*symbols = info.result.symbols;
*minsyms = info.result.minimal_symbols;
- do_cleanups (cleanup);
return;
}
--
2.13.6
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFA 01/10] Remove some cleanups from search_minsyms_for_name
2018-04-01 16:35 ` [RFA 01/10] Remove some cleanups from search_minsyms_for_name Tom Tromey
@ 2018-04-02 1:39 ` Simon Marchi
0 siblings, 0 replies; 18+ messages in thread
From: Simon Marchi @ 2018-04-02 1:39 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 2018-04-01 12:35 PM, Tom Tromey wrote:
> This changes struct collect_minsyms to use a std::vector, which
> enables the removal of a cleanup from search_minsyms_for_name. This
> also changes iterate_over_minimal_symbols to take a
> gdb::function_view, which makes a function in linespec.c more
> type-safe.
>
> gdb/ChangeLog
> 2018-03-31 Tom Tromey <tom@tromey.com>
>
> * minsyms.h (iterate_over_minimal_symbols): Update.
> * minsyms.c (iterate_over_minimal_symbols): Take a
> gdb::function_view.
> * linespec.c (struct collect_minsyms): Add constructor and
> initializers.
> <msyms>: Now a std::vector.
> (compare_msyms): Now a std::sort comparator.
> (add_minsym): Change type of second parameter.
> (search_minsyms_for_name): Update.
> ---
> gdb/ChangeLog | 12 +++++++
> gdb/linespec.c | 112 ++++++++++++++++++++++++++++-----------------------------
> gdb/minsyms.c | 9 ++---
> gdb/minsyms.h | 8 ++---
> 4 files changed, 72 insertions(+), 69 deletions(-)
>
> diff --git a/gdb/linespec.c b/gdb/linespec.c
> index 1236b3f475..bd09f57b05 100644
> --- a/gdb/linespec.c
> +++ b/gdb/linespec.c
> @@ -46,6 +46,7 @@
> #include "location.h"
> #include "common/function-view.h"
> #include "common/def-vector.h"
> +#include <algorithm>
>
> /* An enumeration of the various things a user might attempt to
> complete for a linespec location. */
> @@ -4375,8 +4376,15 @@ minsym_found (struct linespec_state *self, struct objfile *objfile,
>
> struct collect_minsyms
> {
> + collect_minsyms (int funfirstline_, int list_mode_, struct symtab *symtab_)
> + : symtab (symtab_),
> + funfirstline (funfirstline_),
Note, funfirstline is unused, so you might as well remove it.
> + list_mode (list_mode_)
> + {
> + }
> +
> /* The objfile we're examining. */
> - struct objfile *objfile;
> + struct objfile *objfile = nullptr;
>
> /* Only search the given symtab, or NULL to search for all symbols. */
> struct symtab *symtab;
> @@ -4388,7 +4396,7 @@ struct collect_minsyms
> int list_mode;
>
> /* The resulting symbols. */
> - VEC (bound_minimal_symbol_d) *msyms;
> + std::vector<struct bound_minimal_symbol> msyms;
> };
I would lean towards removing the collect_minsyms structure and just pass anything
that's required by parameter to add_minsym. It's easier to follow the flow of
execution when parameters are passed explicitly than through a structure.
>
> /* A helper function to classify a minimal_symbol_type according to
> @@ -4415,47 +4423,43 @@ classify_mtype (enum minimal_symbol_type t)
> }
> }
>
> -/* Callback for qsort that sorts symbols by priority. */
> +/* Callback for std::sort that sorts symbols by priority. */
>
> -static int
> -compare_msyms (const void *a, const void *b)
> +static bool
> +compare_msyms (const bound_minimal_symbol &a, const bound_minimal_symbol &b)
> {
> - const bound_minimal_symbol_d *moa = (const bound_minimal_symbol_d *) a;
> - const bound_minimal_symbol_d *mob = (const bound_minimal_symbol_d *) b;
> - enum minimal_symbol_type ta = MSYMBOL_TYPE (moa->minsym);
> - enum minimal_symbol_type tb = MSYMBOL_TYPE (mob->minsym);
> + enum minimal_symbol_type ta = MSYMBOL_TYPE (a.minsym);
> + enum minimal_symbol_type tb = MSYMBOL_TYPE (b.minsym);
>
> - return classify_mtype (ta) - classify_mtype (tb);
> + return classify_mtype (ta) < classify_mtype (tb);
> }
>
> /* Callback for iterate_over_minimal_symbols that adds the symbol to
> the result. */
This comment should probably updated. It is not really a callback for
iterate_over_minimal_symbols anymore (at least not directly).
>
> static void
> -add_minsym (struct minimal_symbol *minsym, void *d)
> +add_minsym (struct minimal_symbol *minsym, struct collect_minsyms &info)
...
> diff --git a/gdb/minsyms.c b/gdb/minsyms.c
> index 72969b7778..08efa1dc7e 100644
> --- a/gdb/minsyms.c
> +++ b/gdb/minsyms.c
> @@ -471,11 +471,8 @@ linkage_name_str (const lookup_name_info &lookup_name)
> void
> iterate_over_minimal_symbols (struct objfile *objf,
> const lookup_name_info &lookup_name,
> - void (*callback) (struct minimal_symbol *,
> - void *),
> - void *user_data)
> + gdb::function_view<void (struct minimal_symbol *)> callback)
This line is too long, you should probably wrap the whole parameter list.
> {
> -
> /* The first pass is over the ordinary hash table. */
> {
> const char *name = linkage_name_str (lookup_name);
> @@ -490,7 +487,7 @@ iterate_over_minimal_symbols (struct objfile *objf,
> iter = iter->hash_next)
> {
> if (mangled_cmp (MSYMBOL_LINKAGE_NAME (iter), name) == 0)
> - (*callback) (iter, user_data);
> + callback (iter);
> }
> }
>
> @@ -509,7 +506,7 @@ iterate_over_minimal_symbols (struct objfile *objf,
> iter != NULL;
> iter = iter->demangled_hash_next)
> if (name_match (MSYMBOL_SEARCH_NAME (iter), lookup_name, NULL))
> - (*callback) (iter, user_data);
> + callback (iter);
> }
> }
>
> diff --git a/gdb/minsyms.h b/gdb/minsyms.h
> index 11a202025d..b05f717575 100644
> --- a/gdb/minsyms.h
> +++ b/gdb/minsyms.h
> @@ -268,11 +268,9 @@ struct bound_minimal_symbol lookup_minimal_symbol_by_pc (CORE_ADDR);
> For each matching symbol, CALLBACK is called with the symbol and
> USER_DATA as arguments. */
This comment should be updated.
>
> -void iterate_over_minimal_symbols (struct objfile *objf,
> - const lookup_name_info &name,
> - void (*callback) (struct minimal_symbol *,
> - void *),
> - void *user_data);
> +void iterate_over_minimal_symbols
> + (struct objfile *objf, const lookup_name_info &name,
> + gdb::function_view<void (struct minimal_symbol *)> callback);
>
> /* Compute the upper bound of MINSYM. The upper bound is the last
> address thought to be part of the symbol. If the symbol has a
>
Thanks,
Simon
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFA 02/10] Fix some indentation in linespec.c
2018-04-01 16:35 ` [RFA 02/10] Fix some indentation in linespec.c Tom Tromey
@ 2018-04-02 1:49 ` Simon Marchi
0 siblings, 0 replies; 18+ messages in thread
From: Simon Marchi @ 2018-04-02 1:49 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 2018-04-01 12:35 PM, Tom Tromey wrote:
> This removes some leftover comments and fixes the indentation in a
> couple of spots in linespec.c.
Thanks, git show -w shows that it is trivial. There are three lines that become
too long though. Feel free to push it by itself.
> gdb/ChangeLog
> 2018-03-31 Tom Tromey <tom@tromey.com>
>
> * linespec.c (linespec_parse_basic): Reindent.
> ---
> gdb/ChangeLog | 4 ++
> gdb/linespec.c | 136 ++++++++++++++++++++++++++++-----------------------------
> 2 files changed, 70 insertions(+), 70 deletions(-)
>
> diff --git a/gdb/linespec.c b/gdb/linespec.c
> index bd09f57b05..9273dcde88 100644
> --- a/gdb/linespec.c
> +++ b/gdb/linespec.c
> @@ -1897,68 +1897,66 @@ linespec_parse_basic (linespec_parser *parser)
> }
> else
> {
> - /* XXX Reindent before pushing. */
> -
> - /* Try looking it up as a function/method. */
> - find_linespec_symbols (PARSER_STATE (parser),
> - PARSER_RESULT (parser)->file_symtabs, name,
> - PARSER_EXPLICIT (parser)->func_name_match_type,
> - &symbols, &minimal_symbols);
> + /* Try looking it up as a function/method. */
> + find_linespec_symbols (PARSER_STATE (parser),
> + PARSER_RESULT (parser)->file_symtabs, name,
> + PARSER_EXPLICIT (parser)->func_name_match_type,
> + &symbols, &minimal_symbols);
>
> - if (symbols != NULL || minimal_symbols != NULL)
> - {
> - PARSER_RESULT (parser)->function_symbols = symbols;
> - PARSER_RESULT (parser)->minimal_symbols = minimal_symbols;
> - PARSER_EXPLICIT (parser)->function_name = name;
> - symbols = NULL;
> - discard_cleanups (cleanup);
> - }
> - else
> - {
> - /* NAME was not a function or a method. So it must be a label
> - name or user specified variable like "break foo.c:$zippo". */
> - labels = find_label_symbols (PARSER_STATE (parser), NULL,
> - &symbols, name);
> - if (labels != NULL)
> + if (symbols != NULL || minimal_symbols != NULL)
> {
> - PARSER_RESULT (parser)->labels.label_symbols = labels;
> - PARSER_RESULT (parser)->labels.function_symbols = symbols;
> - PARSER_EXPLICIT (parser)->label_name = name;
> + PARSER_RESULT (parser)->function_symbols = symbols;
> + PARSER_RESULT (parser)->minimal_symbols = minimal_symbols;
> + PARSER_EXPLICIT (parser)->function_name = name;
> symbols = NULL;
> discard_cleanups (cleanup);
> }
> - else if (token.type == LSTOKEN_STRING
> - && *LS_TOKEN_STOKEN (token).ptr == '$')
> + else
> {
> - /* User specified a convenience variable or history value. */
> - PARSER_EXPLICIT (parser)->line_offset
> - = linespec_parse_variable (PARSER_STATE (parser), name);
> + /* NAME was not a function or a method. So it must be a label
> + name or user specified variable like "break foo.c:$zippo". */
> + labels = find_label_symbols (PARSER_STATE (parser), NULL,
> + &symbols, name);
> + if (labels != NULL)
> + {
> + PARSER_RESULT (parser)->labels.label_symbols = labels;
> + PARSER_RESULT (parser)->labels.function_symbols = symbols;
> + PARSER_EXPLICIT (parser)->label_name = name;
> + symbols = NULL;
> + discard_cleanups (cleanup);
> + }
> + else if (token.type == LSTOKEN_STRING
> + && *LS_TOKEN_STOKEN (token).ptr == '$')
> + {
> + /* User specified a convenience variable or history value. */
> + PARSER_EXPLICIT (parser)->line_offset
> + = linespec_parse_variable (PARSER_STATE (parser), name);
>
> - if (PARSER_EXPLICIT (parser)->line_offset.sign == LINE_OFFSET_UNKNOWN)
> + if (PARSER_EXPLICIT (parser)->line_offset.sign == LINE_OFFSET_UNKNOWN)
Here...
> + {
> + /* The user-specified variable was not valid. Do not
> + throw an error here. parse_linespec will do it for us. */
> + PARSER_EXPLICIT (parser)->function_name = name;
> + discard_cleanups (cleanup);
> + return;
> + }
> +
> + /* The convenience variable/history value parsed correctly.
> + NAME is no longer needed. */
> + do_cleanups (cleanup);
> + }
> + else
> {
> - /* The user-specified variable was not valid. Do not
> - throw an error here. parse_linespec will do it for us. */
> + /* The name is also not a label. Abort parsing. Do not throw
> + an error here. parse_linespec will do it for us. */
> +
> + /* Save a copy of the name we were trying to lookup. */
> PARSER_EXPLICIT (parser)->function_name = name;
> discard_cleanups (cleanup);
> return;
> }
> -
> - /* The convenience variable/history value parsed correctly.
> - NAME is no longer needed. */
> - do_cleanups (cleanup);
> - }
> - else
> - {
> - /* The name is also not a label. Abort parsing. Do not throw
> - an error here. parse_linespec will do it for us. */
> -
> - /* Save a copy of the name we were trying to lookup. */
> - PARSER_EXPLICIT (parser)->function_name = name;
> - discard_cleanups (cleanup);
> - return;
> }
> }
> - }
>
> int previous_qc = parser->completion_quote_char;
>
> @@ -2027,29 +2025,27 @@ linespec_parse_basic (linespec_parser *parser)
> }
> else
> {
> - /* XXX Reindent before pushing. */
> -
> - /* Grab a copy of the label's name and look it up. */
> - name = copy_token_string (token);
> - cleanup = make_cleanup (xfree, name);
> - labels = find_label_symbols (PARSER_STATE (parser),
> - PARSER_RESULT (parser)->function_symbols,
> - &symbols, name);
> + /* Grab a copy of the label's name and look it up. */
> + name = copy_token_string (token);
> + cleanup = make_cleanup (xfree, name);
> + labels = find_label_symbols (PARSER_STATE (parser),
> + PARSER_RESULT (parser)->function_symbols,
... here ...
> + &symbols, name);
>
> - if (labels != NULL)
> - {
> - PARSER_RESULT (parser)->labels.label_symbols = labels;
> - PARSER_RESULT (parser)->labels.function_symbols = symbols;
> - PARSER_EXPLICIT (parser)->label_name = name;
> - symbols = NULL;
> - discard_cleanups (cleanup);
> - }
> - else
> - {
> - /* We don't know what it was, but it isn't a label. */
> - undefined_label_error (PARSER_EXPLICIT (parser)->function_name,
> - name);
> - }
> + if (labels != NULL)
> + {
> + PARSER_RESULT (parser)->labels.label_symbols = labels;
> + PARSER_RESULT (parser)->labels.function_symbols = symbols;
> + PARSER_EXPLICIT (parser)->label_name = name;
> + symbols = NULL;
> + discard_cleanups (cleanup);
> + }
> + else
> + {
> + /* We don't know what it was, but it isn't a label. */
> + undefined_label_error (PARSER_EXPLICIT (parser)->function_name,
... and here.
Simon
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFA 04/10] Return std::string from canonical_to_fullform
2018-04-01 16:35 ` [RFA 04/10] Return std::string from canonical_to_fullform Tom Tromey
@ 2018-04-02 2:15 ` Simon Marchi
0 siblings, 0 replies; 18+ messages in thread
From: Simon Marchi @ 2018-04-02 2:15 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 2018-04-01 12:35 PM, Tom Tromey wrote:
> @@ -1458,34 +1453,39 @@ convert_results_to_lsals (struct linespec_state *self,
>
> struct decode_line_2_item
> {
> - /* The form using symtab_to_fullname.
> - It must be xfree'ed after use. */
> - char *fullform;
> + decode_line_2_item (std::string &&fullform_, std::string &&displayform_,
> + bool selected_)
> + : fullform (std::move (fullform_)),
> + displayform (std::move (displayform_)),
> + selected (selected_)
> + {
> + }
> +
> + /* The form using symtab_to_fullname. */
> + std::string fullform;
>
> - /* The form using symtab_to_filename_for_display.
> - It must be xfree'ed after use. */
> - char *displayform;
> + /* The form using symtab_to_filename_for_display. */
> + std::string displayform;
>
> /* Field is initialized to zero and it is set to one if the user
> requested breakpoint for this entry. */
> unsigned int selected : 1;
> };
>
> -/* Helper for qsort to sort decode_line_2_item entries by DISPLAYFORM and
> - secondarily by FULLFORM. */
> +/* Helper for std::sort to sort decode_line_2_item entries by
> + DISPLAYFORM and secondarily by FULLFORM. */
>
> -static int
> -decode_line_2_compare_items (const void *ap, const void *bp)
> +static bool
> +decode_line_2_compare_items (const decode_line_2_item &a,
> + const decode_line_2_item &b)
> {
> - const struct decode_line_2_item *a = (const struct decode_line_2_item *) ap;
> - const struct decode_line_2_item *b = (const struct decode_line_2_item *) bp;
> int retval;
>
> - retval = strcmp (a->displayform, b->displayform);
> - if (retval != 0)
> - return retval;
> -
> - return strcmp (a->fullform, b->fullform);
> + if (a.displayform < b.displayform)
> + return true;
> + if (a.displayform == b.displayform)
> + return a.fullform < b.fullform;
> + return false;
It's probably a matter of opinion, but I think this would read better like this:
if (a.displayform != b.displayform)
return a.displayform < b.displayform;
return a.fullform < b.fullform;
In any case, the retval variable can be removed.
Otherwise, LGTM.
Simon
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFA 07/10] Change streq to return bool
2018-04-01 16:35 ` [RFA 07/10] Change streq to return bool Tom Tromey
@ 2018-04-02 2:28 ` Simon Marchi
0 siblings, 0 replies; 18+ messages in thread
From: Simon Marchi @ 2018-04-02 2:28 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 2018-04-01 12:35 PM, Tom Tromey wrote:
> diff --git a/gdb/utils.h b/gdb/utils.h
> index 0de0fe2baa..7c211391e0 100644
> --- a/gdb/utils.h
> +++ b/gdb/utils.h
> @@ -89,7 +89,15 @@ extern int strcmp_iw (const char *string1, const char *string2);
>
> extern int strcmp_iw_ordered (const char *, const char *);
>
> -extern int streq (const char *, const char *);
> +/* Return true if the strings are equal. This uses strcmp but inverts
> + the result. */
Just a nit:
Personally, I wouldn't include the second sentence. These comments should
not discuss the internal implementation of functions if it's not necessary.
Simon
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFA 08/10] More use of std::vector in linespec.c
2018-04-01 16:35 ` [RFA 08/10] More use of std::vector in linespec.c Tom Tromey
@ 2018-04-02 2:35 ` Simon Marchi
0 siblings, 0 replies; 18+ messages in thread
From: Simon Marchi @ 2018-04-02 2:35 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 2018-04-01 12:35 PM, Tom Tromey wrote:
> This changes some spots in linespec.c to take a std::vector. This
> patch spilled out to objc-lang.c a bit as well. This change allows
> for the removal of some cleanups.
>
> 2018-03-31 Tom Tromey <tom@tromey.com>
>
> * utils.c (compare_strings): Remove.
> * utils.h (compare_strings): Remove.
> * objc-lang.h (find_imps): Update.
> * objc-lang.c (find_methods): Take a std::vector.
> (uniquify_strings, find_imps): Likewise.
> * linespec.c (find_methods): Take a std::vector.
> (decode_objc): Use std::vector.
> (add_all_symbol_names_from_pspace, find_superclass_methods): Take
> a std::vector.
> (find_method, find_function_symbols): Use std::vector.
> ---
> gdb/ChangeLog | 13 +++++++++++++
> gdb/linespec.c | 47 ++++++++++++++++++-----------------------------
> gdb/objc-lang.c | 44 ++++++++++++--------------------------------
> gdb/objc-lang.h | 4 ++--
> gdb/utils.c | 11 -----------
> gdb/utils.h | 1 -
> 6 files changed, 45 insertions(+), 75 deletions(-)
>
> diff --git a/gdb/linespec.c b/gdb/linespec.c
> index 48168eaaa6..96a3117293 100644
> --- a/gdb/linespec.c
> +++ b/gdb/linespec.c
> @@ -379,7 +379,7 @@ static void add_matching_symbols_to_info (const char *name,
>
> static void add_all_symbol_names_from_pspace (struct collect_info *info,
> struct program_space *pspace,
> - VEC (const_char_ptr) *names,
> + const std::vector<const char *> &names,
This line is now too long, otherwise LGTM.
Simon
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFA 00/10] Remove some cleanups from linespec.c
2018-04-01 16:35 [RFA 00/10] Remove some cleanups from linespec.c Tom Tromey
` (9 preceding siblings ...)
2018-04-01 16:35 ` [RFA 06/10] Remove a string copy from event_location_to_sals Tom Tromey
@ 2018-04-02 2:44 ` Simon Marchi
2018-04-03 22:31 ` Tom Tromey
10 siblings, 1 reply; 18+ messages in thread
From: Simon Marchi @ 2018-04-02 2:44 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 2018-04-01 12:35 PM, Tom Tromey wrote:
> This series removes many (but not all) cleanups from linespec.c.
> Generally the removals are done in the normal way: replacing manual
> memory management with a self-managing data structure.
>
> I've tried to make each patch relatively small to make them simpler to
> review.
>
> In a few cases the patch required changes outside of linespec.c.
>
> A couple of the patches (at least #2 and #10) are obvious, though of
> course it doesn't hurt to read them anyhow.
>
> Regression tested by the buildbot. I've also built each patch in the
> series locally and run it through the gdb.linespec tests, while I was
> tracking down the failures described in patch #7 (though of course the
> series has changed slightly since then).
>
> Tom
>
Hi Tom,
The patches I haven't commented on LGTM.
Thanks!
Simon
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFA 00/10] Remove some cleanups from linespec.c
2018-04-02 2:44 ` [RFA 00/10] Remove some cleanups from linespec.c Simon Marchi
@ 2018-04-03 22:31 ` Tom Tromey
0 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2018-04-03 22:31 UTC (permalink / raw)
To: Simon Marchi; +Cc: Tom Tromey, gdb-patches
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
Simon> The patches I haven't commented on LGTM.
Thanks. I've updated the patches and I'll send a new version once I run
it through the buildbot.
Tom
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2018-04-03 22:31 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-01 16:35 [RFA 00/10] Remove some cleanups from linespec.c Tom Tromey
2018-04-01 16:35 ` [RFA 07/10] Change streq to return bool Tom Tromey
2018-04-02 2:28 ` Simon Marchi
2018-04-01 16:35 ` [RFA 03/10] Make copy_token_string return unique_xmalloc_ptr Tom Tromey
2018-04-01 16:35 ` [RFA 08/10] More use of std::vector in linespec.c Tom Tromey
2018-04-02 2:35 ` Simon Marchi
2018-04-01 16:35 ` [RFA 10/10] Remove unnecessary include from linespec.h Tom Tromey
2018-04-01 16:35 ` [RFA 04/10] Return std::string from canonical_to_fullform Tom Tromey
2018-04-02 2:15 ` Simon Marchi
2018-04-01 16:35 ` [RFA 05/10] Have filter_results take a std::vector Tom Tromey
2018-04-01 16:35 ` [RFA 01/10] Remove some cleanups from search_minsyms_for_name Tom Tromey
2018-04-02 1:39 ` Simon Marchi
2018-04-01 16:35 ` [RFA 09/10] Remove typep and VEC(typep) from linespec.c Tom Tromey
2018-04-01 16:35 ` [RFA 02/10] Fix some indentation in linespec.c Tom Tromey
2018-04-02 1:49 ` Simon Marchi
2018-04-01 16:35 ` [RFA 06/10] Remove a string copy from event_location_to_sals Tom Tromey
2018-04-02 2:44 ` [RFA 00/10] Remove some cleanups from linespec.c Simon Marchi
2018-04-03 22:31 ` Tom Tromey
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).