On 2017-06-05 00:53, Tom Tromey wrote:
> This changes signal_catchpoint to be more of a C++ class, using
> std::vector and updating the users.
>
> gdb/ChangeLog
> 2017-06-04 Tom Tromey <tom@tromey.com>
>
> * break-catch-sig.c (gdb_signal_type): Remove typedef.
> (struct signal_catchpoint) <signals_to_be_caught>: Now a
> std::vector.
> (~signal_catchpoint, signal_catchpoint_insert_location)
> (signal_catchpoint_remove_location)
> (signal_catchpoint_breakpoint_hit, signal_catchpoint_print_one)
> (signal_catchpoint_print_mention)
> (signal_catchpoint_print_recreate)
> (signal_catchpoint_explains_signal): Update.
> (create_signal_catchpoint): Change type of "filter".
> (catch_signal_split_args): Return a std::vector.
> (catch_signal_command): Update.
> ---
Hi Tom,
Thanks for doing this. The patch looks good in general. I'd suggest
doing these in the same patch:
- make the catch_all field/parameter/variable a bool,
- remove the destructor, since it's now empty.
> @@ -99,20 +94,17 @@ static int
> signal_catchpoint_insert_location (struct bp_location *bl)
> {
> struct signal_catchpoint *c = (struct signal_catchpoint *)
> bl->owner;
> - int i;
>
> - if (c->signals_to_be_caught != NULL)
> + if (!c->signals_to_be_caught.empty ())
> {
> - gdb_signal_type iter;
> + gdb_signal iter;
You don't need this declaration...
> - for (i = 0;
> - VEC_iterate (gdb_signal_type, c->signals_to_be_caught, i, iter);
> - i++)
> + for (auto iter : c->signals_to_be_caught)
...because it's overriden by this one. Although I'd prefer if you used
"gdb_signal" instead of "auto" here. It's not much longer, is more
expressive. There are a few instances of this throughout the patch.
> @@ -246,26 +231,25 @@ signal_catchpoint_print_one (struct breakpoint
> *b,
> uiout->field_skip ("addr");
> annotate_field (5);
>
> - if (c->signals_to_be_caught
> - && VEC_length (gdb_signal_type, c->signals_to_be_caught) > 1)
> + if (!c->signals_to_be_caught.empty ())
I think you changed the behavior of this condition. It should probably
be:
c->signals_to_be_caught.size () > 1
With those comments addressed, the patch is good to me.
Thanks!
Simon
@@ -1,3 +1,18 @@
+2017-06-04 Tom Tromey <tom@tromey.com>
+
+ * break-catch-sig.c (gdb_signal_type): Remove typedef.
+ (struct signal_catchpoint) <signals_to_be_caught>: Now a
+ std::vector.
+ (~signal_catchpoint, signal_catchpoint_insert_location)
+ (signal_catchpoint_remove_location)
+ (signal_catchpoint_breakpoint_hit, signal_catchpoint_print_one)
+ (signal_catchpoint_print_mention)
+ (signal_catchpoint_print_recreate)
+ (signal_catchpoint_explains_signal): Update.
+ (create_signal_catchpoint): Change type of "filter".
+ (catch_signal_split_args): Return a std::vector.
+ (catch_signal_command): Update.
+
2017-06-03 Simon Marchi <simon.marchi@ericsson.com>
* x86-linux-nat.c (struct arch_lwp_info): Remove.
@@ -33,10 +33,6 @@
#define INTERNAL_SIGNAL(x) ((x) == GDB_SIGNAL_TRAP || (x) == GDB_SIGNAL_INT)
-typedef enum gdb_signal gdb_signal_type;
-
-DEF_VEC_I (gdb_signal_type);
-
/* An instance of this type is used to represent a signal catchpoint.
A breakpoint is really of this type iff its ops pointer points to
SIGNAL_CATCHPOINT_OPS. */
@@ -46,14 +42,14 @@ struct signal_catchpoint : public breakpoint
~signal_catchpoint () override;
/* Signal numbers used for the 'catch signal' feature. If no signal
- has been specified for filtering, its value is NULL. Otherwise,
+ has been specified for filtering, it is empty. Otherwise,
it holds a list of all signals to be caught. */
- VEC (gdb_signal_type) *signals_to_be_caught;
+ std::vector<gdb_signal> signals_to_be_caught;
- /* If SIGNALS_TO_BE_CAUGHT is NULL, then all "ordinary" signals are
+ /* If SIGNALS_TO_BE_CAUGHT is empty, then all "ordinary" signals are
caught. If CATCH_ALL is non-zero, then internal signals are
- caught as well. If SIGNALS_TO_BE_CAUGHT is non-NULL, then this
+ caught as well. If SIGNALS_TO_BE_CAUGHT is not empty, then this
field is ignored. */
int catch_all;
@@ -89,7 +85,6 @@ signal_to_name_or_int (enum gdb_signal sig)
signal_catchpoint::~signal_catchpoint ()
{
- VEC_free (gdb_signal_type, this->signals_to_be_caught);
}
/* Implement the "insert_location" breakpoint_ops method for signal
@@ -99,20 +94,17 @@ static int
signal_catchpoint_insert_location (struct bp_location *bl)
{
struct signal_catchpoint *c = (struct signal_catchpoint *) bl->owner;
- int i;
- if (c->signals_to_be_caught != NULL)
+ if (!c->signals_to_be_caught.empty ())
{
- gdb_signal_type iter;
+ gdb_signal iter;
- for (i = 0;
- VEC_iterate (gdb_signal_type, c->signals_to_be_caught, i, iter);
- i++)
+ for (auto iter : c->signals_to_be_caught)
++signal_catch_counts[iter];
}
else
{
- for (i = 0; i < GDB_SIGNAL_LAST; ++i)
+ for (int i = 0; i < GDB_SIGNAL_LAST; ++i)
{
if (c->catch_all || !INTERNAL_SIGNAL (i))
++signal_catch_counts[i];
@@ -132,15 +124,12 @@ signal_catchpoint_remove_location (struct bp_location *bl,
enum remove_bp_reason reason)
{
struct signal_catchpoint *c = (struct signal_catchpoint *) bl->owner;
- int i;
- if (c->signals_to_be_caught != NULL)
+ if (!c->signals_to_be_caught.empty ())
{
- gdb_signal_type iter;
+ gdb_signal iter;
- for (i = 0;
- VEC_iterate (gdb_signal_type, c->signals_to_be_caught, i, iter);
- i++)
+ for (auto iter : c->signals_to_be_caught)
{
gdb_assert (signal_catch_counts[iter] > 0);
--signal_catch_counts[iter];
@@ -148,7 +137,7 @@ signal_catchpoint_remove_location (struct bp_location *bl,
}
else
{
- for (i = 0; i < GDB_SIGNAL_LAST; ++i)
+ for (int i = 0; i < GDB_SIGNAL_LAST; ++i)
{
if (c->catch_all || !INTERNAL_SIGNAL (i))
{
@@ -174,7 +163,7 @@ signal_catchpoint_breakpoint_hit (const struct bp_location *bl,
{
const struct signal_catchpoint *c
= (const struct signal_catchpoint *) bl->owner;
- gdb_signal_type signal_number;
+ gdb_signal signal_number;
if (ws->kind != TARGET_WAITKIND_STOPPED)
return 0;
@@ -184,18 +173,14 @@ signal_catchpoint_breakpoint_hit (const struct bp_location *bl,
/* If we are catching specific signals in this breakpoint, then we
must guarantee that the called signal is the same signal we are
catching. */
- if (c->signals_to_be_caught)
+ if (!c->signals_to_be_caught.empty ())
{
- int i;
- gdb_signal_type iter;
+ gdb_signal iter;
- for (i = 0;
- VEC_iterate (gdb_signal_type, c->signals_to_be_caught, i, iter);
- i++)
+ for (auto iter : c->signals_to_be_caught)
if (signal_number == iter)
return 1;
/* Not the same. */
- gdb_assert (!iter);
return 0;
}
else
@@ -246,26 +231,25 @@ signal_catchpoint_print_one (struct breakpoint *b,
uiout->field_skip ("addr");
annotate_field (5);
- if (c->signals_to_be_caught
- && VEC_length (gdb_signal_type, c->signals_to_be_caught) > 1)
+ if (!c->signals_to_be_caught.empty ())
uiout->text ("signals \"");
else
uiout->text ("signal \"");
- if (c->signals_to_be_caught)
+ if (!c->signals_to_be_caught.empty ())
{
- int i;
- gdb_signal_type iter;
+ gdb_signal iter;
std::string text;
- for (i = 0;
- VEC_iterate (gdb_signal_type, c->signals_to_be_caught, i, iter);
- i++)
+ bool first = true;
+ for (auto iter : c->signals_to_be_caught)
{
const char *name = signal_to_name_or_int (iter);
- if (i > 0)
+ if (!first)
text += " ";
+ first = false;
+
text += name;
}
uiout->field_string ("what", text.c_str ());
@@ -287,19 +271,16 @@ signal_catchpoint_print_mention (struct breakpoint *b)
{
struct signal_catchpoint *c = (struct signal_catchpoint *) b;
- if (c->signals_to_be_caught)
+ if (!c->signals_to_be_caught.empty ())
{
- int i;
- gdb_signal_type iter;
+ gdb_signal iter;
- if (VEC_length (gdb_signal_type, c->signals_to_be_caught) > 1)
+ if (c->signals_to_be_caught.size () > 1)
printf_filtered (_("Catchpoint %d (signals"), b->number);
else
printf_filtered (_("Catchpoint %d (signal"), b->number);
- for (i = 0;
- VEC_iterate (gdb_signal_type, c->signals_to_be_caught, i, iter);
- i++)
+ for (auto iter : c->signals_to_be_caught)
{
const char *name = signal_to_name_or_int (iter);
@@ -323,14 +304,11 @@ signal_catchpoint_print_recreate (struct breakpoint *b, struct ui_file *fp)
fprintf_unfiltered (fp, "catch signal");
- if (c->signals_to_be_caught)
+ if (!c->signals_to_be_caught.empty ())
{
- int i;
- gdb_signal_type iter;
+ gdb_signal iter;
- for (i = 0;
- VEC_iterate (gdb_signal_type, c->signals_to_be_caught, i, iter);
- i++)
+ for (auto iter : c->signals_to_be_caught)
fprintf_unfiltered (fp, " %s", signal_to_name_or_int (iter));
}
else if (c->catch_all)
@@ -355,7 +333,7 @@ signal_catchpoint_explains_signal (struct breakpoint *b, enum gdb_signal sig)
then internal signals like SIGTRAP are not caught. */
static void
-create_signal_catchpoint (int tempflag, VEC (gdb_signal_type) *filter,
+create_signal_catchpoint (int tempflag, std::vector<gdb_signal> &&filter,
int catch_all)
{
struct signal_catchpoint *c;
@@ -363,7 +341,7 @@ create_signal_catchpoint (int tempflag, VEC (gdb_signal_type) *filter,
c = new signal_catchpoint ();
init_catchpoint (c, gdbarch, tempflag, NULL, &signal_catchpoint_ops);
- c->signals_to_be_caught = filter;
+ c->signals_to_be_caught = std::move (filter);
c->catch_all = catch_all;
install_breakpoint (0, c, 1);
@@ -373,57 +351,50 @@ create_signal_catchpoint (int tempflag, VEC (gdb_signal_type) *filter,
/* Splits the argument using space as delimiter. Returns an xmalloc'd
filter list, or NULL if no filtering is required. */
-static VEC (gdb_signal_type) *
+static std::vector<gdb_signal>
catch_signal_split_args (char *arg, int *catch_all)
{
- VEC (gdb_signal_type) *result = NULL;
- struct cleanup *cleanup = make_cleanup (VEC_cleanup (gdb_signal_type),
- &result);
+ std::vector<gdb_signal> result;
int first = 1;
while (*arg != '\0')
{
int num;
- gdb_signal_type signal_number;
- char *one_arg, *endptr;
- struct cleanup *inner_cleanup;
+ gdb_signal signal_number;
+ char *endptr;
- one_arg = extract_arg (&arg);
+ gdb::unique_xmalloc_ptr<char> one_arg (extract_arg (&arg));
if (one_arg == NULL)
break;
- inner_cleanup = make_cleanup (xfree, one_arg);
/* Check for the special flag "all". */
- if (strcmp (one_arg, "all") == 0)
+ if (strcmp (one_arg.get (), "all") == 0)
{
arg = skip_spaces (arg);
if (*arg != '\0' || !first)
error (_("'all' cannot be caught with other signals"));
*catch_all = 1;
- gdb_assert (result == NULL);
- do_cleanups (inner_cleanup);
- discard_cleanups (cleanup);
- return NULL;
+ gdb_assert (result.empty ());
+ return result;
}
first = 0;
/* Check if the user provided a signal name or a number. */
- num = (int) strtol (one_arg, &endptr, 0);
+ num = (int) strtol (one_arg.get (), &endptr, 0);
if (*endptr == '\0')
signal_number = gdb_signal_from_command (num);
else
{
- signal_number = gdb_signal_from_name (one_arg);
+ signal_number = gdb_signal_from_name (one_arg.get ());
if (signal_number == GDB_SIGNAL_UNKNOWN)
- error (_("Unknown signal name '%s'."), one_arg);
+ error (_("Unknown signal name '%s'."), one_arg.get ());
}
- VEC_safe_push (gdb_signal_type, result, signal_number);
- do_cleanups (inner_cleanup);
+ result.push_back (signal_number);
}
- discard_cleanups (cleanup);
+ result.shrink_to_fit ();
return result;
}
@@ -434,7 +405,7 @@ catch_signal_command (char *arg, int from_tty,
struct cmd_list_element *command)
{
int tempflag, catch_all = 0;
- VEC (gdb_signal_type) *filter;
+ std::vector<gdb_signal> filter;
tempflag = get_cmd_context (command) == CATCH_TEMPORARY;
@@ -448,10 +419,8 @@ catch_signal_command (char *arg, int from_tty,
if (arg != NULL)
filter = catch_signal_split_args (arg, &catch_all);
- else
- filter = NULL;
- create_signal_catchpoint (tempflag, filter, catch_all);
+ create_signal_catchpoint (tempflag, std::move (filter), catch_all);
}
static void