This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA 1/2] C++-ify break-catch-sig


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]