* [RFA 06/11] Change some gdb_* functions to use a std::string out parameter
2017-09-12 18:57 more cleanup removal, particularly in MI Tom Tromey
@ 2017-09-12 18:57 ` Tom Tromey
2017-09-28 9:42 ` Pedro Alves
2017-09-12 18:57 ` [RFA 09/11] Use std::set in mi-main.c Tom Tromey
` (10 subsequent siblings)
11 siblings, 1 reply; 35+ messages in thread
From: Tom Tromey @ 2017-09-12 18:57 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This changes a few gdb_* functions to use a std::string out parameter.
Perhaps these functions should just go away entirely; I think they're
vesitiges of a now-defunct libgdb plan.
ChangeLog
2017-09-12 Tom Tromey <tom@tromey.com>
* thread.c (gdb_list_thread_ids, gdb_thread_select): Change
error_message to std::string.
* mi/mi-main.c (mi_cmd_thread_select): Use std::string.
(mi_cmd_thread_list_ids): Likewise.
* gdb.h (gdb_breakpoint_query, gdb_thread_select)
(gdb_list_thread_ids): Update.
* exceptions.h (catch_exceptions_with_msg): Update.
* exceptions.c (catch_exceptions_with_msg): Change error_message
to std::string.
* breakpoint.c (enum gdb_rc): Change error_message to
std::string.
---
gdb/ChangeLog | 14 ++++++++++++++
gdb/breakpoint.c | 2 +-
gdb/exceptions.c | 11 +++--------
gdb/exceptions.h | 2 +-
gdb/gdb.h | 6 +++---
gdb/mi/mi-main.c | 14 ++++----------
gdb/thread.c | 4 ++--
7 files changed, 28 insertions(+), 25 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index e1c409d..47632fa 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,19 @@
2017-09-12 Tom Tromey <tom@tromey.com>
+ * thread.c (gdb_list_thread_ids, gdb_thread_select): Change
+ error_message to std::string.
+ * mi/mi-main.c (mi_cmd_thread_select): Use std::string.
+ (mi_cmd_thread_list_ids): Likewise.
+ * gdb.h (gdb_breakpoint_query, gdb_thread_select)
+ (gdb_list_thread_ids): Update.
+ * exceptions.h (catch_exceptions_with_msg): Update.
+ * exceptions.c (catch_exceptions_with_msg): Change error_message
+ to std::string.
+ * breakpoint.c (enum gdb_rc): Change error_message to
+ std::string.
+
+2017-09-12 Tom Tromey <tom@tromey.com>
+
* mi/mi-parse.c (mi_parse): Remove unused declaration.
2017-09-12 Tom Tromey <tom@tromey.com>
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 235dab4..4c58a8c 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -6750,7 +6750,7 @@ do_captured_breakpoint_query (struct ui_out *uiout, void *data)
enum gdb_rc
gdb_breakpoint_query (struct ui_out *uiout, int bnum,
- char **error_message)
+ std::string *error_message)
{
struct captured_breakpoint_query_args args;
diff --git a/gdb/exceptions.c b/gdb/exceptions.c
index f9a80a0..0385bec 100644
--- a/gdb/exceptions.c
+++ b/gdb/exceptions.c
@@ -167,7 +167,7 @@ int
catch_exceptions_with_msg (struct ui_out *func_uiout,
catch_exceptions_ftype *func,
void *func_args,
- char **gdberrmsg,
+ std::string *gdberrmsg,
return_mask mask)
{
struct gdb_exception exception = exception_none;
@@ -206,13 +206,8 @@ catch_exceptions_with_msg (struct ui_out *func_uiout,
/* If caller wants a copy of the low-level error message, make
one. This is used in the case of a silent error whereby the
caller may optionally want to issue the message. */
- if (gdberrmsg != NULL)
- {
- if (exception.message != NULL)
- *gdberrmsg = xstrdup (exception.message);
- else
- *gdberrmsg = NULL;
- }
+ if (gdberrmsg != NULL && exception.message != NULL)
+ *gdberrmsg = exception.message;
return exception.reason;
}
return val;
diff --git a/gdb/exceptions.h b/gdb/exceptions.h
index b2cdee3..e1a517b 100644
--- a/gdb/exceptions.h
+++ b/gdb/exceptions.h
@@ -73,7 +73,7 @@ typedef void (catch_exception_ftype) (struct ui_out *ui_out, void *args);
extern int catch_exceptions_with_msg (struct ui_out *uiout,
catch_exceptions_ftype *func,
void *func_args,
- char **gdberrmsg,
+ std::string *gdberrmsg,
return_mask mask);
/* If CATCH_ERRORS_FTYPE throws an error, catch_errors() returns zero
diff --git a/gdb/gdb.h b/gdb/gdb.h
index ac1e683..9403823 100644
--- a/gdb/gdb.h
+++ b/gdb/gdb.h
@@ -45,14 +45,14 @@ enum gdb_rc {
/* Print the specified breakpoint on GDB_STDOUT. (Eventually this
function will ``print'' the object on ``output''). */
enum gdb_rc gdb_breakpoint_query (struct ui_out *uiout, int bnum,
- char **error_message);
+ std::string *error_message);
/* Switch thread and print notification. */
enum gdb_rc gdb_thread_select (struct ui_out *uiout, char *tidstr,
- char **error_message);
+ std::string *error_message);
/* Print a list of known thread ids. */
enum gdb_rc gdb_list_thread_ids (struct ui_out *uiout,
- char **error_message);
+ std::string *error_message);
#endif
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 0ee2605..d01f578 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -554,7 +554,7 @@ void
mi_cmd_thread_select (const char *command, char **argv, int argc)
{
enum gdb_rc rc;
- char *mi_error_message;
+ std::string mi_error_message;
ptid_t previous_ptid = inferior_ptid;
if (argc != 1)
@@ -564,10 +564,7 @@ mi_cmd_thread_select (const char *command, char **argv, int argc)
/* If thread switch did not succeed don't notify or print. */
if (rc == GDB_RC_FAIL)
- {
- make_cleanup (xfree, mi_error_message);
- error ("%s", mi_error_message);
- }
+ error ("%s", mi_error_message.c_str ());
print_selected_thread_frame (current_uiout,
USER_SELECTED_THREAD | USER_SELECTED_FRAME);
@@ -584,7 +581,7 @@ void
mi_cmd_thread_list_ids (const char *command, char **argv, int argc)
{
enum gdb_rc rc;
- char *mi_error_message;
+ std::string mi_error_message;
if (argc != 0)
error (_("-thread-list-ids: No arguments required."));
@@ -592,10 +589,7 @@ mi_cmd_thread_list_ids (const char *command, char **argv, int argc)
rc = gdb_list_thread_ids (current_uiout, &mi_error_message);
if (rc == GDB_RC_FAIL)
- {
- make_cleanup (xfree, mi_error_message);
- error ("%s", mi_error_message);
- }
+ error ("%s", mi_error_message.c_str ());
}
void
diff --git a/gdb/thread.c b/gdb/thread.c
index 2539d43..a378c13 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -744,7 +744,7 @@ do_captured_list_thread_ids (struct ui_out *uiout, void *arg)
/* Official gdblib interface function to get a list of thread ids and
the total number. */
enum gdb_rc
-gdb_list_thread_ids (struct ui_out *uiout, char **error_message)
+gdb_list_thread_ids (struct ui_out *uiout, std::string *error_message)
{
if (catch_exceptions_with_msg (uiout, do_captured_list_thread_ids, NULL,
error_message, RETURN_MASK_ALL) < 0)
@@ -2056,7 +2056,7 @@ print_selected_thread_frame (struct ui_out *uiout,
}
enum gdb_rc
-gdb_thread_select (struct ui_out *uiout, char *tidstr, char **error_message)
+gdb_thread_select (struct ui_out *uiout, char *tidstr, std::string *error_message)
{
if (catch_exceptions_with_msg (uiout, do_captured_thread_select, tidstr,
error_message, RETURN_MASK_ALL) < 0)
--
2.9.4
^ permalink raw reply [flat|nested] 35+ messages in thread
* [RFA 09/11] Use std::set in mi-main.c
2017-09-12 18:57 more cleanup removal, particularly in MI Tom Tromey
2017-09-12 18:57 ` [RFA 06/11] Change some gdb_* functions to use a std::string out parameter Tom Tromey
@ 2017-09-12 18:57 ` Tom Tromey
2017-09-28 10:10 ` Pedro Alves
2017-10-03 11:21 ` Simon Marchi
2017-09-12 18:57 ` [RFA 11/11] Change captured_mi_execute_command to use scoped_restore Tom Tromey
` (9 subsequent siblings)
11 siblings, 2 replies; 35+ messages in thread
From: Tom Tromey @ 2017-09-12 18:57 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
Change a couple of spots in mi-main.c to use std::set. This
simplifies the code and removes some cleanups.
ChangeLog
2017-09-12 Tom Tromey <tom@tromey.com>
* mi/mi-main.c (struct print_one_inferior_data) <inferiors>: Now a
'std::set *'.
(print_one_inferior): Update.
(free_vector_of_ints): Remove.
(list_available_thread_groups): Change "ids" to std::set.
(mi_cmd_list_thread_groups): Update.
(struct collect_cores_data) <core>: Now a std::set.
(collect_cores): Update.
(unique): Remove.
(print_one_inferior): Update.
---
gdb/ChangeLog | 13 ++++++++++
gdb/mi/mi-main.c | 77 +++++++++++++-------------------------------------------
2 files changed, 31 insertions(+), 59 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 6368f41..54928fc 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,18 @@
2017-09-12 Tom Tromey <tom@tromey.com>
+ * mi/mi-main.c (struct print_one_inferior_data) <inferiors>: Now a
+ 'std::set *'.
+ (print_one_inferior): Update.
+ (free_vector_of_ints): Remove.
+ (list_available_thread_groups): Change "ids" to std::set.
+ (mi_cmd_list_thread_groups): Update.
+ (struct collect_cores_data) <core>: Now a std::set.
+ (collect_cores): Update.
+ (unique): Remove.
+ (print_one_inferior): Update.
+
+2017-09-12 Tom Tromey <tom@tromey.com>
+
* mi/mi-main.c (mi_execute_cli_command): Use unique_xmalloc_ptr.
(mi_execute_async_cli_command): Likewise.
(mi_cmd_trace_frame_collected): Use std::string.
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 0147fb9..3d73446 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -62,6 +62,8 @@
#include <chrono>
#include "progspace-and-thread.h"
#include "common/rsp-low.h"
+#include <algorithm>
+#include <set>
enum
{
@@ -604,8 +606,7 @@ mi_cmd_thread_info (const char *command, char **argv, int argc)
struct collect_cores_data
{
int pid;
-
- VEC (int) *cores;
+ std::set<int> cores;
};
static int
@@ -618,27 +619,16 @@ collect_cores (struct thread_info *ti, void *xdata)
int core = target_core_of_thread (ti->ptid);
if (core != -1)
- VEC_safe_push (int, data->cores, core);
+ data->cores.insert (core);
}
return 0;
}
-static int *
-unique (int *b, int *e)
-{
- int *d = b;
-
- while (++b != e)
- if (*d != *b)
- *++d = *b;
- return ++d;
-}
-
struct print_one_inferior_data
{
int recurse;
- VEC (int) *inferiors;
+ const std::set<int> *inferiors;
};
static int
@@ -648,10 +638,9 @@ print_one_inferior (struct inferior *inferior, void *xdata)
= (struct print_one_inferior_data *) xdata;
struct ui_out *uiout = current_uiout;
- if (VEC_empty (int, top_data->inferiors)
- || bsearch (&(inferior->pid), VEC_address (int, top_data->inferiors),
- VEC_length (int, top_data->inferiors), sizeof (int),
- compare_positive_ints))
+ if (top_data->inferiors->empty ()
+ || (top_data->inferiors->find (inferior->pid)
+ != top_data->inferiors->end ()))
{
struct collect_cores_data data;
ui_out_emit_tuple tuple_emitter (uiout, NULL);
@@ -670,28 +659,18 @@ print_one_inferior (struct inferior *inferior, void *xdata)
inferior->pspace->pspace_exec_filename);
}
- data.cores = 0;
if (inferior->pid != 0)
{
data.pid = inferior->pid;
iterate_over_threads (collect_cores, &data);
}
- if (!VEC_empty (int, data.cores))
+ if (!data.cores.empty ())
{
- int *b, *e;
ui_out_emit_list list_emitter (uiout, "cores");
- qsort (VEC_address (int, data.cores),
- VEC_length (int, data.cores), sizeof (int),
- compare_positive_ints);
-
- b = VEC_address (int, data.cores);
- e = b + VEC_length (int, data.cores);
- e = unique (b, e);
-
- for (; b != e; ++b)
- uiout->field_int (NULL, *b);
+ for (int b : data.cores)
+ uiout->field_int (NULL, b);
}
if (top_data->recurse)
@@ -717,14 +696,6 @@ output_cores (struct ui_out *uiout, const char *field_name, const char *xcores)
}
static void
-free_vector_of_ints (void *xvector)
-{
- VEC (int) **vector = (VEC (int) **) xvector;
-
- VEC_free (int, *vector);
-}
-
-static void
do_nothing (splay_tree_key k)
{
}
@@ -755,7 +726,7 @@ free_splay_tree (void *xt)
}
static void
-list_available_thread_groups (VEC (int) *ids, int recurse)
+list_available_thread_groups (const std::set<int> &ids, int recurse)
{
struct osdata *data;
struct osdata_item *item;
@@ -824,12 +795,9 @@ list_available_thread_groups (VEC (int) *ids, int recurse)
/* At present, the target will return all available processes
and if information about specific ones was required, we filter
undesired processes here. */
- if (ids && bsearch (&pid_i, VEC_address (int, ids),
- VEC_length (int, ids),
- sizeof (int), compare_positive_ints) == NULL)
+ if (!ids.empty () && ids.find (pid_i) != ids.end ())
continue;
-
ui_out_emit_tuple tuple_emitter (uiout, NULL);
uiout->field_fmt ("id", "%s", pid);
@@ -875,10 +843,9 @@ void
mi_cmd_list_thread_groups (const char *command, char **argv, int argc)
{
struct ui_out *uiout = current_uiout;
- struct cleanup *back_to;
int available = 0;
int recurse = 0;
- VEC (int) *ids = 0;
+ std::set<int> ids;
enum opt
{
@@ -930,23 +897,17 @@ mi_cmd_list_thread_groups (const char *command, char **argv, int argc)
if (*end != '\0')
error (_("invalid syntax of group id '%s'"), argv[oind]);
- VEC_safe_push (int, ids, inf);
+ ids.insert (inf);
}
- if (VEC_length (int, ids) > 1)
- qsort (VEC_address (int, ids),
- VEC_length (int, ids),
- sizeof (int), compare_positive_ints);
-
- back_to = make_cleanup (free_vector_of_ints, &ids);
if (available)
{
list_available_thread_groups (ids, recurse);
}
- else if (VEC_length (int, ids) == 1)
+ else if (ids.size () == 1)
{
/* Local thread groups, single id. */
- int id = *VEC_address (int, ids);
+ int id = *(ids.begin ());
struct inferior *inf = find_inferior_id (id);
if (!inf)
@@ -959,7 +920,7 @@ mi_cmd_list_thread_groups (const char *command, char **argv, int argc)
struct print_one_inferior_data data;
data.recurse = recurse;
- data.inferiors = ids;
+ data.inferiors = &ids;
/* Local thread groups. Either no explicit ids -- and we
print everything, or several explicit ids. In both cases,
@@ -969,8 +930,6 @@ mi_cmd_list_thread_groups (const char *command, char **argv, int argc)
update_thread_list ();
iterate_over_inferiors (print_one_inferior, &data);
}
-
- do_cleanups (back_to);
}
void
--
2.9.4
^ permalink raw reply [flat|nested] 35+ messages in thread
* [RFA 10/11] Use a std::vector for ada_exceptions_list
2017-09-12 18:57 more cleanup removal, particularly in MI Tom Tromey
` (5 preceding siblings ...)
2017-09-12 18:57 ` [RFA 07/11] Use gdb::byte_vector in mi_cmd_data_write_memory_bytes Tom Tromey
@ 2017-09-12 18:57 ` Tom Tromey
2017-09-28 10:20 ` Pedro Alves
2017-09-12 18:57 ` [RFA 02/11] Remove cleanups from mi_cmd_break_insert_1 Tom Tromey
` (4 subsequent siblings)
11 siblings, 1 reply; 35+ messages in thread
From: Tom Tromey @ 2017-09-12 18:57 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
Change ada_exceptions_list to return a std::vector and fix up the
users. This allows removing a cleanup in MI.
ChangeLog
2017-09-12 Tom Tromey <tom@tromey.com>
* mi/mi-cmd-info.c (mi_cmd_info_ada_exceptions): Update.
* ada-lang.h (struct ada_exc_info): Remove typedef. Declare
operator< and operator==.
(ada_exceptions_list): Return a std::vector.
* ada-lang.c (ada_exc_info::operator<): Rename from
compare_ada_exception_info.
(ada_exc_info::operator==): New.
(sort_remove_dups_ada_exceptions_list): Change type of
"exceptions".
(ada_add_standard_exceptions, ada_add_exceptions_from_frame)
(ada_add_global_exceptions): Likewise.
(ada_exceptions_list_1): Return a std::vector.
(ada_exceptions_list): Likewise.
---
gdb/ChangeLog | 16 ++++++++++
gdb/ada-lang.c | 88 ++++++++++++++++++++--------------------------------
gdb/ada-lang.h | 9 +++---
gdb/mi/mi-cmd-info.c | 17 +++-------
4 files changed, 60 insertions(+), 70 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 54928fc..ee965c4 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,21 @@
2017-09-12 Tom Tromey <tom@tromey.com>
+ * mi/mi-cmd-info.c (mi_cmd_info_ada_exceptions): Update.
+ * ada-lang.h (struct ada_exc_info): Remove typedef. Declare
+ operator< and operator==.
+ (ada_exceptions_list): Return a std::vector.
+ * ada-lang.c (ada_exc_info::operator<): Rename from
+ compare_ada_exception_info.
+ (ada_exc_info::operator==): New.
+ (sort_remove_dups_ada_exceptions_list): Change type of
+ "exceptions".
+ (ada_add_standard_exceptions, ada_add_exceptions_from_frame)
+ (ada_add_global_exceptions): Likewise.
+ (ada_exceptions_list_1): Return a std::vector.
+ (ada_exceptions_list): Likewise.
+
+2017-09-12 Tom Tromey <tom@tromey.com>
+
* mi/mi-main.c (struct print_one_inferior_data) <inferiors>: Now a
'std::set *'.
(print_one_inferior): Update.
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 64f1a33..cafba2d 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -62,6 +62,7 @@
#include "cli/cli-utils.h"
#include "common/function-view.h"
#include "common/byte-vector.h"
+#include <algorithm>
/* Define whether or not the C operator '/' truncates towards zero for
differently signed operands (truncation direction is undefined in C).
@@ -13121,23 +13122,23 @@ ada_is_non_standard_exception_sym (struct symbol *sym)
The comparison is determined first by exception name, and then
by exception address. */
-static int
-compare_ada_exception_info (const void *a, const void *b)
+bool
+ada_exc_info::operator< (const ada_exc_info &other)
{
- const struct ada_exc_info *exc_a = (struct ada_exc_info *) a;
- const struct ada_exc_info *exc_b = (struct ada_exc_info *) b;
int result;
- result = strcmp (exc_a->name, exc_b->name);
- if (result != 0)
- return result;
-
- if (exc_a->addr < exc_b->addr)
- return -1;
- if (exc_a->addr > exc_b->addr)
- return 1;
+ result = strcmp (name, other.name);
+ if (result < 0)
+ return true;
+ if (result == 0 && addr < other.addr)
+ return true;
+ return false;
+}
- return 0;
+bool
+ada_exc_info::operator== (const ada_exc_info &other)
+{
+ return strcmp (name, other.name) == 0 && addr == other.addr;
}
/* Sort EXCEPTIONS using compare_ada_exception_info as the comparison
@@ -13146,23 +13147,12 @@ compare_ada_exception_info (const void *a, const void *b)
All duplicates are also removed. */
static void
-sort_remove_dups_ada_exceptions_list (VEC(ada_exc_info) **exceptions,
+sort_remove_dups_ada_exceptions_list (std::vector<ada_exc_info> *exceptions,
int skip)
{
- struct ada_exc_info *to_sort
- = VEC_address (ada_exc_info, *exceptions) + skip;
- int to_sort_len
- = VEC_length (ada_exc_info, *exceptions) - skip;
- int i, j;
-
- qsort (to_sort, to_sort_len, sizeof (struct ada_exc_info),
- compare_ada_exception_info);
-
- for (i = 1, j = 1; i < to_sort_len; i++)
- if (compare_ada_exception_info (&to_sort[i], &to_sort[j - 1]) != 0)
- to_sort[j++] = to_sort[i];
- to_sort_len = j;
- VEC_truncate(ada_exc_info, *exceptions, skip + to_sort_len);
+ std::sort (exceptions->begin () + skip, exceptions->end ());
+ exceptions->erase (std::unique (exceptions->begin () + skip, exceptions->end ()),
+ exceptions->end ());
}
/* Add all exceptions defined by the Ada standard whose name match
@@ -13177,7 +13167,7 @@ sort_remove_dups_ada_exceptions_list (VEC(ada_exc_info) **exceptions,
static void
ada_add_standard_exceptions (compiled_regex *preg,
- VEC(ada_exc_info) **exceptions)
+ std::vector<ada_exc_info> *exceptions)
{
int i;
@@ -13194,7 +13184,7 @@ ada_add_standard_exceptions (compiled_regex *preg,
struct ada_exc_info info
= {standard_exc[i], BMSYMBOL_VALUE_ADDRESS (msymbol)};
- VEC_safe_push (ada_exc_info, *exceptions, &info);
+ exceptions->push_back (info);
}
}
}
@@ -13213,7 +13203,7 @@ ada_add_standard_exceptions (compiled_regex *preg,
static void
ada_add_exceptions_from_frame (compiled_regex *preg,
struct frame_info *frame,
- VEC(ada_exc_info) **exceptions)
+ std::vector<ada_exc_info> *exceptions)
{
const struct block *block = get_frame_block (frame, 0);
@@ -13236,7 +13226,7 @@ ada_add_exceptions_from_frame (compiled_regex *preg,
struct ada_exc_info info = {SYMBOL_PRINT_NAME (sym),
SYMBOL_VALUE_ADDRESS (sym)};
- VEC_safe_push (ada_exc_info, *exceptions, &info);
+ exceptions->push_back (info);
}
}
}
@@ -13276,7 +13266,7 @@ name_matches_regex (const char *name, compiled_regex *preg)
static void
ada_add_global_exceptions (compiled_regex *preg,
- VEC(ada_exc_info) **exceptions)
+ std::vector<ada_exc_info> *exceptions)
{
struct objfile *objfile;
struct compunit_symtab *s;
@@ -13311,7 +13301,7 @@ ada_add_global_exceptions (compiled_regex *preg,
struct ada_exc_info info
= {SYMBOL_PRINT_NAME (sym), SYMBOL_VALUE_ADDRESS (sym)};
- VEC_safe_push (ada_exc_info, *exceptions, &info);
+ exceptions->push_back (info);
}
}
}
@@ -13323,12 +13313,10 @@ ada_add_global_exceptions (compiled_regex *preg,
If not NULL, PREG is used to filter out exceptions whose names
do not match. Otherwise, all exceptions are listed. */
-static VEC(ada_exc_info) *
+static std::vector<ada_exc_info>
ada_exceptions_list_1 (compiled_regex *preg)
{
- VEC(ada_exc_info) *result = NULL;
- struct cleanup *old_chain
- = make_cleanup (VEC_cleanup (ada_exc_info), &result);
+ std::vector<ada_exc_info> result;
int prev_len;
/* First, list the known standard exceptions. These exceptions
@@ -13342,21 +13330,20 @@ ada_exceptions_list_1 (compiled_regex *preg)
if (has_stack_frames ())
{
- prev_len = VEC_length (ada_exc_info, result);
+ prev_len = result.size ();
ada_add_exceptions_from_frame (preg, get_selected_frame (NULL),
&result);
- if (VEC_length (ada_exc_info, result) > prev_len)
+ if (result.size () > prev_len)
sort_remove_dups_ada_exceptions_list (&result, prev_len);
}
/* Add all exceptions whose scope is global. */
- prev_len = VEC_length (ada_exc_info, result);
+ prev_len = result.size ();
ada_add_global_exceptions (preg, &result);
- if (VEC_length (ada_exc_info, result) > prev_len)
+ if (result.size () > prev_len)
sort_remove_dups_ada_exceptions_list (&result, prev_len);
- discard_cleanups (old_chain);
return result;
}
@@ -13374,7 +13361,7 @@ ada_exceptions_list_1 (compiled_regex *preg)
alphabetical order;
- Exceptions whose scope is global, in alphabetical order. */
-VEC(ada_exc_info) *
+std::vector<ada_exc_info>
ada_exceptions_list (const char *regexp)
{
if (regexp == NULL)
@@ -13389,14 +13376,9 @@ ada_exceptions_list (const char *regexp)
static void
info_exceptions_command (char *regexp, int from_tty)
{
- VEC(ada_exc_info) *exceptions;
- struct cleanup *cleanup;
struct gdbarch *gdbarch = get_current_arch ();
- int ix;
- struct ada_exc_info *info;
- exceptions = ada_exceptions_list (regexp);
- cleanup = make_cleanup (VEC_cleanup (ada_exc_info), &exceptions);
+ std::vector<ada_exc_info> exceptions = ada_exceptions_list (regexp);
if (regexp != NULL)
printf_filtered
@@ -13404,10 +13386,8 @@ info_exceptions_command (char *regexp, int from_tty)
else
printf_filtered (_("All defined Ada exceptions:\n"));
- for (ix = 0; VEC_iterate(ada_exc_info, exceptions, ix, info); ix++)
- printf_filtered ("%s: %s\n", info->name, paddress (gdbarch, info->addr));
-
- do_cleanups (cleanup);
+ for (ada_exc_info &info : exceptions)
+ printf_filtered ("%s: %s\n", info.name, paddress (gdbarch, info.addr));
}
/* Operators */
diff --git a/gdb/ada-lang.h b/gdb/ada-lang.h
index f5b3bca..f92b88d 100644
--- a/gdb/ada-lang.h
+++ b/gdb/ada-lang.h
@@ -379,18 +379,19 @@ extern void create_ada_exception_catchpoint
/* Some information about a given Ada exception. */
-typedef struct ada_exc_info
+struct ada_exc_info
{
/* The name of the exception. */
const char *name;
/* The address of the symbol corresponding to that exception. */
CORE_ADDR addr;
-} ada_exc_info;
-DEF_VEC_O(ada_exc_info);
+ bool operator< (const ada_exc_info &);
+ bool operator== (const ada_exc_info &);
+};
-extern VEC(ada_exc_info) *ada_exceptions_list (const char *regexp);
+extern std::vector<ada_exc_info> ada_exceptions_list (const char *regexp);
/* Tasking-related: ada-tasks.c */
diff --git a/gdb/mi/mi-cmd-info.c b/gdb/mi/mi-cmd-info.c
index fa0d16e..33c64f0 100644
--- a/gdb/mi/mi-cmd-info.c
+++ b/gdb/mi/mi-cmd-info.c
@@ -30,10 +30,6 @@ mi_cmd_info_ada_exceptions (const char *command, char **argv, int argc)
struct ui_out *uiout = current_uiout;
struct gdbarch *gdbarch = get_current_arch ();
char *regexp;
- struct cleanup *old_chain;
- VEC(ada_exc_info) *exceptions;
- int ix;
- struct ada_exc_info *info;
switch (argc)
{
@@ -48,24 +44,21 @@ mi_cmd_info_ada_exceptions (const char *command, char **argv, int argc)
break;
}
- exceptions = ada_exceptions_list (regexp);
- old_chain = make_cleanup (VEC_cleanup (ada_exc_info), &exceptions);
+ std::vector<ada_exc_info> exceptions = ada_exceptions_list (regexp);
ui_out_emit_table table_emitter (uiout, 2,
- VEC_length (ada_exc_info, exceptions),
+ exceptions.size (),
"ada-exceptions");
uiout->table_header (1, ui_left, "name", "Name");
uiout->table_header (1, ui_left, "address", "Address");
uiout->table_body ();
- for (ix = 0; VEC_iterate(ada_exc_info, exceptions, ix, info); ix++)
+ for (ada_exc_info &info : exceptions)
{
ui_out_emit_tuple tuple_emitter (uiout, NULL);
- uiout->field_string ("name", info->name);
- uiout->field_core_addr ("address", gdbarch, info->addr);
+ uiout->field_string ("name", info.name);
+ uiout->field_core_addr ("address", gdbarch, info.addr);
}
-
- do_cleanups (old_chain);
}
/* Implement the "-info-gdb-mi-command" GDB/MI command. */
--
2.9.4
^ permalink raw reply [flat|nested] 35+ messages in thread
* [RFA 04/11] Don't copy a string in mi_cmd_disassemble
2017-09-12 18:57 more cleanup removal, particularly in MI Tom Tromey
` (7 preceding siblings ...)
2017-09-12 18:57 ` [RFA 02/11] Remove cleanups from mi_cmd_break_insert_1 Tom Tromey
@ 2017-09-12 18:57 ` Tom Tromey
2017-09-28 9:40 ` Pedro Alves
2017-09-12 18:59 ` [RFA 01/11] Remove make_cleanup_defer_target_commit_resume Tom Tromey
` (2 subsequent siblings)
11 siblings, 1 reply; 35+ messages in thread
From: Tom Tromey @ 2017-09-12 18:57 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This string copy in mi_cmd_disassemble seems not to be needed, so
don't do it.
ChangeLog
2017-09-12 Tom Tromey <tom@tromey.com>
* mi/mi-cmd-disas.c (mi_cmd_disassemble): Don't copy "oarg".
---
gdb/ChangeLog | 4 ++++
gdb/mi/mi-cmd-disas.c | 6 +-----
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index b1fdcda..c981c7e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,9 @@
2017-09-12 Tom Tromey <tom@tromey.com>
+ * mi/mi-cmd-disas.c (mi_cmd_disassemble): Don't copy "oarg".
+
+2017-09-12 Tom Tromey <tom@tromey.com>
+
* varobj.h (varobj_gen_name): Return std::string.
* varobj.c (varobj_gen_name): Return std::string.
* mi/mi-cmd-var.c (mi_cmd_var_create): Use std::string.
diff --git a/gdb/mi/mi-cmd-disas.c b/gdb/mi/mi-cmd-disas.c
index d0f9b0b..b3d6245 100644
--- a/gdb/mi/mi-cmd-disas.c
+++ b/gdb/mi/mi-cmd-disas.c
@@ -74,7 +74,6 @@ mi_cmd_disassemble (const char *command, char **argv, int argc)
int how_many = -1;
CORE_ADDR low = 0;
CORE_ADDR high = 0;
- struct cleanup *cleanups = make_cleanup (null_cleanup, NULL);
/* Options processing stuff. */
int oind = 0;
@@ -104,9 +103,8 @@ mi_cmd_disassemble (const char *command, char **argv, int argc)
switch ((enum opt) opt)
{
case FILE_OPT:
- file_string = xstrdup (oarg);
+ file_string = oarg;
file_seen = 1;
- make_cleanup (xfree, file_string);
break;
case LINE_OPT:
line_num = atoi (oarg);
@@ -190,6 +188,4 @@ mi_cmd_disassemble (const char *command, char **argv, int argc)
gdb_disassembly (gdbarch, uiout,
disasm_flags,
how_many, low, high);
-
- do_cleanups (cleanups);
}
--
2.9.4
^ permalink raw reply [flat|nested] 35+ messages in thread
* [RFA 05/11] Remove unused declaration
2017-09-12 18:57 more cleanup removal, particularly in MI Tom Tromey
` (3 preceding siblings ...)
2017-09-12 18:57 ` [RFA 08/11] Use string and unique_xmalloc_ptr in mi-main.c Tom Tromey
@ 2017-09-12 18:57 ` Tom Tromey
2017-09-28 9:40 ` Pedro Alves
2017-09-12 18:57 ` [RFA 07/11] Use gdb::byte_vector in mi_cmd_data_write_memory_bytes Tom Tromey
` (6 subsequent siblings)
11 siblings, 1 reply; 35+ messages in thread
From: Tom Tromey @ 2017-09-12 18:57 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
There was a leftover cleanup declaration in mi_parse. Remove it.
ChangeLog
2017-09-12 Tom Tromey <tom@tromey.com>
* mi/mi-parse.c (mi_parse): Remove unused declaration.
---
gdb/ChangeLog | 4 ++++
gdb/mi/mi-parse.c | 1 -
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index c981c7e..e1c409d 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,9 @@
2017-09-12 Tom Tromey <tom@tromey.com>
+ * mi/mi-parse.c (mi_parse): Remove unused declaration.
+
+2017-09-12 Tom Tromey <tom@tromey.com>
+
* mi/mi-cmd-disas.c (mi_cmd_disassemble): Don't copy "oarg".
2017-09-12 Tom Tromey <tom@tromey.com>
diff --git a/gdb/mi/mi-parse.c b/gdb/mi/mi-parse.c
index cf05fa0..9c1b033 100644
--- a/gdb/mi/mi-parse.c
+++ b/gdb/mi/mi-parse.c
@@ -237,7 +237,6 @@ std::unique_ptr<struct mi_parse>
mi_parse (const char *cmd, char **token)
{
const char *chp;
- struct cleanup *cleanup;
std::unique_ptr<struct mi_parse> parse (new struct mi_parse);
--
2.9.4
^ permalink raw reply [flat|nested] 35+ messages in thread
* [RFA 07/11] Use gdb::byte_vector in mi_cmd_data_write_memory_bytes
2017-09-12 18:57 more cleanup removal, particularly in MI Tom Tromey
` (4 preceding siblings ...)
2017-09-12 18:57 ` [RFA 05/11] Remove unused declaration Tom Tromey
@ 2017-09-12 18:57 ` Tom Tromey
2017-09-28 9:46 ` Pedro Alves
2017-09-12 18:57 ` [RFA 10/11] Use a std::vector for ada_exceptions_list Tom Tromey
` (5 subsequent siblings)
11 siblings, 1 reply; 35+ messages in thread
From: Tom Tromey @ 2017-09-12 18:57 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This changes mi_cmd_data_write_memory_bytes to use gdb::byte_vector,
removing some cleanups.
ChangeLog
2017-09-12 Tom Tromey <tom@tromey.com>
* mi/mi-main.c (mi_cmd_data_write_memory_bytes): Use
gdb::byte_vector.
---
gdb/ChangeLog | 5 +++++
gdb/mi/mi-main.c | 20 +++++++-------------
2 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 47632fa..7158579 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
2017-09-12 Tom Tromey <tom@tromey.com>
+ * mi/mi-main.c (mi_cmd_data_write_memory_bytes): Use
+ gdb::byte_vector.
+
+2017-09-12 Tom Tromey <tom@tromey.com>
+
* thread.c (gdb_list_thread_ids, gdb_thread_select): Change
error_message to std::string.
* mi/mi-main.c (mi_cmd_thread_select): Use std::string.
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index d01f578..c8c4a97 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1710,11 +1710,8 @@ mi_cmd_data_write_memory_bytes (const char *command, char **argv, int argc)
{
CORE_ADDR addr;
char *cdata;
- gdb_byte *data;
- gdb_byte *databuf;
size_t len_hex, len_bytes, len_units, i, steps, remaining_units;
long int count_units;
- struct cleanup *back_to;
int unit_size;
if (argc != 2 && argc != 3)
@@ -1738,8 +1735,7 @@ mi_cmd_data_write_memory_bytes (const char *command, char **argv, int argc)
else
count_units = len_units;
- databuf = XNEWVEC (gdb_byte, len_bytes);
- back_to = make_cleanup (xfree, databuf);
+ gdb::byte_vector databuf (len_bytes);
for (i = 0; i < len_bytes; ++i)
{
@@ -1749,34 +1745,32 @@ mi_cmd_data_write_memory_bytes (const char *command, char **argv, int argc)
databuf[i] = (gdb_byte) x;
}
+ gdb::byte_vector data;
if (len_units < count_units)
{
/* Pattern is made of less units than count:
repeat pattern to fill memory. */
- data = (gdb_byte *) xmalloc (count_units * unit_size);
- make_cleanup (xfree, data);
+ data = gdb::byte_vector (count_units * unit_size);
/* Number of times the pattern is entirely repeated. */
steps = count_units / len_units;
/* Number of remaining addressable memory units. */
remaining_units = count_units % len_units;
for (i = 0; i < steps; i++)
- memcpy (data + i * len_bytes, databuf, len_bytes);
+ memcpy (&data[i * len_bytes], &databuf[0], len_bytes);
if (remaining_units > 0)
- memcpy (data + steps * len_bytes, databuf,
+ memcpy (&data[steps * len_bytes], &databuf[0],
remaining_units * unit_size);
}
else
{
/* Pattern is longer than or equal to count:
just copy count addressable memory units. */
- data = databuf;
+ data = std::move (databuf);
}
- write_memory_with_notification (addr, data, count_units);
-
- do_cleanups (back_to);
+ write_memory_with_notification (addr, data.data (), count_units);
}
void
--
2.9.4
^ permalink raw reply [flat|nested] 35+ messages in thread
* [RFA 11/11] Change captured_mi_execute_command to use scoped_restore
2017-09-12 18:57 more cleanup removal, particularly in MI Tom Tromey
2017-09-12 18:57 ` [RFA 06/11] Change some gdb_* functions to use a std::string out parameter Tom Tromey
2017-09-12 18:57 ` [RFA 09/11] Use std::set in mi-main.c Tom Tromey
@ 2017-09-12 18:57 ` Tom Tromey
2017-09-28 10:35 ` Pedro Alves
2017-09-12 18:57 ` [RFA 08/11] Use string and unique_xmalloc_ptr in mi-main.c Tom Tromey
` (8 subsequent siblings)
11 siblings, 1 reply; 35+ messages in thread
From: Tom Tromey @ 2017-09-12 18:57 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
Change captured_mi_execute_command to use a scoped_restore, removing a
cleanup. The old code copied the current token, but I don't believe
that is necessary.
ChangeLog
2017-09-12 Tom Tromey <tom@tromey.com>
* mi/mi-main.c (captured_mi_execute_command): Use scope_restore.
---
gdb/ChangeLog | 4 ++++
gdb/mi/mi-main.c | 9 +++------
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ee965c4..773f35d 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,9 @@
2017-09-12 Tom Tromey <tom@tromey.com>
+ * mi/mi-main.c (captured_mi_execute_command): Use scope_restore.
+
+2017-09-12 Tom Tromey <tom@tromey.com>
+
* mi/mi-cmd-info.c (mi_cmd_info_ada_exceptions): Update.
* ada-lang.h (struct ada_exc_info): Remove typedef. Declare
operator< and operator==.
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 3d73446..3e5eca2 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1870,20 +1870,19 @@ mi_cmd_remove_inferior (const char *command, char **argv, int argc)
Return <0 for error; >=0 for ok.
args->action will tell mi_execute_command what action
- to perfrom after the given command has executed (display/suppress
+ to perform after the given command has executed (display/suppress
prompt, display error). */
static void
captured_mi_execute_command (struct ui_out *uiout, struct mi_parse *context)
{
struct mi_interp *mi = (struct mi_interp *) command_interp ();
- struct cleanup *cleanup;
if (do_timings)
current_command_ts = context->cmd_start;
- current_token = xstrdup (context->token);
- cleanup = make_cleanup (free_current_contents, ¤t_token);
+ scoped_restore save_token = make_scoped_restore (¤t_token,
+ context->token);
running_result_record_printed = 0;
mi_proceeded = 0;
@@ -1959,8 +1958,6 @@ captured_mi_execute_command (struct ui_out *uiout, struct mi_parse *context)
break;
}
}
-
- do_cleanups (cleanup);
}
/* Print a gdb exception to the MI output stream. */
--
2.9.4
^ permalink raw reply [flat|nested] 35+ messages in thread
* [RFA 08/11] Use string and unique_xmalloc_ptr in mi-main.c
2017-09-12 18:57 more cleanup removal, particularly in MI Tom Tromey
` (2 preceding siblings ...)
2017-09-12 18:57 ` [RFA 11/11] Change captured_mi_execute_command to use scoped_restore Tom Tromey
@ 2017-09-12 18:57 ` Tom Tromey
2017-09-28 9:57 ` Pedro Alves
2017-09-12 18:57 ` [RFA 05/11] Remove unused declaration Tom Tromey
` (7 subsequent siblings)
11 siblings, 1 reply; 35+ messages in thread
From: Tom Tromey @ 2017-09-12 18:57 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
Change a couple of spots in mi-main.c to use std::string or
unique_xmalloc_ptr. unique_xmalloc_ptr is used here where the string
is writeable; I generally prefer to pretend that string is read-only,
perhaps not for a good resaon.
ChangeLog
2017-09-12 Tom Tromey <tom@tromey.com>
* mi/mi-main.c (mi_execute_cli_command): Use unique_xmalloc_ptr.
(mi_execute_async_cli_command): Likewise.
(mi_cmd_trace_frame_collected): Use std::string.
---
gdb/ChangeLog | 6 ++++++
gdb/mi/mi-main.c | 40 +++++++++++-----------------------------
2 files changed, 17 insertions(+), 29 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 7158579..6368f41 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
2017-09-12 Tom Tromey <tom@tromey.com>
+ * mi/mi-main.c (mi_execute_cli_command): Use unique_xmalloc_ptr.
+ (mi_execute_async_cli_command): Likewise.
+ (mi_cmd_trace_frame_collected): Use std::string.
+
+2017-09-12 Tom Tromey <tom@tromey.com>
+
* mi/mi-main.c (mi_cmd_data_write_memory_bytes): Use
gdb::byte_vector.
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index c8c4a97..0147fb9 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -2264,41 +2264,31 @@ mi_execute_cli_command (const char *cmd, int args_p, const char *args)
{
if (cmd != 0)
{
- struct cleanup *old_cleanups;
- char *run;
+ gdb::unique_xmalloc_ptr<char> run;
if (args_p)
- run = xstrprintf ("%s %s", cmd, args);
+ run.reset (xstrprintf ("%s %s", cmd, args));
else
- run = xstrdup (cmd);
+ run.reset (xstrdup (cmd));
if (mi_debug_p)
/* FIXME: gdb_???? */
fprintf_unfiltered (gdb_stdout, "cli=%s run=%s\n",
- cmd, run);
- old_cleanups = make_cleanup (xfree, run);
- execute_command (run, 0 /* from_tty */ );
- do_cleanups (old_cleanups);
- return;
+ cmd, run.get ());
+ execute_command (run.get (), 0 /* from_tty */ );
}
}
void
mi_execute_async_cli_command (const char *cli_command, char **argv, int argc)
{
- struct cleanup *old_cleanups;
- char *run;
+ gdb::unique_xmalloc_ptr<char> run;
if (mi_async_p ())
- run = xstrprintf ("%s %s&", cli_command, argc ? *argv : "");
+ run.reset (xstrprintf ("%s %s&", cli_command, argc ? *argv : ""));
else
- run = xstrprintf ("%s %s", cli_command, argc ? *argv : "");
- old_cleanups = make_cleanup (xfree, run);
-
- execute_command (run, 0 /* from_tty */ );
+ run.reset (xstrprintf ("%s %s", cli_command, argc ? *argv : ""));
- /* Do this before doing any printing. It would appear that some
- print code leaves garbage around in the buffer. */
- do_cleanups (old_cleanups);
+ execute_command (run.get (), 0 /* from_tty */ );
}
void
@@ -2804,14 +2794,10 @@ mi_cmd_trace_frame_collected (const char *command, char **argv, int argc)
{
struct cleanup *cleanups;
int tvar;
- char *tsvname;
int i;
ui_out_emit_list list_emitter (uiout, "tvars");
- tsvname = NULL;
- cleanups = make_cleanup (free_current_contents, &tsvname);
-
for (i = 0; VEC_iterate (int, tinfo->tvars, i, tvar); i++)
{
struct trace_state_variable *tsv;
@@ -2822,10 +2808,8 @@ mi_cmd_trace_frame_collected (const char *command, char **argv, int argc)
if (tsv != NULL)
{
- tsvname = (char *) xrealloc (tsvname, strlen (tsv->name) + 2);
- tsvname[0] = '$';
- strcpy (tsvname + 1, tsv->name);
- uiout->field_string ("name", tsvname);
+ std::string tsvname = std::string ("$") + tsv->name;
+ uiout->field_string ("name", tsvname.c_str ());
tsv->value_known = target_get_trace_state_variable_value (tsv->number,
&tsv->value);
@@ -2837,8 +2821,6 @@ mi_cmd_trace_frame_collected (const char *command, char **argv, int argc)
uiout->field_skip ("current");
}
}
-
- do_cleanups (cleanups);
}
/* Memory. */
--
2.9.4
^ permalink raw reply [flat|nested] 35+ messages in thread
* [RFA 02/11] Remove cleanups from mi_cmd_break_insert_1
2017-09-12 18:57 more cleanup removal, particularly in MI Tom Tromey
` (6 preceding siblings ...)
2017-09-12 18:57 ` [RFA 10/11] Use a std::vector for ada_exceptions_list Tom Tromey
@ 2017-09-12 18:57 ` Tom Tromey
2017-09-28 9:24 ` Pedro Alves
2017-09-12 18:57 ` [RFA 04/11] Don't copy a string in mi_cmd_disassemble Tom Tromey
` (3 subsequent siblings)
11 siblings, 1 reply; 35+ messages in thread
From: Tom Tromey @ 2017-09-12 18:57 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This changes mi_argv_to_format to return a string, allowing the
removal of some cleanups.
ChangeLog
2017-09-12 Tom Tromey <tom@tromey.com>
* mi/mi-cmd-break.c (mi_argv_to_format): Return std::string.
(mi_cmd_break_insert_1): Update.
---
gdb/ChangeLog | 5 +++++
gdb/mi/mi-cmd-break.c | 17 +++++------------
2 files changed, 10 insertions(+), 12 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index b71f23b..15a19bb8 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
2017-09-12 Tom Tromey <tom@tromey.com>
+ * mi/mi-cmd-break.c (mi_argv_to_format): Return std::string.
+ (mi_cmd_break_insert_1): Update.
+
+2017-09-12 Tom Tromey <tom@tromey.com>
+
* target.h (make_scoped_defer_target_commit_resume): Update.
* target.c (make_scoped_defer_target_commit_resume): Rename from
make_cleanup_defer_target_commit_resume. Return a
diff --git a/gdb/mi/mi-cmd-break.c b/gdb/mi/mi-cmd-break.c
index bae8711..4cd134c 100644
--- a/gdb/mi/mi-cmd-break.c
+++ b/gdb/mi/mi-cmd-break.c
@@ -86,12 +86,11 @@ setup_breakpoint_reporting (void)
/* Convert arguments in ARGV to the string in "format",argv,argv...
and return it. */
-static char *
+static std::string
mi_argv_to_format (char **argv, int argc)
{
int i;
- struct obstack obstack;
- char *ret;
+ auto_obstack obstack;
obstack_init (&obstack);
@@ -152,10 +151,7 @@ mi_argv_to_format (char **argv, int argc)
}
obstack_1grow (&obstack, '\0');
- ret = xstrdup ((const char *) obstack_finish (&obstack));
- obstack_free (&obstack, NULL);
-
- return ret;
+ return (const char *) obstack_finish (&obstack);
}
/* Insert breakpoint.
@@ -174,13 +170,12 @@ mi_cmd_break_insert_1 (int dprintf, const char *command, char **argv, int argc)
int pending = 0;
int enabled = 1;
int tracepoint = 0;
- struct cleanup *back_to = make_cleanup (null_cleanup, NULL);
enum bptype type_wanted;
event_location_up location;
struct breakpoint_ops *ops;
int is_explicit = 0;
struct explicit_location explicit_loc;
- char *extra_string = NULL;
+ std::string extra_string;
enum opt
{
@@ -278,7 +273,6 @@ mi_cmd_break_insert_1 (int dprintf, const char *command, char **argv, int argc)
error (_("-dprintf-insert: Missing <format>"));
extra_string = mi_argv_to_format (argv + format_num, argc - format_num);
- make_cleanup (xfree, extra_string);
address = argv[oind];
}
else
@@ -343,13 +337,12 @@ mi_cmd_break_insert_1 (int dprintf, const char *command, char **argv, int argc)
}
create_breakpoint (get_current_arch (), location.get (), condition, thread,
- extra_string,
+ extra_string.c_str (),
0 /* condition and thread are valid. */,
temp_p, type_wanted,
ignore_count,
pending ? AUTO_BOOLEAN_TRUE : AUTO_BOOLEAN_FALSE,
ops, 0, enabled, 0, 0);
- do_cleanups (back_to);
}
/* Implements the -break-insert command.
--
2.9.4
^ permalink raw reply [flat|nested] 35+ messages in thread
* more cleanup removal, particularly in MI
@ 2017-09-12 18:57 Tom Tromey
2017-09-12 18:57 ` [RFA 06/11] Change some gdb_* functions to use a std::string out parameter Tom Tromey
` (11 more replies)
0 siblings, 12 replies; 35+ messages in thread
From: Tom Tromey @ 2017-09-12 18:57 UTC (permalink / raw)
To: gdb-patches
This series removes more cleanups.
The first patch is generic, but the remaining patches focus on
removing cleanups from MI. It doesn't completely remove cleanups from
MI, but does make significant progress toward that goal.
Regression tested on the buildbot.
Tom
^ permalink raw reply [flat|nested] 35+ messages in thread
* [RFA 01/11] Remove make_cleanup_defer_target_commit_resume
2017-09-12 18:57 more cleanup removal, particularly in MI Tom Tromey
` (8 preceding siblings ...)
2017-09-12 18:57 ` [RFA 04/11] Don't copy a string in mi_cmd_disassemble Tom Tromey
@ 2017-09-12 18:59 ` Tom Tromey
2017-09-28 9:17 ` Pedro Alves
2017-09-12 19:03 ` [RFA 03/11] Remove cleanups from mi-cmd-var.c Tom Tromey
2017-09-23 16:15 ` more cleanup removal, particularly in MI Tom Tromey
11 siblings, 1 reply; 35+ messages in thread
From: Tom Tromey @ 2017-09-12 18:59 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This removes make_cleanup_defer_target_commit_resume in favor of using
scoped_restore.
ChangeLog
2017-09-12 Tom Tromey <tom@tromey.com>
* target.h (make_scoped_defer_target_commit_resume): Update.
* target.c (make_scoped_defer_target_commit_resume): Rename from
make_cleanup_defer_target_commit_resume. Return a
scoped_restore.
* infrun.c (proceed): Use make_scoped_defer_target_commit_resume.
---
gdb/ChangeLog | 8 ++++++++
gdb/infrun.c | 60 +++++++++++++++++++++++++++++------------------------------
gdb/target.c | 10 +++-------
gdb/target.h | 6 +++---
4 files changed, 44 insertions(+), 40 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 3905a51..b71f23b 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,11 @@
+2017-09-12 Tom Tromey <tom@tromey.com>
+
+ * target.h (make_scoped_defer_target_commit_resume): Update.
+ * target.c (make_scoped_defer_target_commit_resume): Rename from
+ make_cleanup_defer_target_commit_resume. Return a
+ scoped_restore.
+ * infrun.c (proceed): Use make_scoped_defer_target_commit_resume.
+
2017-09-12 Simon Marchi <simon.marchi@ericsson.com>
* probe.h (probe_ops_cp): Remove typedef.
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 3f2ac85..e46ceab 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2990,7 +2990,6 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
struct execution_control_state ecss;
struct execution_control_state *ecs = &ecss;
struct cleanup *old_chain;
- struct cleanup *defer_resume_cleanup;
int started;
/* If we're stopped at a fork/vfork, follow the branch set by the
@@ -3132,26 +3131,27 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
until the target stops again. */
tp->prev_pc = regcache_read_pc (regcache);
- defer_resume_cleanup = make_cleanup_defer_target_commit_resume ();
+ {
+ scoped_restore save_defer_tc = make_scoped_defer_target_commit_resume ();
- started = start_step_over ();
+ started = start_step_over ();
- if (step_over_info_valid_p ())
- {
- /* Either this thread started a new in-line step over, or some
- other thread was already doing one. In either case, don't
- resume anything else until the step-over is finished. */
- }
- else if (started && !target_is_non_stop_p ())
- {
- /* A new displaced stepping sequence was started. In all-stop,
- we can't talk to the target anymore until it next stops. */
- }
- else if (!non_stop && target_is_non_stop_p ())
- {
- /* In all-stop, but the target is always in non-stop mode.
- Start all other threads that are implicitly resumed too. */
- ALL_NON_EXITED_THREADS (tp)
+ if (step_over_info_valid_p ())
+ {
+ /* Either this thread started a new in-line step over, or some
+ other thread was already doing one. In either case, don't
+ resume anything else until the step-over is finished. */
+ }
+ else if (started && !target_is_non_stop_p ())
+ {
+ /* A new displaced stepping sequence was started. In all-stop,
+ we can't talk to the target anymore until it next stops. */
+ }
+ else if (!non_stop && target_is_non_stop_p ())
+ {
+ /* In all-stop, but the target is always in non-stop mode.
+ Start all other threads that are implicitly resumed too. */
+ ALL_NON_EXITED_THREADS (tp)
{
/* Ignore threads of processes we're not resuming. */
if (!ptid_match (tp->ptid, resume_ptid))
@@ -3187,18 +3187,18 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
if (!ecs->wait_some_more)
error (_("Command aborted."));
}
- }
- else if (!tp->resumed && !thread_is_in_step_over_chain (tp))
- {
- /* The thread wasn't started, and isn't queued, run it now. */
- reset_ecs (ecs, tp);
- switch_to_thread (tp->ptid);
- keep_going_pass_signal (ecs);
- if (!ecs->wait_some_more)
- error (_("Command aborted."));
- }
+ }
+ else if (!tp->resumed && !thread_is_in_step_over_chain (tp))
+ {
+ /* The thread wasn't started, and isn't queued, run it now. */
+ reset_ecs (ecs, tp);
+ switch_to_thread (tp->ptid);
+ keep_going_pass_signal (ecs);
+ if (!ecs->wait_some_more)
+ error (_("Command aborted."));
+ }
+ }
- do_cleanups (defer_resume_cleanup);
target_commit_resume ();
discard_cleanups (old_chain);
diff --git a/gdb/target.c b/gdb/target.c
index 3e2b4d0..bcd0008 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -2330,14 +2330,10 @@ target_commit_resume (void)
/* See target.h. */
-struct cleanup *
-make_cleanup_defer_target_commit_resume (void)
+scoped_restore_tmpl<int>
+make_scoped_defer_target_commit_resume ()
{
- struct cleanup *old_chain;
-
- old_chain = make_cleanup_restore_integer (&defer_target_commit_resume);
- defer_target_commit_resume = 1;
- return old_chain;
+ return make_scoped_restore (&defer_target_commit_resume, 1);
}
void
diff --git a/gdb/target.h b/gdb/target.h
index a3f00ab..5160924 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -1371,10 +1371,10 @@ extern void target_resume (ptid_t ptid, int step, enum gdb_signal signal);
coalesce multiple resumption requests in a single vCont packet. */
extern void target_commit_resume ();
-/* Setup to defer target_commit_resume calls, and return a cleanup
- that reactivates target_commit_resume, if it was previously
+/* Setup to defer target_commit_resume calls, and reactivate
+ target_commit_resume on destruction, if it was previously
active. */
-struct cleanup *make_cleanup_defer_target_commit_resume ();
+extern scoped_restore_tmpl<int> make_scoped_defer_target_commit_resume ();
/* For target_read_memory see target/target.h. */
--
2.9.4
^ permalink raw reply [flat|nested] 35+ messages in thread
* [RFA 03/11] Remove cleanups from mi-cmd-var.c
2017-09-12 18:57 more cleanup removal, particularly in MI Tom Tromey
` (9 preceding siblings ...)
2017-09-12 18:59 ` [RFA 01/11] Remove make_cleanup_defer_target_commit_resume Tom Tromey
@ 2017-09-12 19:03 ` Tom Tromey
2017-09-28 9:36 ` Pedro Alves
2017-09-23 16:15 ` more cleanup removal, particularly in MI Tom Tromey
11 siblings, 1 reply; 35+ messages in thread
From: Tom Tromey @ 2017-09-12 19:03 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This removes some cleanups from mi-cmd-var.c. varobj_gen_name now
returns a string, simplifying mi_cmd_var_create. In
mi_cmd_var_delete, a string copy is apparently unnecessary, so it's
simply removed.
ChangeLog
2017-09-12 Tom Tromey <tom@tromey.com>
* varobj.h (varobj_gen_name): Return std::string.
* varobj.c (varobj_gen_name): Return std::string.
* mi/mi-cmd-var.c (mi_cmd_var_create): Use std::string.
(mi_cmd_var_delete): Don't copy "name".
---
gdb/ChangeLog | 7 +++++++
gdb/mi/mi-cmd-var.c | 43 +++++++++++--------------------------------
gdb/varobj.c | 7 ++-----
gdb/varobj.h | 2 +-
4 files changed, 21 insertions(+), 38 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 15a19bb8..b1fdcda 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,12 @@
2017-09-12 Tom Tromey <tom@tromey.com>
+ * varobj.h (varobj_gen_name): Return std::string.
+ * varobj.c (varobj_gen_name): Return std::string.
+ * mi/mi-cmd-var.c (mi_cmd_var_create): Use std::string.
+ (mi_cmd_var_delete): Don't copy "name".
+
+2017-09-12 Tom Tromey <tom@tromey.com>
+
* mi/mi-cmd-break.c (mi_argv_to_format): Return std::string.
(mi_cmd_break_insert_1): Update.
diff --git a/gdb/mi/mi-cmd-var.c b/gdb/mi/mi-cmd-var.c
index 8b22b2f..b628814 100644
--- a/gdb/mi/mi-cmd-var.c
+++ b/gdb/mi/mi-cmd-var.c
@@ -95,32 +95,21 @@ mi_cmd_var_create (const char *command, char **argv, int argc)
struct ui_out *uiout = current_uiout;
CORE_ADDR frameaddr = 0;
struct varobj *var;
- char *name;
char *frame;
char *expr;
- struct cleanup *old_cleanups;
enum varobj_type var_type;
if (argc != 3)
error (_("-var-create: Usage: NAME FRAME EXPRESSION."));
- name = xstrdup (argv[0]);
- /* Add cleanup for name. Must be free_current_contents as name can
- be reallocated. */
- old_cleanups = make_cleanup (free_current_contents, &name);
-
- frame = xstrdup (argv[1]);
- make_cleanup (xfree, frame);
+ std::string name = argv[0];
- expr = xstrdup (argv[2]);
- make_cleanup (xfree, expr);
+ frame = argv[1];
+ expr = argv[2];
- if (strcmp (name, "-") == 0)
- {
- xfree (name);
- name = varobj_gen_name ();
- }
- else if (!isalpha (*name))
+ if (name == "-")
+ name = varobj_gen_name ();
+ else if (!isalpha (name[0]))
error (_("-var-create: name of object must begin with a letter"));
if (strcmp (frame, "*") == 0)
@@ -135,10 +124,10 @@ mi_cmd_var_create (const char *command, char **argv, int argc)
if (varobjdebug)
fprintf_unfiltered (gdb_stdlog,
- "Name=\"%s\", Frame=\"%s\" (%s), Expression=\"%s\"\n",
- name, frame, hex_string (frameaddr), expr);
+ "Name=\"%s\", Frame=\"%s\" (%s), Expression=\"%s\"\n",
+ name.c_str (), frame, hex_string (frameaddr), expr);
- var = varobj_create (name, expr, frameaddr, var_type);
+ var = varobj_create (name.c_str (), expr, frameaddr, var_type);
if (var == NULL)
error (_("-var-create: unable to create variable object"));
@@ -146,8 +135,6 @@ mi_cmd_var_create (const char *command, char **argv, int argc)
print_varobj (var, PRINT_ALL_VALUES, 0 /* don't print expression */);
uiout->field_int ("has_more", varobj_has_more (var, 0));
-
- do_cleanups (old_cleanups);
}
void
@@ -157,16 +144,12 @@ mi_cmd_var_delete (const char *command, char **argv, int argc)
struct varobj *var;
int numdel;
int children_only_p = 0;
- struct cleanup *old_cleanups;
struct ui_out *uiout = current_uiout;
if (argc < 1 || argc > 2)
error (_("-var-delete: Usage: [-c] EXPRESSION."));
- name = xstrdup (argv[0]);
- /* Add cleanup for name. Must be free_current_contents as name can
- be reallocated. */
- old_cleanups = make_cleanup (free_current_contents, &name);
+ name = argv[0];
/* If we have one single argument it cannot be '-c' or any string
starting with '-'. */
@@ -186,9 +169,7 @@ mi_cmd_var_delete (const char *command, char **argv, int argc)
if (strcmp (name, "-c") != 0)
error (_("-var-delete: Invalid option."));
children_only_p = 1;
- do_cleanups (old_cleanups);
- name = xstrdup (argv[1]);
- old_cleanups = make_cleanup (free_current_contents, &name);
+ name = argv[1];
}
/* If we didn't error out, now NAME contains the name of the
@@ -199,8 +180,6 @@ mi_cmd_var_delete (const char *command, char **argv, int argc)
numdel = varobj_delete (var, children_only_p);
uiout->field_int ("ndeleted", numdel);
-
- do_cleanups (old_cleanups);
}
/* Parse a string argument into a format value. */
diff --git a/gdb/varobj.c b/gdb/varobj.c
index f669180..2d850fb 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -435,17 +435,14 @@ varobj_create (const char *objname,
/* Generates an unique name that can be used for a varobj. */
-char *
+std::string
varobj_gen_name (void)
{
static int id = 0;
- char *obj_name;
/* Generate a name for this object. */
id++;
- obj_name = xstrprintf ("var%d", id);
-
- return obj_name;
+ return string_printf ("var%d", id);
}
/* Given an OBJNAME, returns the pointer to the corresponding varobj. Call
diff --git a/gdb/varobj.h b/gdb/varobj.h
index e35c1b8..0d4a537 100644
--- a/gdb/varobj.h
+++ b/gdb/varobj.h
@@ -233,7 +233,7 @@ extern struct varobj *varobj_create (const char *objname,
const char *expression, CORE_ADDR frame,
enum varobj_type type);
-extern char *varobj_gen_name (void);
+extern std::string varobj_gen_name (void);
extern struct varobj *varobj_get_handle (const char *name);
--
2.9.4
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: more cleanup removal, particularly in MI
2017-09-12 18:57 more cleanup removal, particularly in MI Tom Tromey
` (10 preceding siblings ...)
2017-09-12 19:03 ` [RFA 03/11] Remove cleanups from mi-cmd-var.c Tom Tromey
@ 2017-09-23 16:15 ` Tom Tromey
11 siblings, 0 replies; 35+ messages in thread
From: Tom Tromey @ 2017-09-23 16:15 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:
Tom> This series removes more cleanups.
Tom> The first patch is generic, but the remaining patches focus on
Tom> removing cleanups from MI. It doesn't completely remove cleanups from
Tom> MI, but does make significant progress toward that goal.
Tom> Regression tested on the buildbot.
Ping.
Tom
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [RFA 01/11] Remove make_cleanup_defer_target_commit_resume
2017-09-12 18:59 ` [RFA 01/11] Remove make_cleanup_defer_target_commit_resume Tom Tromey
@ 2017-09-28 9:17 ` Pedro Alves
0 siblings, 0 replies; 35+ messages in thread
From: Pedro Alves @ 2017-09-28 9:17 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 09/12/2017 07:57 PM, Tom Tromey wrote:
> This removes make_cleanup_defer_target_commit_resume in favor of using
> scoped_restore.
>
> ChangeLog
> 2017-09-12 Tom Tromey <tom@tromey.com>
>
> * target.h (make_scoped_defer_target_commit_resume): Update.
> * target.c (make_scoped_defer_target_commit_resume): Rename from
> make_cleanup_defer_target_commit_resume. Return a
> scoped_restore.
> * infrun.c (proceed): Use make_scoped_defer_target_commit_resume.
OK.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [RFA 02/11] Remove cleanups from mi_cmd_break_insert_1
2017-09-12 18:57 ` [RFA 02/11] Remove cleanups from mi_cmd_break_insert_1 Tom Tromey
@ 2017-09-28 9:24 ` Pedro Alves
2017-09-28 19:57 ` Tom Tromey
2017-09-29 1:40 ` Tom Tromey
0 siblings, 2 replies; 35+ messages in thread
From: Pedro Alves @ 2017-09-28 9:24 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 09/12/2017 07:57 PM, Tom Tromey wrote:
> * target.h (make_scoped_defer_target_commit_resume): Update.
> * target.c (make_scoped_defer_target_commit_resume): Rename from
> make_cleanup_defer_target_commit_resume. Return a
> diff --git a/gdb/mi/mi-cmd-break.c b/gdb/mi/mi-cmd-break.c
> index bae8711..4cd134c 100644
> --- a/gdb/mi/mi-cmd-break.c
> +++ b/gdb/mi/mi-cmd-break.c
> @@ -86,12 +86,11 @@ setup_breakpoint_reporting (void)
> /* Convert arguments in ARGV to the string in "format",argv,argv...
> and return it. */
>
> -static char *
> +static std::string
> mi_argv_to_format (char **argv, int argc)
> {
> int i;
> - struct obstack obstack;
> - char *ret;
> + auto_obstack obstack;
>
> obstack_init (&obstack);
Remove the obstack_init call too.
>
> @@ -152,10 +151,7 @@ mi_argv_to_format (char **argv, int argc)
> }
> obstack_1grow (&obstack, '\0');
>
> - ret = xstrdup ((const char *) obstack_finish (&obstack));
> - obstack_free (&obstack, NULL);
> -
> - return ret;
> + return (const char *) obstack_finish (&obstack);
> }
OK.
IMO, it'd be even better to build the std::string directly
instead of building an obstack and then dupping the string.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [RFA 03/11] Remove cleanups from mi-cmd-var.c
2017-09-12 19:03 ` [RFA 03/11] Remove cleanups from mi-cmd-var.c Tom Tromey
@ 2017-09-28 9:36 ` Pedro Alves
2017-09-29 1:40 ` Tom Tromey
0 siblings, 1 reply; 35+ messages in thread
From: Pedro Alves @ 2017-09-28 9:36 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 09/12/2017 07:57 PM, Tom Tromey wrote:
> diff --git a/gdb/mi/mi-cmd-var.c b/gdb/mi/mi-cmd-var.c
> index 8b22b2f..b628814 100644
> --- a/gdb/mi/mi-cmd-var.c
> +++ b/gdb/mi/mi-cmd-var.c
> @@ -95,32 +95,21 @@ mi_cmd_var_create (const char *command, char **argv, int argc)
> struct ui_out *uiout = current_uiout;
> CORE_ADDR frameaddr = 0;
> struct varobj *var;
> - char *name;
> char *frame;
> char *expr;
> - struct cleanup *old_cleanups;
> enum varobj_type var_type;
>
> if (argc != 3)
> error (_("-var-create: Usage: NAME FRAME EXPRESSION."));
>
> - name = xstrdup (argv[0]);
> - /* Add cleanup for name. Must be free_current_contents as name can
> - be reallocated. */
> - old_cleanups = make_cleanup (free_current_contents, &name);
> -
> - frame = xstrdup (argv[1]);
> - make_cleanup (xfree, frame);
> + std::string name = argv[0];
>
> - expr = xstrdup (argv[2]);
> - make_cleanup (xfree, expr);
> + frame = argv[1];
> + expr = argv[2];
>
> - if (strcmp (name, "-") == 0)
> - {
> - xfree (name);
> - name = varobj_gen_name ();
> - }
> - else if (!isalpha (*name))
> + if (name == "-")
> + name = varobj_gen_name ();
> + else if (!isalpha (name[0]))
> error (_("-var-create: name of object must begin with a letter"));
name was a deep copy of argv[0] up to here. That copy
appears unnecessary, if you write it like this:
std::string name;
if (strcmp (argv[0], "-") == 0)
name = varobj_gen_name ();
else if (!isalpha (*argv[0]))
error (_("-var-create: name of object must begin with a letter"));
else
name = argv[0];
There's still a deep copy in that last else above.
We should be able to get rid of that like this:
const char *name;
std::string gen_name;
if (strcmp (argv[0], "-") == 0)
{
gen_name = varobj_gen_name ();
name = gen_name.c_str ();
}
else if (!isalpha (*argv[0]))
error (_("-var-create: name of object must begin with a letter"));
else
name = argv[0];
Otherwise OK.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [RFA 04/11] Don't copy a string in mi_cmd_disassemble
2017-09-12 18:57 ` [RFA 04/11] Don't copy a string in mi_cmd_disassemble Tom Tromey
@ 2017-09-28 9:40 ` Pedro Alves
0 siblings, 0 replies; 35+ messages in thread
From: Pedro Alves @ 2017-09-28 9:40 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 09/12/2017 07:57 PM, Tom Tromey wrote:
> This string copy in mi_cmd_disassemble seems not to be needed, so
> don't do it.
I read mi-getopt.h|c and convinced myself that you are right.
OK.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [RFA 05/11] Remove unused declaration
2017-09-12 18:57 ` [RFA 05/11] Remove unused declaration Tom Tromey
@ 2017-09-28 9:40 ` Pedro Alves
0 siblings, 0 replies; 35+ messages in thread
From: Pedro Alves @ 2017-09-28 9:40 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 09/12/2017 07:57 PM, Tom Tromey wrote:
> There was a leftover cleanup declaration in mi_parse. Remove it.
>
This is obvious.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [RFA 06/11] Change some gdb_* functions to use a std::string out parameter
2017-09-12 18:57 ` [RFA 06/11] Change some gdb_* functions to use a std::string out parameter Tom Tromey
@ 2017-09-28 9:42 ` Pedro Alves
2017-09-28 19:58 ` Tom Tromey
0 siblings, 1 reply; 35+ messages in thread
From: Pedro Alves @ 2017-09-28 9:42 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 09/12/2017 07:57 PM, Tom Tromey wrote:
> This changes a few gdb_* functions to use a std::string out parameter.
> Perhaps these functions should just go away entirely; I think they're
> vesitiges of a now-defunct libgdb plan.
I think it's better to drop this one and go with the full removal
as done in the other thread.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [RFA 07/11] Use gdb::byte_vector in mi_cmd_data_write_memory_bytes
2017-09-12 18:57 ` [RFA 07/11] Use gdb::byte_vector in mi_cmd_data_write_memory_bytes Tom Tromey
@ 2017-09-28 9:46 ` Pedro Alves
0 siblings, 0 replies; 35+ messages in thread
From: Pedro Alves @ 2017-09-28 9:46 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 09/12/2017 07:57 PM, Tom Tromey wrote:
> This changes mi_cmd_data_write_memory_bytes to use gdb::byte_vector,
> removing some cleanups.
Thanks.
>
> + gdb::byte_vector data;
> if (len_units < count_units)
> {
> /* Pattern is made of less units than count:
> repeat pattern to fill memory. */
> - data = (gdb_byte *) xmalloc (count_units * unit_size);
> - make_cleanup (xfree, data);
> + data = gdb::byte_vector (count_units * unit_size);
I think I'd have written instead:
data.resize (count_units * unit_size)
But OK either way.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [RFA 08/11] Use string and unique_xmalloc_ptr in mi-main.c
2017-09-12 18:57 ` [RFA 08/11] Use string and unique_xmalloc_ptr in mi-main.c Tom Tromey
@ 2017-09-28 9:57 ` Pedro Alves
2017-09-29 1:42 ` Tom Tromey
0 siblings, 1 reply; 35+ messages in thread
From: Pedro Alves @ 2017-09-28 9:57 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 09/12/2017 07:57 PM, Tom Tromey wrote:
> Change a couple of spots in mi-main.c to use std::string or
> unique_xmalloc_ptr. unique_xmalloc_ptr is used here where the string
> is writeable; I generally prefer to pretend that string is read-only,
> perhaps not for a good resaon.
Yes, I think not for a good reason. :-) You can
pass &str[0] to execute_command get access to the underlying
modifiable raw string. See b064640146bb for example.
> diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
> index c8c4a97..0147fb9 100644
> --- a/gdb/mi/mi-main.c
> +++ b/gdb/mi/mi-main.c
> @@ -2264,41 +2264,31 @@ mi_execute_cli_command (const char *cmd, int args_p, const char *args)
> {
> if (cmd != 0)
> {
> - struct cleanup *old_cleanups;
> - char *run;
> + gdb::unique_xmalloc_ptr<char> run;
>
> if (args_p)
> - run = xstrprintf ("%s %s", cmd, args);
> + run.reset (xstrprintf ("%s %s", cmd, args));
> else
> - run = xstrdup (cmd);
> + run.reset (xstrdup (cmd));
> if (mi_debug_p)
> /* FIXME: gdb_???? */
> fprintf_unfiltered (gdb_stdout, "cli=%s run=%s\n",
> - cmd, run);
> - old_cleanups = make_cleanup (xfree, run);
> - execute_command (run, 0 /* from_tty */ );
> - do_cleanups (old_cleanups);
> - return;
> + cmd, run.get ());
> + execute_command (run.get (), 0 /* from_tty */ );
> }
> }
>
> void
> mi_execute_async_cli_command (const char *cli_command, char **argv, int argc)
> {
> - struct cleanup *old_cleanups;
> - char *run;
> + gdb::unique_xmalloc_ptr<char> run;
>
> if (mi_async_p ())
> - run = xstrprintf ("%s %s&", cli_command, argc ? *argv : "");
> + run.reset (xstrprintf ("%s %s&", cli_command, argc ? *argv : ""));
> else
> - run = xstrprintf ("%s %s", cli_command, argc ? *argv : "");
> - old_cleanups = make_cleanup (xfree, run);
> -
> - execute_command (run, 0 /* from_tty */ );
> + run.reset (xstrprintf ("%s %s", cli_command, argc ? *argv : ""));
>
> - /* Do this before doing any printing. It would appear that some
> - print code leaves garbage around in the buffer. */
> - do_cleanups (old_cleanups);
> + execute_command (run.get (), 0 /* from_tty */ );
> }
>
I think the above would look better with std::string + string_printf.
> struct trace_state_variable *tsv;
> @@ -2822,10 +2808,8 @@ mi_cmd_trace_frame_collected (const char *command, char **argv, int argc)
>
> if (tsv != NULL)
> {
> - tsvname = (char *) xrealloc (tsvname, strlen (tsv->name) + 2);
> - tsvname[0] = '$';
> - strcpy (tsvname + 1, tsv->name);
> - uiout->field_string ("name", tsvname);
> + std::string tsvname = std::string ("$") + tsv->name;
> + uiout->field_string ("name", tsvname.c_str ());
How about replacing this string building + field_string with a single
call to:
uiout->field_fmt ("name", "$%s", tsv->name);
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [RFA 09/11] Use std::set in mi-main.c
2017-09-12 18:57 ` [RFA 09/11] Use std::set in mi-main.c Tom Tromey
@ 2017-09-28 10:10 ` Pedro Alves
2017-10-02 13:05 ` Simon Marchi
2017-10-03 11:21 ` Simon Marchi
1 sibling, 1 reply; 35+ messages in thread
From: Pedro Alves @ 2017-09-28 10:10 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 09/12/2017 07:57 PM, Tom Tromey wrote:
> Change a couple of spots in mi-main.c to use std::set. This
> simplifies the code and removes some cleanups.
std::set always gives me pause. For small objects like int,
and when the use case is insertion phase + lookup phase + discard set,
unsorted inserting into a vector, sorting, and then binary searching
the vector for lookuups is very likely to have better performance, for
cache locality reasons, and also because fewer allocations (with
std::set being a node-based container...)
But it's likely that in this case it doesn't really matter, so let's
go with the simplicity argument.
(At some point I may propose some data structure on top of
std::vector for use cases like this.)
Patch is OK as is.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [RFA 10/11] Use a std::vector for ada_exceptions_list
2017-09-12 18:57 ` [RFA 10/11] Use a std::vector for ada_exceptions_list Tom Tromey
@ 2017-09-28 10:20 ` Pedro Alves
0 siblings, 0 replies; 35+ messages in thread
From: Pedro Alves @ 2017-09-28 10:20 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 09/12/2017 07:57 PM, Tom Tromey wrote:
> Change ada_exceptions_list to return a std::vector and fix up the
> users. This allows removing a cleanup in MI.
Looks good to me with the nits below addressed.
> diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
> index 64f1a33..cafba2d 100644
> --- a/gdb/ada-lang.c
> +++ b/gdb/ada-lang.c
> @@ -62,6 +62,7 @@
> #include "cli/cli-utils.h"
> #include "common/function-view.h"
> #include "common/byte-vector.h"
> +#include <algorithm>
>
> /* Define whether or not the C operator '/' truncates towards zero for
> differently signed operands (truncation direction is undefined in C).
> @@ -13121,23 +13122,23 @@ ada_is_non_standard_exception_sym (struct symbol *sym)
> The comparison is determined first by exception name, and then
> by exception address. */
This comment talks about qsort. It should be updated to mention
std::sort instead, since the logic is different.
>
> -static int
> -compare_ada_exception_info (const void *a, const void *b)
> +bool
> +ada_exc_info::operator< (const ada_exc_info &other)
> {
> - const struct ada_exc_info *exc_a = (struct ada_exc_info *) a;
> - const struct ada_exc_info *exc_b = (struct ada_exc_info *) b;
> int result;
>
> - result = strcmp (exc_a->name, exc_b->name);
> - if (result != 0)
> - return result;
> -
> - if (exc_a->addr < exc_b->addr)
> - return -1;
> - if (exc_a->addr > exc_b->addr)
> - return 1;
> + result = strcmp (name, other.name);
> + if (result < 0)
> + return true;
> + if (result == 0 && addr < other.addr)
> + return true;
> + return false;
> +}
>
> - return 0;
> +bool
> +ada_exc_info::operator== (const ada_exc_info &other)
> +{
> + return strcmp (name, other.name) == 0 && addr == other.addr;
I'd swap the comparisons to put the cheap addr comparison first.
> if (regexp != NULL)
> printf_filtered
> @@ -13404,10 +13386,8 @@ info_exceptions_command (char *regexp, int from_tty)
> else
> printf_filtered (_("All defined Ada exceptions:\n"));
>
> - for (ix = 0; VEC_iterate(ada_exc_info, exceptions, ix, info); ix++)
> - printf_filtered ("%s: %s\n", info->name, paddress (gdbarch, info->addr));
> -
> - do_cleanups (cleanup);
> + for (ada_exc_info &info : exceptions)
> + printf_filtered ("%s: %s\n", info.name, paddress (gdbarch, info.addr));
I'd write 'const ada_exc_info &', just for general use-const-if-possible
reasons.
> - for (ix = 0; VEC_iterate(ada_exc_info, exceptions, ix, info); ix++)
> + for (ada_exc_info &info : exceptions)
Ditto.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [RFA 11/11] Change captured_mi_execute_command to use scoped_restore
2017-09-12 18:57 ` [RFA 11/11] Change captured_mi_execute_command to use scoped_restore Tom Tromey
@ 2017-09-28 10:35 ` Pedro Alves
0 siblings, 0 replies; 35+ messages in thread
From: Pedro Alves @ 2017-09-28 10:35 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 09/12/2017 07:57 PM, Tom Tromey wrote:
> Change captured_mi_execute_command to use a scoped_restore, removing a
> cleanup. The old code copied the current token, but I don't believe
> that is necessary.
>
I tried to see if it was safe, and I think it is. I wondered whether
the command to be executed could itself run an MI command with a token,
and whether that'd clobber the original token. Like, e.g. (contrived):
321-interpreter-exec mi "123-thread-info"
we currently get:
123^done,threads=[]
321^done
(gdb)
Looks like that's OK because we create a separate parse object for each
of the commands.
> ChangeLog
> 2017-09-12 Tom Tromey <tom@tromey.com>
>
> * mi/mi-main.c (captured_mi_execute_command): Use scope_restore.
OK.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [RFA 02/11] Remove cleanups from mi_cmd_break_insert_1
2017-09-28 9:24 ` Pedro Alves
@ 2017-09-28 19:57 ` Tom Tromey
2017-09-29 1:40 ` Tom Tromey
1 sibling, 0 replies; 35+ messages in thread
From: Tom Tromey @ 2017-09-28 19:57 UTC (permalink / raw)
To: Pedro Alves; +Cc: Tom Tromey, gdb-patches
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
Pedro> IMO, it'd be even better to build the std::string directly
Pedro> instead of building an obstack and then dupping the string.
I made this change.
I'll send the new patch a bit later.
Tom
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [RFA 06/11] Change some gdb_* functions to use a std::string out parameter
2017-09-28 9:42 ` Pedro Alves
@ 2017-09-28 19:58 ` Tom Tromey
0 siblings, 0 replies; 35+ messages in thread
From: Tom Tromey @ 2017-09-28 19:58 UTC (permalink / raw)
To: Pedro Alves; +Cc: Tom Tromey, gdb-patches
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
Pedro> On 09/12/2017 07:57 PM, Tom Tromey wrote:
>> This changes a few gdb_* functions to use a std::string out parameter.
>> Perhaps these functions should just go away entirely; I think they're
>> vesitiges of a now-defunct libgdb plan.
Pedro> I think it's better to drop this one and go with the full removal
Pedro> as done in the other thread.
I agree, I'll drop it here.
Tom
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [RFA 02/11] Remove cleanups from mi_cmd_break_insert_1
2017-09-28 9:24 ` Pedro Alves
2017-09-28 19:57 ` Tom Tromey
@ 2017-09-29 1:40 ` Tom Tromey
2017-09-29 10:21 ` Pedro Alves
1 sibling, 1 reply; 35+ messages in thread
From: Tom Tromey @ 2017-09-29 1:40 UTC (permalink / raw)
To: Pedro Alves; +Cc: Tom Tromey, gdb-patches
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
Pedro> IMO, it'd be even better to build the std::string directly
Pedro> instead of building an obstack and then dupping the string.
Here's the new patch.
Tom
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index d06ed22..1a58d82 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2017-09-28 Tom Tromey <tom@tromey.com>
+
+ * mi/mi-cmd-break.c (mi_argv_to_format): Return std::string.
+ (mi_cmd_break_insert_1): Update.
+
2017-09-12 Tom Tromey <tom@tromey.com>
* target.h (make_scoped_defer_target_commit_resume): Update.
diff --git a/gdb/mi/mi-cmd-break.c b/gdb/mi/mi-cmd-break.c
index 188e4e2..6519e29 100644
--- a/gdb/mi/mi-cmd-break.c
+++ b/gdb/mi/mi-cmd-break.c
@@ -86,76 +86,69 @@ setup_breakpoint_reporting (void)
/* Convert arguments in ARGV to the string in "format",argv,argv...
and return it. */
-static char *
+static std::string
mi_argv_to_format (char **argv, int argc)
{
int i;
- struct obstack obstack;
- char *ret;
-
- obstack_init (&obstack);
+ std::string result;
/* Convert ARGV[OIND + 1] to format string and save to FORMAT. */
- obstack_1grow (&obstack, '\"');
+ result += '\"';
for (i = 0; i < strlen (argv[0]); i++)
{
switch (argv[0][i])
{
case '\\':
- obstack_grow (&obstack, "\\\\", 2);
+ result += "\\\\";
break;
case '\a':
- obstack_grow (&obstack, "\\a", 2);
+ result += "\\a";
break;
case '\b':
- obstack_grow (&obstack, "\\b", 2);
+ result += "\\b";
break;
case '\f':
- obstack_grow (&obstack, "\\f", 2);
+ result += "\\f";
break;
case '\n':
- obstack_grow (&obstack, "\\n", 2);
+ result += "\\n";
break;
case '\r':
- obstack_grow (&obstack, "\\r", 2);
+ result += "\\r";
break;
case '\t':
- obstack_grow (&obstack, "\\t", 2);
+ result += "\\t";
break;
case '\v':
- obstack_grow (&obstack, "\\v", 2);
+ result += "\\v";
break;
case '"':
- obstack_grow (&obstack, "\\\"", 2);
+ result += "\\\"";
break;
default:
if (isprint (argv[0][i]))
- obstack_grow (&obstack, argv[0] + i, 1);
+ result += argv[0][i];
else
{
char tmp[5];
xsnprintf (tmp, sizeof (tmp), "\\%o",
(unsigned char) argv[0][i]);
- obstack_grow_str (&obstack, tmp);
+ result += tmp;
}
break;
}
}
- obstack_1grow (&obstack, '\"');
+ result += '\"';
/* Apply other argv to FORMAT. */
for (i = 1; i < argc; i++)
{
- obstack_1grow (&obstack, ',');
- obstack_grow_str (&obstack, argv[i]);
+ result += ',';
+ result += argv[i];
}
- obstack_1grow (&obstack, '\0');
-
- ret = xstrdup ((const char *) obstack_finish (&obstack));
- obstack_free (&obstack, NULL);
- return ret;
+ return result;
}
/* Insert breakpoint.
@@ -174,13 +167,12 @@ mi_cmd_break_insert_1 (int dprintf, const char *command, char **argv, int argc)
int pending = 0;
int enabled = 1;
int tracepoint = 0;
- struct cleanup *back_to = make_cleanup (null_cleanup, NULL);
enum bptype type_wanted;
event_location_up location;
struct breakpoint_ops *ops;
int is_explicit = 0;
struct explicit_location explicit_loc;
- char *extra_string = NULL;
+ std::string extra_string;
enum opt
{
@@ -278,7 +270,6 @@ mi_cmd_break_insert_1 (int dprintf, const char *command, char **argv, int argc)
error (_("-dprintf-insert: Missing <format>"));
extra_string = mi_argv_to_format (argv + format_num, argc - format_num);
- make_cleanup (xfree, extra_string);
address = argv[oind];
}
else
@@ -343,13 +334,12 @@ mi_cmd_break_insert_1 (int dprintf, const char *command, char **argv, int argc)
}
create_breakpoint (get_current_arch (), location.get (), condition, thread,
- extra_string,
+ extra_string.c_str (),
0 /* condition and thread are valid. */,
temp_p, type_wanted,
ignore_count,
pending ? AUTO_BOOLEAN_TRUE : AUTO_BOOLEAN_FALSE,
ops, 0, enabled, 0, 0);
- do_cleanups (back_to);
}
/* Implements the -break-insert command.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [RFA 03/11] Remove cleanups from mi-cmd-var.c
2017-09-28 9:36 ` Pedro Alves
@ 2017-09-29 1:40 ` Tom Tromey
2017-09-29 10:22 ` Pedro Alves
0 siblings, 1 reply; 35+ messages in thread
From: Tom Tromey @ 2017-09-29 1:40 UTC (permalink / raw)
To: Pedro Alves; +Cc: Tom Tromey, gdb-patches
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
Pedro> There's still a deep copy in that last else above.
Pedro> We should be able to get rid of that like this:
[...]
Here's my latest.
Tom
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 1a58d82..5a6791e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2017-09-12 Tom Tromey <tom@tromey.com>
+
+ * varobj.h (varobj_gen_name): Return std::string.
+ * varobj.c (varobj_gen_name): Return std::string.
+ * mi/mi-cmd-var.c (mi_cmd_var_create): Use std::string.
+ (mi_cmd_var_delete): Don't copy "name".
+
2017-09-28 Tom Tromey <tom@tromey.com>
* mi/mi-cmd-break.c (mi_argv_to_format): Return std::string.
diff --git a/gdb/mi/mi-cmd-var.c b/gdb/mi/mi-cmd-var.c
index 8b22b2f..0215b1a 100644
--- a/gdb/mi/mi-cmd-var.c
+++ b/gdb/mi/mi-cmd-var.c
@@ -95,32 +95,24 @@ mi_cmd_var_create (const char *command, char **argv, int argc)
struct ui_out *uiout = current_uiout;
CORE_ADDR frameaddr = 0;
struct varobj *var;
- char *name;
char *frame;
char *expr;
- struct cleanup *old_cleanups;
enum varobj_type var_type;
if (argc != 3)
error (_("-var-create: Usage: NAME FRAME EXPRESSION."));
- name = xstrdup (argv[0]);
- /* Add cleanup for name. Must be free_current_contents as name can
- be reallocated. */
- old_cleanups = make_cleanup (free_current_contents, &name);
-
- frame = xstrdup (argv[1]);
- make_cleanup (xfree, frame);
-
- expr = xstrdup (argv[2]);
- make_cleanup (xfree, expr);
+ frame = argv[1];
+ expr = argv[2];
+ const char *name = argv[0];
+ std::string gen_name;
if (strcmp (name, "-") == 0)
{
- xfree (name);
- name = varobj_gen_name ();
+ gen_name = varobj_gen_name ();
+ name = gen_name.c_str ();
}
- else if (!isalpha (*name))
+ else if (!isalpha (name[0]))
error (_("-var-create: name of object must begin with a letter"));
if (strcmp (frame, "*") == 0)
@@ -135,7 +127,7 @@ mi_cmd_var_create (const char *command, char **argv, int argc)
if (varobjdebug)
fprintf_unfiltered (gdb_stdlog,
- "Name=\"%s\", Frame=\"%s\" (%s), Expression=\"%s\"\n",
+ "Name=\"%s\", Frame=\"%s\" (%s), Expression=\"%s\"\n",
name, frame, hex_string (frameaddr), expr);
var = varobj_create (name, expr, frameaddr, var_type);
@@ -146,8 +138,6 @@ mi_cmd_var_create (const char *command, char **argv, int argc)
print_varobj (var, PRINT_ALL_VALUES, 0 /* don't print expression */);
uiout->field_int ("has_more", varobj_has_more (var, 0));
-
- do_cleanups (old_cleanups);
}
void
@@ -157,16 +147,12 @@ mi_cmd_var_delete (const char *command, char **argv, int argc)
struct varobj *var;
int numdel;
int children_only_p = 0;
- struct cleanup *old_cleanups;
struct ui_out *uiout = current_uiout;
if (argc < 1 || argc > 2)
error (_("-var-delete: Usage: [-c] EXPRESSION."));
- name = xstrdup (argv[0]);
- /* Add cleanup for name. Must be free_current_contents as name can
- be reallocated. */
- old_cleanups = make_cleanup (free_current_contents, &name);
+ name = argv[0];
/* If we have one single argument it cannot be '-c' or any string
starting with '-'. */
@@ -186,9 +172,7 @@ mi_cmd_var_delete (const char *command, char **argv, int argc)
if (strcmp (name, "-c") != 0)
error (_("-var-delete: Invalid option."));
children_only_p = 1;
- do_cleanups (old_cleanups);
- name = xstrdup (argv[1]);
- old_cleanups = make_cleanup (free_current_contents, &name);
+ name = argv[1];
}
/* If we didn't error out, now NAME contains the name of the
@@ -199,8 +183,6 @@ mi_cmd_var_delete (const char *command, char **argv, int argc)
numdel = varobj_delete (var, children_only_p);
uiout->field_int ("ndeleted", numdel);
-
- do_cleanups (old_cleanups);
}
/* Parse a string argument into a format value. */
diff --git a/gdb/varobj.c b/gdb/varobj.c
index f669180..2d850fb 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -435,17 +435,14 @@ varobj_create (const char *objname,
/* Generates an unique name that can be used for a varobj. */
-char *
+std::string
varobj_gen_name (void)
{
static int id = 0;
- char *obj_name;
/* Generate a name for this object. */
id++;
- obj_name = xstrprintf ("var%d", id);
-
- return obj_name;
+ return string_printf ("var%d", id);
}
/* Given an OBJNAME, returns the pointer to the corresponding varobj. Call
diff --git a/gdb/varobj.h b/gdb/varobj.h
index e35c1b8..0d4a537 100644
--- a/gdb/varobj.h
+++ b/gdb/varobj.h
@@ -233,7 +233,7 @@ extern struct varobj *varobj_create (const char *objname,
const char *expression, CORE_ADDR frame,
enum varobj_type type);
-extern char *varobj_gen_name (void);
+extern std::string varobj_gen_name (void);
extern struct varobj *varobj_get_handle (const char *name);
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [RFA 08/11] Use string and unique_xmalloc_ptr in mi-main.c
2017-09-28 9:57 ` Pedro Alves
@ 2017-09-29 1:42 ` Tom Tromey
2017-09-29 10:23 ` Pedro Alves
0 siblings, 1 reply; 35+ messages in thread
From: Tom Tromey @ 2017-09-29 1:42 UTC (permalink / raw)
To: Pedro Alves; +Cc: Tom Tromey, gdb-patches
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
Pedro> How about replacing this string building + field_string with a single
Pedro> call to:
Pedro> uiout-> field_fmt ("name", "$%s", tsv->name);
Here's an update.
Tom
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 9cd584b..1edd8ee 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
2017-09-12 Tom Tromey <tom@tromey.com>
+ * mi/mi-main.c (mi_execute_cli_command): Use std::string.
+ (mi_execute_async_cli_command): Likewise.
+ (mi_cmd_trace_frame_collected): Use field_fmt.
+
+2017-09-12 Tom Tromey <tom@tromey.com>
+
* mi/mi-main.c (mi_cmd_data_write_memory_bytes): Use
gdb::byte_vector.
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index e1ba8e2..2d560a4 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -2269,41 +2269,29 @@ mi_execute_cli_command (const char *cmd, int args_p, const char *args)
{
if (cmd != 0)
{
- struct cleanup *old_cleanups;
- char *run;
+ std::string run = cmd;
if (args_p)
- run = xstrprintf ("%s %s", cmd, args);
- else
- run = xstrdup (cmd);
+ run = run + " " + args;
if (mi_debug_p)
/* FIXME: gdb_???? */
fprintf_unfiltered (gdb_stdout, "cli=%s run=%s\n",
- cmd, run);
- old_cleanups = make_cleanup (xfree, run);
- execute_command (run, 0 /* from_tty */ );
- do_cleanups (old_cleanups);
- return;
+ cmd, run.c_str ());
+ execute_command (&run[0], 0 /* from_tty */ );
}
}
void
mi_execute_async_cli_command (const char *cli_command, char **argv, int argc)
{
- struct cleanup *old_cleanups;
- char *run;
+ std::string run = cli_command;
+ if (argc)
+ run = run + " " + *argv;
if (mi_async_p ())
- run = xstrprintf ("%s %s&", cli_command, argc ? *argv : "");
- else
- run = xstrprintf ("%s %s", cli_command, argc ? *argv : "");
- old_cleanups = make_cleanup (xfree, run);
+ run += "&";
- execute_command (run, 0 /* from_tty */ );
-
- /* Do this before doing any printing. It would appear that some
- print code leaves garbage around in the buffer. */
- do_cleanups (old_cleanups);
+ execute_command (&run[0], 0 /* from_tty */ );
}
void
@@ -2806,14 +2794,10 @@ mi_cmd_trace_frame_collected (const char *command, char **argv, int argc)
{
struct cleanup *cleanups;
int tvar;
- char *tsvname;
int i;
ui_out_emit_list list_emitter (uiout, "tvars");
- tsvname = NULL;
- cleanups = make_cleanup (free_current_contents, &tsvname);
-
for (i = 0; VEC_iterate (int, tinfo->tvars, i, tvar); i++)
{
struct trace_state_variable *tsv;
@@ -2824,10 +2808,7 @@ mi_cmd_trace_frame_collected (const char *command, char **argv, int argc)
if (tsv != NULL)
{
- tsvname = (char *) xrealloc (tsvname, strlen (tsv->name) + 2);
- tsvname[0] = '$';
- strcpy (tsvname + 1, tsv->name);
- uiout->field_string ("name", tsvname);
+ uiout->field_fmt ("name", "$%s", tsv->name);
tsv->value_known = target_get_trace_state_variable_value (tsv->number,
&tsv->value);
@@ -2839,8 +2820,6 @@ mi_cmd_trace_frame_collected (const char *command, char **argv, int argc)
uiout->field_skip ("current");
}
}
-
- do_cleanups (cleanups);
}
/* Memory. */
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [RFA 02/11] Remove cleanups from mi_cmd_break_insert_1
2017-09-29 1:40 ` Tom Tromey
@ 2017-09-29 10:21 ` Pedro Alves
0 siblings, 0 replies; 35+ messages in thread
From: Pedro Alves @ 2017-09-29 10:21 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On 09/29/2017 02:39 AM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
>
> Pedro> IMO, it'd be even better to build the std::string directly
> Pedro> instead of building an obstack and then dupping the string.
>
> Here's the new patch.
OK, please push. Thanks for doing this.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [RFA 03/11] Remove cleanups from mi-cmd-var.c
2017-09-29 1:40 ` Tom Tromey
@ 2017-09-29 10:22 ` Pedro Alves
0 siblings, 0 replies; 35+ messages in thread
From: Pedro Alves @ 2017-09-29 10:22 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
>
> Pedro> There's still a deep copy in that last else above.
> Pedro> We should be able to get rid of that like this:
> [...]
>
> Here's my latest.
OK.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [RFA 08/11] Use string and unique_xmalloc_ptr in mi-main.c
2017-09-29 1:42 ` Tom Tromey
@ 2017-09-29 10:23 ` Pedro Alves
0 siblings, 0 replies; 35+ messages in thread
From: Pedro Alves @ 2017-09-29 10:23 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On 09/29/2017 02:41 AM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
>
> Pedro> How about replacing this string building + field_string with a single
> Pedro> call to:
> Pedro> uiout-> field_fmt ("name", "$%s", tsv->name);
>
> Here's an update.
OK.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [RFA 09/11] Use std::set in mi-main.c
2017-09-28 10:10 ` Pedro Alves
@ 2017-10-02 13:05 ` Simon Marchi
0 siblings, 0 replies; 35+ messages in thread
From: Simon Marchi @ 2017-10-02 13:05 UTC (permalink / raw)
To: Pedro Alves; +Cc: Tom Tromey, gdb-patches
On 2017-09-28 12:10, Pedro Alves wrote:
> On 09/12/2017 07:57 PM, Tom Tromey wrote:
>> Change a couple of spots in mi-main.c to use std::set. This
>> simplifies the code and removes some cleanups.
>
> std::set always gives me pause. For small objects like int,
> and when the use case is insertion phase + lookup phase + discard set,
> unsorted inserting into a vector, sorting, and then binary searching
> the vector for lookuups is very likely to have better performance, for
> cache locality reasons, and also because fewer allocations (with
> std::set being a node-based container...)
>
> But it's likely that in this case it doesn't really matter, so let's
> go with the simplicity argument.
>
> (At some point I may propose some data structure on top of
> std::vector for use cases like this.)
We have discussed something like this with Sergio in this thread:
https://sourceware.org/ml/gdb-patches/2017-07/msg00434.html
(you can ctrl-f "boilerplate")
We managed to sneak in an std::set while you were not looking/on
vacation :). The idea was that the std::set interface really helped to
keep the code clean and short, and that we could later implement
something with the same behavior and interface as std::set, but based on
a vector. It's a bit different than what you propose, because to be a
drop-in replacement for a set, we would always keep the vector sorted
(insert the new elements where they belong). IIUC, what you propose is
to insert everything and then sort it. I think the latter has a better
worst-case performance (sorting at the end is n*log(n)) compared to the
former (insertion in reverse order will become n^2). I don't think it
matters much though, since this data structure would be explicitly for
relatively small amount of items.
I am not sure if the reasoning is different for a set of string (as in
common/environ.{h,c}). I assume it would be similar, since the actual
string object is rather small, and when inserting a string in the
middle, the following strings will be moved one position and not copied
(I haven't verified though).
Simon
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [RFA 09/11] Use std::set in mi-main.c
2017-09-12 18:57 ` [RFA 09/11] Use std::set in mi-main.c Tom Tromey
2017-09-28 10:10 ` Pedro Alves
@ 2017-10-03 11:21 ` Simon Marchi
2017-10-03 11:39 ` Tom Tromey
1 sibling, 1 reply; 35+ messages in thread
From: Simon Marchi @ 2017-10-03 11:21 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On 2017-09-12 20:57, Tom Tromey wrote:
> static void
> -list_available_thread_groups (VEC (int) *ids, int recurse)
> +list_available_thread_groups (const std::set<int> &ids, int recurse)
> {
> struct osdata *data;
> struct osdata_item *item;
> @@ -824,12 +795,9 @@ list_available_thread_groups (VEC (int) *ids, int
> recurse)
> /* At present, the target will return all available processes
> and if information about specific ones was required, we filter
> undesired processes here. */
> - if (ids && bsearch (&pid_i, VEC_address (int, ids),
> - VEC_length (int, ids),
> - sizeof (int), compare_positive_ints) == NULL)
> + if (!ids.empty () && ids.find (pid_i) != ids.end ())
I think the condition is the wrong way, it should be == and not !=. It
probably means we don't have a test for this feature.
Simon
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [RFA 09/11] Use std::set in mi-main.c
2017-10-03 11:21 ` Simon Marchi
@ 2017-10-03 11:39 ` Tom Tromey
0 siblings, 0 replies; 35+ messages in thread
From: Tom Tromey @ 2017-10-03 11:39 UTC (permalink / raw)
To: Simon Marchi; +Cc: Tom Tromey, gdb-patches
>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
Simon> On 2017-09-12 20:57, Tom Tromey wrote:
>> static void
>> -list_available_thread_groups (VEC (int) *ids, int recurse)
>> +list_available_thread_groups (const std::set<int> &ids, int recurse)
>> {
>> struct osdata *data;
>> struct osdata_item *item;
>> @@ -824,12 +795,9 @@ list_available_thread_groups (VEC (int) *ids,
>> int recurse)
>> /* At present, the target will return all available processes
>> and if information about specific ones was required, we filter
>> undesired processes here. */
>> - if (ids && bsearch (&pid_i, VEC_address (int, ids),
>> - VEC_length (int, ids),
>> - sizeof (int), compare_positive_ints) == NULL)
>> + if (!ids.empty () && ids.find (pid_i) != ids.end ())
Simon> I think the condition is the wrong way, it should be == and not !=.
Simon> It probably means we don't have a test for this feature.
Yes, I think you're right. Changing it to == doesn't affect the gdb.mi
test results for me.
Tom
^ permalink raw reply [flat|nested] 35+ messages in thread
end of thread, other threads:[~2017-10-03 11:39 UTC | newest]
Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-12 18:57 more cleanup removal, particularly in MI Tom Tromey
2017-09-12 18:57 ` [RFA 06/11] Change some gdb_* functions to use a std::string out parameter Tom Tromey
2017-09-28 9:42 ` Pedro Alves
2017-09-28 19:58 ` Tom Tromey
2017-09-12 18:57 ` [RFA 09/11] Use std::set in mi-main.c Tom Tromey
2017-09-28 10:10 ` Pedro Alves
2017-10-02 13:05 ` Simon Marchi
2017-10-03 11:21 ` Simon Marchi
2017-10-03 11:39 ` Tom Tromey
2017-09-12 18:57 ` [RFA 11/11] Change captured_mi_execute_command to use scoped_restore Tom Tromey
2017-09-28 10:35 ` Pedro Alves
2017-09-12 18:57 ` [RFA 08/11] Use string and unique_xmalloc_ptr in mi-main.c Tom Tromey
2017-09-28 9:57 ` Pedro Alves
2017-09-29 1:42 ` Tom Tromey
2017-09-29 10:23 ` Pedro Alves
2017-09-12 18:57 ` [RFA 05/11] Remove unused declaration Tom Tromey
2017-09-28 9:40 ` Pedro Alves
2017-09-12 18:57 ` [RFA 07/11] Use gdb::byte_vector in mi_cmd_data_write_memory_bytes Tom Tromey
2017-09-28 9:46 ` Pedro Alves
2017-09-12 18:57 ` [RFA 10/11] Use a std::vector for ada_exceptions_list Tom Tromey
2017-09-28 10:20 ` Pedro Alves
2017-09-12 18:57 ` [RFA 02/11] Remove cleanups from mi_cmd_break_insert_1 Tom Tromey
2017-09-28 9:24 ` Pedro Alves
2017-09-28 19:57 ` Tom Tromey
2017-09-29 1:40 ` Tom Tromey
2017-09-29 10:21 ` Pedro Alves
2017-09-12 18:57 ` [RFA 04/11] Don't copy a string in mi_cmd_disassemble Tom Tromey
2017-09-28 9:40 ` Pedro Alves
2017-09-12 18:59 ` [RFA 01/11] Remove make_cleanup_defer_target_commit_resume Tom Tromey
2017-09-28 9:17 ` Pedro Alves
2017-09-12 19:03 ` [RFA 03/11] Remove cleanups from mi-cmd-var.c Tom Tromey
2017-09-28 9:36 ` Pedro Alves
2017-09-29 1:40 ` Tom Tromey
2017-09-29 10:22 ` Pedro Alves
2017-09-23 16:15 ` more cleanup removal, particularly in MI 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).