* [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
@ 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
` (9 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 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
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
` (8 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* 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
* [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
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-01 16:35 ` Tom Tromey
2018-04-01 16:35 ` [RFA 02/10] Fix some indentation in linespec.c Tom Tromey
` (7 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* [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
` (2 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
` (6 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 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* 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
* [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
` (3 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-01 16:35 ` [RFA 07/10] Change streq to return bool Tom Tromey
` (5 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
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 07/10] Change streq to return bool
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 06/10] Remove a string copy from event_location_to_sals 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
` (4 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* 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
* [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
` (5 preceding siblings ...)
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 04/10] Return std::string from canonical_to_fullform Tom Tromey
` (3 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 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
` (6 preceding siblings ...)
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:15 ` Simon Marchi
2018-04-01 16:35 ` [RFA 10/10] Remove unnecessary include from linespec.h Tom Tromey
` (2 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* 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
* [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
` (7 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 08/10] More use of std::vector in linespec.c 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
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 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
` (8 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:35 ` Simon Marchi
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 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* 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 08/10] More use of std::vector in linespec.c 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