Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* PR8554: New command to save breakpoints to a file
@ 2010-04-09  2:41 Pedro Alves
  2010-04-09 16:17 ` Tom Tromey
  0 siblings, 1 reply; 23+ messages in thread
From: Pedro Alves @ 2010-04-09  2:41 UTC (permalink / raw)
  To: gdb-patches

(I finished this instead of dumping it.)

Add a new save-breakpoints command to save breakpoint definitions
to a file.  Use the "source" command to bring the breakpoints back in.

<http://sourceware.org/bugzilla/show_bug.cgi?id=8554>

This handles all types of breakpoints, tracepoints, catchpoints,
watchpoints, whatnot.  The new breakpoint_ops->print_recreate
method implementation for all catchpoints is always mostly
a simplified version of breakpoint_ops->print_mention method.

Docs/NEWS bits included below.

-- 
Pedro Alves

2010-04-09  Pedro Alves  <pedro@codesourcery.com>

	PR breakpoints/8554.
	
	Implement `save-breakpoints'.

	gdb/
	* breakpoint.c (print_recreate_catch_fork): New.
	(catch_fork_breakpoint_ops): Install it.
	(print_recreate_catch_vfork): New.
	(catch_vfork_breakpoint_ops): Install it.
	(print_recreate_catch_syscall): New.
	(catch_syscall_breakpoint_ops): Install it.
	(print_recreate_catch_exec): New.
	(catch_exec_breakpoint_ops): Install it.
	(print_recreate_exception_catchpoint): New.
	(gnu_v3_exception_catchpoint_ops): Install it.
	(breakpoint_save): New, based on tracepoint_save_command.
	(save_breakpoints_command): New.
	(tracepoint_save_command): Reimplement using breakpoint_save.
	(_initialize_breakpoints): Install the "save-breakpoints" command.
	* breakpoint.h (struct breakpoint_ops): New field `print_recreate'.
	* ada-lang.c (print_recreate_exception): New.
	(print_recreate_catch_exception): New.
	(catch_exception_breakpoint_ops): Install it.
	(print_recreate_catch_exception_unhandled): New.
	(catch_exception_unhandled_breakpoint_ops): Install it.
	(print_recreate_catch_assert): New.
	(catch_assert_breakpoint_ops): Install it.

	* NEWS: Mention the new save-breakpoints command.

	doc/
	* gdb.texinfo (Save Breakpoints): New node.

	testsuite/
	* gdb.trace/save-trace.exp: Adjust.

---
 gdb/NEWS                               |    7 +
 gdb/ada-lang.c                         |   57 +++++++-
 gdb/breakpoint.c                       |  231 ++++++++++++++++++++++++++++-----
 gdb/breakpoint.h                       |    3 
 gdb/doc/gdb.texinfo                    |   18 ++
 gdb/testsuite/gdb.trace/save-trace.exp |    4 
 6 files changed, 286 insertions(+), 34 deletions(-)

Index: src/gdb/breakpoint.c
===================================================================
--- src.orig/gdb/breakpoint.c	2010-04-08 23:27:37.000000000 +0100
+++ src/gdb/breakpoint.c	2010-04-09 03:29:07.000000000 +0100
@@ -5838,6 +5838,15 @@ print_mention_catch_fork (struct breakpo
   printf_filtered (_("Catchpoint %d (fork)"), b->number);
 }
 
+/* Implement the "print_recreate" breakpoint_ops method for fork
+   catchpoints.  */
+
+static void
+print_recreate_catch_fork (struct breakpoint *b, struct ui_file *fp)
+{
+  fprintf_unfiltered (fp, "catch fork");
+}
+
 /* The breakpoint_ops structure to be used in fork catchpoints.  */
 
 static struct breakpoint_ops catch_fork_breakpoint_ops =
@@ -5847,7 +5856,8 @@ static struct breakpoint_ops catch_fork_
   breakpoint_hit_catch_fork,
   print_it_catch_fork,
   print_one_catch_fork,
-  print_mention_catch_fork
+  print_mention_catch_fork,
+  print_recreate_catch_fork
 };
 
 /* Implement the "insert" breakpoint_ops method for vfork catchpoints.  */
@@ -5919,6 +5929,15 @@ print_mention_catch_vfork (struct breakp
   printf_filtered (_("Catchpoint %d (vfork)"), b->number);
 }
 
+/* Implement the "print_recreate" breakpoint_ops method for vfork
+   catchpoints.  */
+
+static void
+print_recreate_catch_vfork (struct breakpoint *b, struct ui_file *fp)
+{
+  fprintf_unfiltered (fp, "catch vfork");
+}
+
 /* The breakpoint_ops structure to be used in vfork catchpoints.  */
 
 static struct breakpoint_ops catch_vfork_breakpoint_ops =
@@ -5928,7 +5947,8 @@ static struct breakpoint_ops catch_vfork
   breakpoint_hit_catch_vfork,
   print_it_catch_vfork,
   print_one_catch_vfork,
-  print_mention_catch_vfork
+  print_mention_catch_vfork,
+  print_recreate_catch_vfork
 };
 
 /* Implement the "insert" breakpoint_ops method for syscall
@@ -6167,6 +6187,33 @@ print_mention_catch_syscall (struct brea
                      b->number);
 }
 
+/* Implement the "print_recreate" breakpoint_ops method for syscall
+   catchpoints.  */
+
+static void
+print_recreate_catch_syscall (struct breakpoint *b, struct ui_file *fp)
+{
+  fprintf_unfiltered (fp, "catch syscall");
+
+  if (b->syscalls_to_be_caught)
+    {
+      int i, iter;
+
+      for (i = 0;
+           VEC_iterate (int, b->syscalls_to_be_caught, i, iter);
+           i++)
+        {
+          struct syscall s;
+
+          get_syscall_by_number (iter, &s);
+          if (s.name)
+            fprintf_unfiltered (fp, " %s", s.name);
+          else
+            fprintf_unfiltered (fp, " %d", s.number);
+        }
+    }
+}
+
 /* The breakpoint_ops structure to be used in syscall catchpoints.  */
 
 static struct breakpoint_ops catch_syscall_breakpoint_ops =
@@ -6176,7 +6223,8 @@ static struct breakpoint_ops catch_sysca
   breakpoint_hit_catch_syscall,
   print_it_catch_syscall,
   print_one_catch_syscall,
-  print_mention_catch_syscall
+  print_mention_catch_syscall,
+  print_recreate_catch_syscall
 };
 
 /* Returns non-zero if 'b' is a syscall catchpoint.  */
@@ -6312,6 +6360,15 @@ print_mention_catch_exec (struct breakpo
   printf_filtered (_("Catchpoint %d (exec)"), b->number);
 }
 
+/* Implement the "print_recreate" breakpoint_ops method for exec
+   catchpoints.  */
+
+static void
+print_recreate_catch_exec (struct breakpoint *b, struct ui_file *fp)
+{
+  fprintf_unfiltered (fp, "catch exec");
+}
+
 static struct breakpoint_ops catch_exec_breakpoint_ops =
 {
   insert_catch_exec,
@@ -6319,7 +6376,8 @@ static struct breakpoint_ops catch_exec_
   breakpoint_hit_catch_exec,
   print_it_catch_exec,
   print_one_catch_exec,
-  print_mention_catch_exec
+  print_mention_catch_exec,
+  print_recreate_catch_exec
 };
 
 static void
@@ -8279,13 +8337,29 @@ print_mention_exception_catchpoint (stru
 			       : _(" (catch)"));
 }
 
+/* Implement the "print_recreate" breakpoint_ops method for throw and
+   catch catchpoints.  */
+
+static void
+print_recreate_exception_catchpoint (struct breakpoint *b, struct ui_file *fp)
+{
+  int bp_temp;
+  int bp_throw;
+
+  bp_temp = b->disposition == disp_del;
+  bp_throw = strstr (b->addr_string, "throw") != NULL;
+  fprintf_unfiltered (fp, bp_temp ? "tcatch " : "catch ");
+  fprintf_unfiltered (fp, bp_throw ? "throw" : "catch");
+}
+
 static struct breakpoint_ops gnu_v3_exception_catchpoint_ops = {
   NULL, /* insert */
   NULL, /* remove */
   NULL, /* breakpoint_hit */
   print_exception_catchpoint,
   print_one_exception_catchpoint,
-  print_mention_exception_catchpoint
+  print_mention_exception_catchpoint,
+  print_recreate_exception_catchpoint
 };
 
 static int
@@ -10644,30 +10718,45 @@ get_tracepoint_by_number (char **arg, in
   return NULL;
 }
 
-/* save-tracepoints command */
+/* The save-breakpoints command.  */
+
 static void
-tracepoint_save_command (char *args, int from_tty)
+breakpoint_save (char *args, int from_tty,
+		 int (*filter) (const struct breakpoint *))
 {
   struct breakpoint *tp;
-  int any_tp = 0;
-  struct command_line *line;
+  int any = 0;
   char *pathname;
   char tmp[40];
   struct cleanup *cleanup;
   struct ui_file *fp;
+  int extra_trace_bits = 0;
+  int created_count = 0;
 
   if (args == 0 || *args == 0)
-    error (_("Argument required (file name in which to save tracepoints)"));
+    error (_("Argument required (file name in which to save)"));
 
   /* See if we have anything to save.  */
-  ALL_TRACEPOINTS (tp)
+  ALL_BREAKPOINTS (tp)
   {
-    any_tp = 1;
+    /* Skip internal and momentary breakpoints.  */
+    if (tp->number <= 0)
+      continue;
+
+    /* If we have a filter, only save the breakpoints it accepts.  */
+    if (filter && !filter (tp))
+      continue;
+
+    if (is_tracepoint (tp))
+      extra_trace_bits = 1;
+
+    any = 1;
     break;
   }
-  if (!any_tp)
+
+  if (!any)
     {
-      warning (_("save-tracepoints: no tracepoints to save."));
+      warning (_("Nothing to save."));
       return;
     }
 
@@ -10675,32 +10764,82 @@ tracepoint_save_command (char *args, int
   cleanup = make_cleanup (xfree, pathname);
   fp = gdb_fopen (pathname, "w");
   if (!fp)
-    error (_("Unable to open file '%s' for saving tracepoints (%s)"),
+    error (_("Unable to open file '%s' for saving (%s)"),
 	   args, safe_strerror (errno));
   make_cleanup_ui_file_delete (fp);
 
-  save_trace_state_variables (fp);
+  if (extra_trace_bits)
+    save_trace_state_variables (fp);
 
-  ALL_TRACEPOINTS (tp)
+  ALL_BREAKPOINTS (tp)
   {
-    if (tp->type == bp_fast_tracepoint)
-      fprintf_unfiltered (fp, "ftrace");
-    else
-      fprintf_unfiltered (fp, "trace");
+    /* Skip internal and momentary breakpoints.  */
+    if (tp->number <= 0)
+      continue;
+
+    /* If we have a filter, only save the breakpoints it accepts.  */
+    if (filter && !filter (tp))
+      continue;
 
-    if (tp->addr_string)
-      fprintf_unfiltered (fp, " %s", tp->addr_string);
+    if (tp->ops != NULL)
+      (tp->ops->print_recreate) (tp, fp);
     else
       {
-	sprintf_vma (tmp, tp->loc->address);
-	fprintf_unfiltered (fp, " *0x%s", tmp);
+	if (tp->type == bp_fast_tracepoint)
+	  fprintf_unfiltered (fp, "ftrace");
+	else if (tp->type == bp_tracepoint)
+	  fprintf_unfiltered (fp, "trace");
+	else if (tp->type == bp_breakpoint && tp->disposition == disp_del)
+	  fprintf_unfiltered (fp, "tbreak");
+	else if (tp->type == bp_breakpoint)
+	  fprintf_unfiltered (fp, "break");
+	else if (tp->type == bp_hardware_breakpoint
+		 && tp->disposition == disp_del)
+	  fprintf_unfiltered (fp, "thbreak");
+	else if (tp->type == bp_hardware_breakpoint)
+	  fprintf_unfiltered (fp, "hbreak");
+	else if (tp->type == bp_watchpoint)
+	  fprintf_unfiltered (fp, "watch");
+	else if (tp->type == bp_hardware_watchpoint)
+	  fprintf_unfiltered (fp, "watch");
+	else if (tp->type == bp_read_watchpoint)
+	  fprintf_unfiltered (fp, "rwatch");
+	else if (tp->type == bp_access_watchpoint)
+	  fprintf_unfiltered (fp, "awatch");
+	else
+	  internal_error (__FILE__, __LINE__,
+			  _("unhandled breakpoint type %d"),
+			  (int) tp->type);
+
+	if (tp->exp_string)
+	  fprintf_unfiltered (fp, " %s", tp->exp_string);
+	else if (tp->addr_string)
+	  fprintf_unfiltered (fp, " %s", tp->addr_string);
+	else
+	  {
+	    sprintf_vma (tmp, tp->loc->address);
+	    fprintf_unfiltered (fp, " *0x%s", tmp);
+	  }
       }
 
+    if (tp->thread != -1)
+      fprintf_unfiltered (fp, " thread %d", tp->thread);
+
+    if (tp->task != 0)
+      fprintf_unfiltered (fp, " task %d", tp->task);
+
     if (tp->cond_string)
       fprintf_unfiltered (fp, " if %s", tp->cond_string);
 
     fprintf_unfiltered (fp, "\n");
 
+    /* Note, can't rely on tp->number for anything, as we can't assume
+       the recreated breakpoint numbers will match.  Use $bpnum
+       instead.  */
+
+    if (tp->ignore_count)
+      fprintf_unfiltered (fp, "  ignore $bpnum %d\n", tp->ignore_count);
+
     if (tp->pass_count)
       fprintf_unfiltered (fp, "  passcount %d\n", tp->pass_count);
 
@@ -10708,7 +10847,7 @@ tracepoint_save_command (char *args, int
       {
 	volatile struct gdb_exception ex;	
 
-	fprintf_unfiltered (fp, "  actions\n");
+	fprintf_unfiltered (fp, "  commands\n");
 	
 	ui_out_redirect (uiout, fp);
 	TRY_CATCH (ex, RETURN_MASK_ERROR)
@@ -10722,15 +10861,44 @@ tracepoint_save_command (char *args, int
 
 	fprintf_unfiltered (fp, "  end\n");
       }
+
+    if (tp->enable_state == bp_disabled)
+      fprintf_unfiltered (fp, "disable $bpnum\n");
+
+    /* If this is a multi-location breakpoint, check if the locations
+       should be individually disabled.  Watchpoint locations are
+       special, and not user visible.  */
+    if (!is_watchpoint (tp) && tp->loc && tp->loc->next)
+      {
+	struct bp_location *loc;
+	int n = 1;
+
+	for (loc = tp->loc; loc != NULL; loc = loc->next, n++)
+	  if (!loc->enabled)
+	    fprintf_unfiltered (fp, "disable $bpnum.%d\n", n);
+      }
   }
 
-  if (*default_collect)
+  if (extra_trace_bits && *default_collect)
     fprintf_unfiltered (fp, "set default-collect %s\n", default_collect);
 
   do_cleanups (cleanup);
   if (from_tty)
-    printf_filtered (_("Tracepoints saved to file '%s'.\n"), args);
-  return;
+    printf_filtered (_("Saved to file '%s'.\n"), args);
+}
+
+/* save-breakpoints command */
+static void
+save_breakpoints_command (char *args, int from_tty)
+{
+  breakpoint_save (args, from_tty, NULL);
+}
+
+/* save-tracepoints command */
+static void
+tracepoint_save_command (char *args, int from_tty)
+{
+  breakpoint_save (args, from_tty, is_tracepoint);
 }
 
 /* Create a vector of all tracepoints.  */
@@ -11232,6 +11400,11 @@ Save current tracepoint definitions as a
 Use the 'source' command in another debug session to restore them."));
   set_cmd_completer (c, filename_completer);
 
+  c = add_com ("save-breakpoints", class_breakpoint, save_breakpoints_command, _("\
+Save current breakpoint definitions as a script.\n\
+Use the 'source' command in another debug session to restore them."));
+  set_cmd_completer (c, filename_completer);
+
   add_prefix_cmd ("breakpoint", class_maintenance, set_breakpoint_cmd, _("\
 Breakpoint specific settings\n\
 Configure various breakpoint-specific variables such as\n\
Index: src/gdb/breakpoint.h
===================================================================
--- src.orig/gdb/breakpoint.h	2010-04-09 00:00:42.000000000 +0100
+++ src/gdb/breakpoint.h	2010-04-09 01:52:13.000000000 +0100
@@ -362,6 +362,9 @@ struct breakpoint_ops 
   /* Display information about this breakpoint after setting it (roughly
      speaking; this is called from "mention").  */
   void (*print_mention) (struct breakpoint *);
+
+  /* Print to FP the CLI command that recreates this breakpoint.  */
+  void (*print_recreate) (struct breakpoint *, struct ui_file *fp);
 };
 
 enum watchpoint_triggered
Index: src/gdb/ada-lang.c
===================================================================
--- src.orig/gdb/ada-lang.c	2010-04-09 01:45:10.000000000 +0100
+++ src/gdb/ada-lang.c	2010-04-09 03:09:50.000000000 +0100
@@ -10348,6 +10348,35 @@ print_mention_exception (enum exception_
     }
 }
 
+/* Implement the PRINT_RECREATE method in the breakpoint_ops structure
+   for all exception catchpoint kinds.  */
+
+static void
+print_recreate_exception (enum exception_catchpoint_kind ex,
+			  struct breakpoint *b, struct ui_file *fp)
+{
+  switch (ex)
+    {
+      case ex_catch_exception:
+	fprintf_filtered (fp, "catch exception");
+	if (b->exp_string != NULL)
+	  fprintf_filtered (fp, " %s", b->exp_string);
+	break;
+
+      case ex_catch_exception_unhandled:
+	fprintf_filtered (fp, "catch unhandled");
+	break;
+
+      case ex_catch_assert:
+	fprintf_filtered (fp, "catch assert");
+	break;
+
+      default:
+	internal_error (__FILE__, __LINE__, _("unexpected catchpoint type"));
+	break;
+    }
+}
+
 /* Virtual table for "catch exception" breakpoints.  */
 
 static enum print_stop_action
@@ -10368,6 +10397,12 @@ print_mention_catch_exception (struct br
   print_mention_exception (ex_catch_exception, b);
 }
 
+static void
+print_recreate_catch_exception (struct breakpoint *b, struct ui_file *fp)
+{
+  print_recreate_exception (ex_catch_exception, b, fp);
+}
+
 static struct breakpoint_ops catch_exception_breakpoint_ops =
 {
   NULL, /* insert */
@@ -10375,7 +10410,8 @@ static struct breakpoint_ops catch_excep
   NULL, /* breakpoint_hit */
   print_it_catch_exception,
   print_one_catch_exception,
-  print_mention_catch_exception
+  print_mention_catch_exception,
+  print_recreate_catch_exception
 };
 
 /* Virtual table for "catch exception unhandled" breakpoints.  */
@@ -10399,13 +10435,21 @@ print_mention_catch_exception_unhandled 
   print_mention_exception (ex_catch_exception_unhandled, b);
 }
 
+static void
+print_recreate_catch_exception_unhandled (struct breakpoint *b,
+					  struct ui_file *fp)
+{
+  print_recreate_exception (ex_catch_exception_unhandled, b, fp);
+}
+
 static struct breakpoint_ops catch_exception_unhandled_breakpoint_ops = {
   NULL, /* insert */
   NULL, /* remove */
   NULL, /* breakpoint_hit */
   print_it_catch_exception_unhandled,
   print_one_catch_exception_unhandled,
-  print_mention_catch_exception_unhandled
+  print_mention_catch_exception_unhandled,
+  print_recreate_catch_exception_unhandled
 };
 
 /* Virtual table for "catch assert" breakpoints.  */
@@ -10428,13 +10472,20 @@ print_mention_catch_assert (struct break
   print_mention_exception (ex_catch_assert, b);
 }
 
+static void
+print_recreate_catch_assert (struct breakpoint *b, struct ui_file *fp)
+{
+  print_recreate_exception (ex_catch_assert, b, fp);
+}
+
 static struct breakpoint_ops catch_assert_breakpoint_ops = {
   NULL, /* insert */
   NULL, /* remove */
   NULL, /* breakpoint_hit */
   print_it_catch_assert,
   print_one_catch_assert,
-  print_mention_catch_assert
+  print_mention_catch_assert,
+  print_recreate_catch_assert
 };
 
 /* Return non-zero if B is an Ada exception catchpoint.  */
Index: src/gdb/doc/gdb.texinfo
===================================================================
--- src.orig/gdb/doc/gdb.texinfo	2010-04-09 02:34:45.000000000 +0100
+++ src/gdb/doc/gdb.texinfo	2010-04-09 03:20:04.000000000 +0100
@@ -3247,6 +3247,7 @@ all breakpoints in that range are operat
 * Disabling::                   Disabling breakpoints
 * Conditions::                  Break conditions
 * Break Commands::              Breakpoint command lists
+* Save Breakpoints::            How to save breakpoints in a file
 * Error in Breakpoints::        ``Cannot insert breakpoints''
 * Breakpoint-related Warnings:: ``Breakpoint address adjusted...''
 @end menu
@@ -4401,6 +4402,23 @@ cont
 end
 @end smallexample
 
+@node Save Breakpoints
+@subsection How to save breakpoints to a file
+
+To save breakpoint definitions to a file use the
+@code{save-breakpoints} command.
+
+@table @code
+@kindex save-breakpoints
+@cindex save breakpoints to a file for future sessions
+@item save-breakpoints [@var{filename}]
+This command saves all current breakpoint definitions together with
+their commands and ignore counts, into a file @file{@var{filename}}
+suitable for use in a later debugging session.  To read the saved
+breakpoint definitions, use the @code{source} command (@pxref{Command
+Files}).
+@end table
+
 @c  @ifclear BARETARGET
 @node Error in Breakpoints
 @subsection ``Cannot insert breakpoints''
Index: src/gdb/NEWS
===================================================================
--- src.orig/gdb/NEWS	2010-04-09 02:38:50.000000000 +0100
+++ src/gdb/NEWS	2010-04-09 02:44:43.000000000 +0100
@@ -23,6 +23,13 @@
   single `break' command creates multiple breakpoints (e.g.,
   breakpoints on overloaded c++ functions).
 
+* New commands
+
+save-breakpoints filename
+  Save all current breakpoint definitions to a file suitable for use
+  in a later debugging session.  To read the saved breakpoint
+  definitions, use the `source' command.
+
 * Python scripting
 
 ** The GDB Python API now has access to symbols, symbol tables, and
Index: src/gdb/testsuite/gdb.trace/save-trace.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.trace/save-trace.exp	2010-04-09 03:29:29.000000000 +0100
+++ src/gdb/testsuite/gdb.trace/save-trace.exp	2010-04-09 03:29:49.000000000 +0100
@@ -129,7 +129,7 @@ gdb_verify_tracepoints "10.x: verify tra
 
 remote_file host delete savetrace.tr
 gdb_test "save-tracepoints savetrace.tr" \
-	"Tracepoints saved to file 'savetrace.tr'." \
+	"Saved to file 'savetrace.tr'." \
 	"10.1: save tracepoint definitions"
 
 # 10.2 Read back tracepoint definitions
@@ -147,7 +147,7 @@ remote_file host delete savetrace.tr
 set escapedfilename [string_to_regexp $objdir/savetrace.tr]
 remote_file host delete $objdir/savetrace.tr
 gdb_test "save-tracepoints $objdir/savetrace.tr" \
-	"Tracepoints saved to file '${escapedfilename}'." \
+	"Saved to file '${escapedfilename}'." \
 	"10.3: save tracepoint definitions, full path"
 
 gdb_delete_tracepoints


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: PR8554: New command to save breakpoints to a file
  2010-04-09  2:41 PR8554: New command to save breakpoints to a file Pedro Alves
@ 2010-04-09 16:17 ` Tom Tromey
  2010-04-09 17:23   ` Pedro Alves
  0 siblings, 1 reply; 23+ messages in thread
From: Tom Tromey @ 2010-04-09 16:17 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:

Pedro> (I finished this instead of dumping it.)

Thanks.  I do like this approach.

Pedro> Add a new save-breakpoints command to save breakpoint definitions
Pedro> to a file.

I'd personally prefer "save breakpoints", with a space, and make
save-tracepoints a deprecated alias for "save tracepoints".  What do you
(and others) think of this?  I tend to like simple commands with spaces,
especially when a subcommand comes along.

Pedro> The new breakpoint_ops->print_recreate method implementation for
Pedro> all catchpoints is always mostly a simplified version of
Pedro> breakpoint_ops->print_mention method.

Could you enlighten me on a historical (?) point?  Why is it that some
kinds of breakpoints have methods like this and some do not?  Is this an
incomplete transition, or an intentional design choice?

Pedro> +    if (tp->thread != -1)
Pedro> +      fprintf_unfiltered (fp, " thread %d", tp->thread);
Pedro> +
Pedro> +    if (tp->task != 0)
Pedro> +      fprintf_unfiltered (fp, " task %d", tp->task);
Pedro> +
Pedro>      if (tp->cond_string)
Pedro>        fprintf_unfiltered (fp, " if %s", tp->cond_string);

I don't think this syntax will work for a conditional catchpoint.
Our Python-based implementation gets this wrong as well.  I think you
need a separate "cond" command in the output.

Tom


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: PR8554: New command to save breakpoints to a file
  2010-04-09 16:17 ` Tom Tromey
@ 2010-04-09 17:23   ` Pedro Alves
  2010-04-10  4:10     ` Sergio Durigan Junior
  2010-04-12 18:15     ` Pedro Alves
  0 siblings, 2 replies; 23+ messages in thread
From: Pedro Alves @ 2010-04-09 17:23 UTC (permalink / raw)
  To: gdb-patches, tromey

On Friday 09 April 2010 17:17:31, Tom Tromey wrote:
> >>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:
> 
> Pedro> (I finished this instead of dumping it.)
> 
> Thanks.  I do like this approach.
> 
> Pedro> Add a new save-breakpoints command to save breakpoint definitions
> Pedro> to a file.
> 
> I'd personally prefer "save breakpoints", with a space, and make
> save-tracepoints a deprecated alias for "save tracepoints".  What do you
> (and others) think of this?  I tend to like simple commands with spaces,
> especially when a subcommand comes along.

Fine with me.

> Pedro> The new breakpoint_ops->print_recreate method implementation for
> Pedro> all catchpoints is always mostly a simplified version of
> Pedro> breakpoint_ops->print_mention method.
> 
> Could you enlighten me on a historical (?) point?  Why is it that some
> kinds of breakpoints have methods like this and some do not?  Is this an
> incomplete transition, or an intentional design choice?

I think the former.  breakpoint_ops was added initialy for some
catchpoints in 2003.  I wasn't around then.

> 
> Pedro> +    if (tp->thread != -1)
> Pedro> +      fprintf_unfiltered (fp, " thread %d", tp->thread);
> Pedro> +
> Pedro> +    if (tp->task != 0)
> Pedro> +      fprintf_unfiltered (fp, " task %d", tp->task);
> Pedro> +
> Pedro>      if (tp->cond_string)
> Pedro>        fprintf_unfiltered (fp, " if %s", tp->cond_string);
> 
> I don't think this syntax will work for a conditional catchpoint.
> Our Python-based implementation gets this wrong as well.  I think you
> need a separate "cond" command in the output.

You're right.  It works for breakpoints, but not for other types:

 (top-gdb) b main thread 1 if 0
 Breakpoint 4 at 0x4572b3: file ../../src/gdb/gdb.c, line 28.
 (top-gdb) watch main thread 1 if 0
 Junk at end of command.

The `b *0xaddr thread 1 if 0' syntax may not work for
some languages.

I'll post an updated patch, once I update it.  :-)

-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: PR8554: New command to save breakpoints to a file
  2010-04-09 17:23   ` Pedro Alves
@ 2010-04-10  4:10     ` Sergio Durigan Junior
  2010-04-12 18:15     ` Pedro Alves
  1 sibling, 0 replies; 23+ messages in thread
From: Sergio Durigan Junior @ 2010-04-10  4:10 UTC (permalink / raw)
  To: gdb-patches; +Cc: Pedro Alves, tromey

On Friday 09 April 2010 14:23:31, Pedro Alves wrote:
> > Pedro> The new breakpoint_ops->print_recreate method implementation for
> > Pedro> all catchpoints is always mostly a simplified version of
> > Pedro> breakpoint_ops->print_mention method.
> > 
> > Could you enlighten me on a historical (?) point?  Why is it that some
> > kinds of breakpoints have methods like this and some do not?  Is this an
> > incomplete transition, or an intentional design choice?
> 
> I think the former.  breakpoint_ops was added initialy for some
> catchpoints in 2003.  I wasn't around then.

FWIW, Joel has kindly made fork/vfork catchpoints use the breakpoint_ops, but 
this was only in 2008.

http://sourceware.org/ml/gdb-patches/2008-10/msg00148.html

-- 
Sergio


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: PR8554: New command to save breakpoints to a file
  2010-04-09 17:23   ` Pedro Alves
  2010-04-10  4:10     ` Sergio Durigan Junior
@ 2010-04-12 18:15     ` Pedro Alves
  2010-04-15 17:52       ` Michael Snyder
  1 sibling, 1 reply; 23+ messages in thread
From: Pedro Alves @ 2010-04-12 18:15 UTC (permalink / raw)
  To: gdb-patches; +Cc: tromey

On Friday 09 April 2010 18:23:31, Pedro Alves wrote:
> On Friday 09 April 2010 17:17:31, Tom Tromey wrote:
> > >>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:
> > 
> > Pedro> (I finished this instead of dumping it.)
> > 
> > Thanks.  I do like this approach.
> > 
> > Pedro> Add a new save-breakpoints command to save breakpoint definitions
> > Pedro> to a file.
> > 
> > I'd personally prefer "save breakpoints", with a space, and make
> > save-tracepoints a deprecated alias for "save tracepoints".  What do you
> > (and others) think of this?  I tend to like simple commands with spaces,
> > especially when a subcommand comes along.
> 
> Fine with me.

Done now.

> > Pedro> +    if (tp->thread != -1)
> > Pedro> +      fprintf_unfiltered (fp, " thread %d", tp->thread);
> > Pedro> +
> > Pedro> +    if (tp->task != 0)
> > Pedro> +      fprintf_unfiltered (fp, " task %d", tp->task);
> > Pedro> +
> > Pedro>      if (tp->cond_string)
> > Pedro>        fprintf_unfiltered (fp, " if %s", tp->cond_string);
> > 
> > I don't think this syntax will work for a conditional catchpoint.
> > Our Python-based implementation gets this wrong as well.  I think you
> > need a separate "cond" command in the output.
> 
> You're right.  It works for breakpoints, but not for other types:
> 
>  (top-gdb) b main thread 1 if 0
>  Breakpoint 4 at 0x4572b3: file ../../src/gdb/gdb.c, line 28.
>  (top-gdb) watch main thread 1 if 0
>  Junk at end of command.
> 
> The `b *0xaddr thread 1 if 0' syntax may not work for
> some languages.

Also done.

> I'll post an updated patch, once I update it.  :-)

Here's the updated patch.  Docs and NEWS also updated to
reflect the new save-tracepoints -> `save tracepoints' alias,
and the deprecation of the former.  Eli, do you want to take
a look at the documentation and NEWS bits?

-- 
Pedro Alves

2010-04-12  Pedro Alves  <pedro@codesourcery.com>

	PR breakpoints/8554.

	Implement `save-breakpoints'.

	gdb/
	
	* breakpoint.c (print_recreate_catch_fork): New.
	(catch_fork_breakpoint_ops): Install it.
	(print_recreate_catch_vfork): New.
	(catch_vfork_breakpoint_ops): Install it.
	(print_recreate_catch_syscall): New.
	(catch_syscall_breakpoint_ops): Install it.
	(print_recreate_catch_exec): New.
	(catch_exec_breakpoint_ops): Install it.
	(print_recreate_exception_catchpoint): New.
	(gnu_v3_exception_catchpoint_ops): Install it.
	(save_breakpoints): New, based on tracepoint_save_command, but
	handle all breakpoint types.
	(save_breakpoints_command): New.
	(tracepoint_save_command): Rename to...
	(save_tracepoints_command): ... this, and reimplement using
	save_breakpoints.
	(save_command): New.
	
	(_initialize_breakpoints): Install the "save" command prefix.
	Install the "save breakpoints" command.  Make "save-tracepoints" a
	deprecated alias for "save tracepoints".

	* breakpoint.h (struct breakpoint_ops): New field `print_recreate'.
	* ada-lang.c (print_recreate_exception): New.
	(print_recreate_catch_exception): New.
	(catch_exception_breakpoint_ops): Install it.
	(print_recreate_catch_exception_unhandled): New.
	(catch_exception_unhandled_breakpoint_ops): Install it.
	(print_recreate_catch_assert): New.
	(catch_assert_breakpoint_ops): Install it.

	* NEWS: Mention the new `save breakpoints' command.  Mention the
	new `save tracepoints' alias and that `save-tracepoints' is now
	deprecated.

	doc/
	* gdb.texinfo (Save Breakpoints): New node.
	(save-tracepoints): Rename to ...
	(save tracepoints): ... this.  Mention that `save-tracepoints' is
	a deprecated alias to `save tracepoints'.

	gdb/testsuite/
	* gdb.trace/save-trace.exp: Adjust.

---
 gdb/NEWS                               |   10 +
 gdb/ada-lang.c                         |   56 ++++++
 gdb/breakpoint.c                       |  276 ++++++++++++++++++++++++++++-----
 gdb/breakpoint.h                       |    3 
 gdb/doc/gdb.texinfo                    |   28 ++-
 gdb/testsuite/gdb.trace/save-trace.exp |    4 
 6 files changed, 330 insertions(+), 47 deletions(-)

Index: src/gdb/breakpoint.c
===================================================================
--- src.orig/gdb/breakpoint.c	2010-04-12 09:47:53.000000000 +0100
+++ src/gdb/breakpoint.c	2010-04-12 19:06:44.000000000 +0100
@@ -5870,6 +5870,15 @@ print_mention_catch_fork (struct breakpo
   printf_filtered (_("Catchpoint %d (fork)"), b->number);
 }
 
+/* Implement the "print_recreate" breakpoint_ops method for fork
+   catchpoints.  */
+
+static void
+print_recreate_catch_fork (struct breakpoint *b, struct ui_file *fp)
+{
+  fprintf_unfiltered (fp, "catch fork");
+}
+
 /* The breakpoint_ops structure to be used in fork catchpoints.  */
 
 static struct breakpoint_ops catch_fork_breakpoint_ops =
@@ -5879,7 +5888,8 @@ static struct breakpoint_ops catch_fork_
   breakpoint_hit_catch_fork,
   print_it_catch_fork,
   print_one_catch_fork,
-  print_mention_catch_fork
+  print_mention_catch_fork,
+  print_recreate_catch_fork
 };
 
 /* Implement the "insert" breakpoint_ops method for vfork catchpoints.  */
@@ -5951,6 +5961,15 @@ print_mention_catch_vfork (struct breakp
   printf_filtered (_("Catchpoint %d (vfork)"), b->number);
 }
 
+/* Implement the "print_recreate" breakpoint_ops method for vfork
+   catchpoints.  */
+
+static void
+print_recreate_catch_vfork (struct breakpoint *b, struct ui_file *fp)
+{
+  fprintf_unfiltered (fp, "catch vfork");
+}
+
 /* The breakpoint_ops structure to be used in vfork catchpoints.  */
 
 static struct breakpoint_ops catch_vfork_breakpoint_ops =
@@ -5960,7 +5979,8 @@ static struct breakpoint_ops catch_vfork
   breakpoint_hit_catch_vfork,
   print_it_catch_vfork,
   print_one_catch_vfork,
-  print_mention_catch_vfork
+  print_mention_catch_vfork,
+  print_recreate_catch_vfork
 };
 
 /* Implement the "insert" breakpoint_ops method for syscall
@@ -6199,6 +6219,33 @@ print_mention_catch_syscall (struct brea
                      b->number);
 }
 
+/* Implement the "print_recreate" breakpoint_ops method for syscall
+   catchpoints.  */
+
+static void
+print_recreate_catch_syscall (struct breakpoint *b, struct ui_file *fp)
+{
+  fprintf_unfiltered (fp, "catch syscall");
+
+  if (b->syscalls_to_be_caught)
+    {
+      int i, iter;
+
+      for (i = 0;
+           VEC_iterate (int, b->syscalls_to_be_caught, i, iter);
+           i++)
+        {
+          struct syscall s;
+
+          get_syscall_by_number (iter, &s);
+          if (s.name)
+            fprintf_unfiltered (fp, " %s", s.name);
+          else
+            fprintf_unfiltered (fp, " %d", s.number);
+        }
+    }
+}
+
 /* The breakpoint_ops structure to be used in syscall catchpoints.  */
 
 static struct breakpoint_ops catch_syscall_breakpoint_ops =
@@ -6208,7 +6255,8 @@ static struct breakpoint_ops catch_sysca
   breakpoint_hit_catch_syscall,
   print_it_catch_syscall,
   print_one_catch_syscall,
-  print_mention_catch_syscall
+  print_mention_catch_syscall,
+  print_recreate_catch_syscall
 };
 
 /* Returns non-zero if 'b' is a syscall catchpoint.  */
@@ -6344,6 +6392,15 @@ print_mention_catch_exec (struct breakpo
   printf_filtered (_("Catchpoint %d (exec)"), b->number);
 }
 
+/* Implement the "print_recreate" breakpoint_ops method for exec
+   catchpoints.  */
+
+static void
+print_recreate_catch_exec (struct breakpoint *b, struct ui_file *fp)
+{
+  fprintf_unfiltered (fp, "catch exec");
+}
+
 static struct breakpoint_ops catch_exec_breakpoint_ops =
 {
   insert_catch_exec,
@@ -6351,7 +6408,8 @@ static struct breakpoint_ops catch_exec_
   breakpoint_hit_catch_exec,
   print_it_catch_exec,
   print_one_catch_exec,
-  print_mention_catch_exec
+  print_mention_catch_exec,
+  print_recreate_catch_exec
 };
 
 static void
@@ -8311,13 +8369,29 @@ print_mention_exception_catchpoint (stru
 			       : _(" (catch)"));
 }
 
+/* Implement the "print_recreate" breakpoint_ops method for throw and
+   catch catchpoints.  */
+
+static void
+print_recreate_exception_catchpoint (struct breakpoint *b, struct ui_file *fp)
+{
+  int bp_temp;
+  int bp_throw;
+
+  bp_temp = b->disposition == disp_del;
+  bp_throw = strstr (b->addr_string, "throw") != NULL;
+  fprintf_unfiltered (fp, bp_temp ? "tcatch " : "catch ");
+  fprintf_unfiltered (fp, bp_throw ? "throw" : "catch");
+}
+
 static struct breakpoint_ops gnu_v3_exception_catchpoint_ops = {
   NULL, /* insert */
   NULL, /* remove */
   NULL, /* breakpoint_hit */
   print_exception_catchpoint,
   print_one_exception_catchpoint,
-  print_mention_exception_catchpoint
+  print_mention_exception_catchpoint,
+  print_recreate_exception_catchpoint
 };
 
 static int
@@ -10678,63 +10752,134 @@ get_tracepoint_by_number (char **arg, in
   return NULL;
 }
 
-/* save-tracepoints command */
+/* Save information on user settable breakpoints (watchpoints, etc) to
+   a new script file named FILENAME.  If FILTER is non-NULL, call it
+   on each breakpoint and only include the ones for which it returns
+   non-zero.  */
+
 static void
-tracepoint_save_command (char *args, int from_tty)
+save_breakpoints (char *filename, int from_tty,
+		  int (*filter) (const struct breakpoint *))
 {
   struct breakpoint *tp;
-  int any_tp = 0;
-  struct command_line *line;
+  int any = 0;
   char *pathname;
-  char tmp[40];
   struct cleanup *cleanup;
   struct ui_file *fp;
+  int extra_trace_bits = 0;
 
-  if (args == 0 || *args == 0)
-    error (_("Argument required (file name in which to save tracepoints)"));
+  if (filename == 0 || *filename == 0)
+    error (_("Argument required (file name in which to save)"));
 
   /* See if we have anything to save.  */
-  ALL_TRACEPOINTS (tp)
+  ALL_BREAKPOINTS (tp)
   {
-    any_tp = 1;
-    break;
+    /* Skip internal and momentary breakpoints.  */
+    if (tp->number <= 0)
+      continue;
+
+    /* If we have a filter, only save the breakpoints it accepts.  */
+    if (filter && !filter (tp))
+      continue;
+
+    any = 1;
+
+    if (is_tracepoint (tp))
+      {
+	extra_trace_bits = 1;
+
+	/* We can stop searching.  */
+	break;
+      }
   }
-  if (!any_tp)
+
+  if (!any)
     {
-      warning (_("save-tracepoints: no tracepoints to save."));
+      warning (_("Nothing to save."));
       return;
     }
 
-  pathname = tilde_expand (args);
+  pathname = tilde_expand (filename);
   cleanup = make_cleanup (xfree, pathname);
   fp = gdb_fopen (pathname, "w");
   if (!fp)
-    error (_("Unable to open file '%s' for saving tracepoints (%s)"),
-	   args, safe_strerror (errno));
+    error (_("Unable to open file '%s' for saving (%s)"),
+	   filename, safe_strerror (errno));
   make_cleanup_ui_file_delete (fp);
 
-  save_trace_state_variables (fp);
+  if (extra_trace_bits)
+    save_trace_state_variables (fp);
 
-  ALL_TRACEPOINTS (tp)
+  ALL_BREAKPOINTS (tp)
   {
-    if (tp->type == bp_fast_tracepoint)
-      fprintf_unfiltered (fp, "ftrace");
-    else
-      fprintf_unfiltered (fp, "trace");
+    /* Skip internal and momentary breakpoints.  */
+    if (tp->number <= 0)
+      continue;
+
+    /* If we have a filter, only save the breakpoints it accepts.  */
+    if (filter && !filter (tp))
+      continue;
 
-    if (tp->addr_string)
-      fprintf_unfiltered (fp, " %s", tp->addr_string);
+    if (tp->ops != NULL)
+      (tp->ops->print_recreate) (tp, fp);
     else
       {
-	sprintf_vma (tmp, tp->loc->address);
-	fprintf_unfiltered (fp, " *0x%s", tmp);
+	if (tp->type == bp_fast_tracepoint)
+	  fprintf_unfiltered (fp, "ftrace");
+	else if (tp->type == bp_tracepoint)
+	  fprintf_unfiltered (fp, "trace");
+	else if (tp->type == bp_breakpoint && tp->disposition == disp_del)
+	  fprintf_unfiltered (fp, "tbreak");
+	else if (tp->type == bp_breakpoint)
+	  fprintf_unfiltered (fp, "break");
+	else if (tp->type == bp_hardware_breakpoint
+		 && tp->disposition == disp_del)
+	  fprintf_unfiltered (fp, "thbreak");
+	else if (tp->type == bp_hardware_breakpoint)
+	  fprintf_unfiltered (fp, "hbreak");
+	else if (tp->type == bp_watchpoint)
+	  fprintf_unfiltered (fp, "watch");
+	else if (tp->type == bp_hardware_watchpoint)
+	  fprintf_unfiltered (fp, "watch");
+	else if (tp->type == bp_read_watchpoint)
+	  fprintf_unfiltered (fp, "rwatch");
+	else if (tp->type == bp_access_watchpoint)
+	  fprintf_unfiltered (fp, "awatch");
+	else
+	  internal_error (__FILE__, __LINE__,
+			  _("unhandled breakpoint type %d"), (int) tp->type);
+
+	if (tp->exp_string)
+	  fprintf_unfiltered (fp, " %s", tp->exp_string);
+	else if (tp->addr_string)
+	  fprintf_unfiltered (fp, " %s", tp->addr_string);
+	else
+	  {
+	    char tmp[40];
+
+	    sprintf_vma (tmp, tp->loc->address);
+	    fprintf_unfiltered (fp, " *0x%s", tmp);
+	  }
       }
 
-    if (tp->cond_string)
-      fprintf_unfiltered (fp, " if %s", tp->cond_string);
+    if (tp->thread != -1)
+      fprintf_unfiltered (fp, " thread %d", tp->thread);
+
+    if (tp->task != 0)
+      fprintf_unfiltered (fp, " task %d", tp->task);
 
     fprintf_unfiltered (fp, "\n");
 
+    /* Note, we can't rely on tp->number for anything, as we can't
+       assume the recreated breakpoint numbers will match.  Use $bpnum
+       instead.  */
+
+    if (tp->cond_string)
+      fprintf_unfiltered (fp, "  condition $bpnum %s\n", tp->cond_string);
+
+    if (tp->ignore_count)
+      fprintf_unfiltered (fp, "  ignore $bpnum %d\n", tp->ignore_count);
+
     if (tp->pass_count)
       fprintf_unfiltered (fp, "  passcount %d\n", tp->pass_count);
 
@@ -10742,7 +10887,7 @@ tracepoint_save_command (char *args, int
       {
 	volatile struct gdb_exception ex;	
 
-	fprintf_unfiltered (fp, "  actions\n");
+	fprintf_unfiltered (fp, "  commands\n");
 	
 	ui_out_redirect (uiout, fp);
 	TRY_CATCH (ex, RETURN_MASK_ERROR)
@@ -10756,15 +10901,46 @@ tracepoint_save_command (char *args, int
 
 	fprintf_unfiltered (fp, "  end\n");
       }
+
+    if (tp->enable_state == bp_disabled)
+      fprintf_unfiltered (fp, "disable\n");
+
+    /* If this is a multi-location breakpoint, check if the locations
+       should be individually disabled.  Watchpoint locations are
+       special, and not user visible.  */
+    if (!is_watchpoint (tp) && tp->loc && tp->loc->next)
+      {
+	struct bp_location *loc;
+	int n = 1;
+
+	for (loc = tp->loc; loc != NULL; loc = loc->next, n++)
+	  if (!loc->enabled)
+	    fprintf_unfiltered (fp, "disable $bpnum.%d\n", n);
+      }
   }
 
-  if (*default_collect)
+  if (extra_trace_bits && *default_collect)
     fprintf_unfiltered (fp, "set default-collect %s\n", default_collect);
 
   do_cleanups (cleanup);
   if (from_tty)
-    printf_filtered (_("Tracepoints saved to file '%s'.\n"), args);
-  return;
+    printf_filtered (_("Saved to file '%s'.\n"), filename);
+}
+
+/* The `save breakpoints' command.  */
+
+static void
+save_breakpoints_command (char *args, int from_tty)
+{
+  save_breakpoints (args, from_tty, NULL);
+}
+
+/* The `save tracepoints' command.  */
+
+static void
+save_tracepoints_command (char *args, int from_tty)
+{
+  save_breakpoints (args, from_tty, is_tracepoint);
 }
 
 /* Create a vector of all tracepoints.  */
@@ -10843,11 +11019,18 @@ clear_syscall_counts (struct inferior *i
   VEC_free (int, inf->syscalls_counts);
 }
 
+static void
+save_command (char *arg, int from_tty)
+{
+  /* Dummy function to keep add_prefix_cmd happy.  */
+}
+
 void
 _initialize_breakpoint (void)
 {
   static struct cmd_list_element *breakpoint_set_cmdlist;
   static struct cmd_list_element *breakpoint_show_cmdlist;
+  static struct cmd_list_element *save_cmdlist;
   struct cmd_list_element *c;
 
   observer_attach_solib_unloaded (disable_breakpoints_in_unloaded_shlib);
@@ -11261,11 +11444,28 @@ The trace will end when the tracepoint h
 Usage: passcount COUNT TPNUM, where TPNUM may also be \"all\";\n\
 if TPNUM is omitted, passcount refers to the last tracepoint defined."));
 
-  c = add_com ("save-tracepoints", class_trace, tracepoint_save_command, _("\
+  add_prefix_cmd ("save", class_breakpoint, save_command,
+		  _("Save breakpoint definitions as a script."),
+		  &save_cmdlist, "save ",
+		  0/*allow-unknown*/, &cmdlist);
+
+  c = add_cmd ("breakpoints", class_breakpoint, save_breakpoints_command, _("\
+Save current breakpoint definitions as a script.\n\
+This includes all types of breakpoints (breakpoints, watchpoints, \n\
+catchpoints, tracepoints).  Use the 'source' command in another debug\n\
+session to restore them."),
+	       &save_cmdlist);
+  set_cmd_completer (c, filename_completer);
+
+  c = add_cmd ("tracepoints", class_trace, save_tracepoints_command, _("\
 Save current tracepoint definitions as a script.\n\
-Use the 'source' command in another debug session to restore them."));
+Use the 'source' command in another debug session to restore them."),
+	       &save_cmdlist);
   set_cmd_completer (c, filename_completer);
 
+  c = add_com_alias ("save-tracepoints", "save tracepoints", class_trace, 0);
+  deprecate_cmd (c, "save tracepoints");
+
   add_prefix_cmd ("breakpoint", class_maintenance, set_breakpoint_cmd, _("\
 Breakpoint specific settings\n\
 Configure various breakpoint-specific variables such as\n\
Index: src/gdb/breakpoint.h
===================================================================
--- src.orig/gdb/breakpoint.h	2010-04-12 09:47:53.000000000 +0100
+++ src/gdb/breakpoint.h	2010-04-12 18:51:30.000000000 +0100
@@ -362,6 +362,9 @@ struct breakpoint_ops 
   /* Display information about this breakpoint after setting it (roughly
      speaking; this is called from "mention").  */
   void (*print_mention) (struct breakpoint *);
+
+  /* Print to FP the CLI command that recreates this breakpoint.  */
+  void (*print_recreate) (struct breakpoint *, struct ui_file *fp);
 };
 
 enum watchpoint_triggered
Index: src/gdb/ada-lang.c
===================================================================
--- src.orig/gdb/ada-lang.c	2010-04-12 09:47:53.000000000 +0100
+++ src/gdb/ada-lang.c	2010-04-12 18:51:30.000000000 +0100
@@ -10348,6 +10348,34 @@ print_mention_exception (enum exception_
     }
 }
 
+/* Implement the PRINT_RECREATE method in the breakpoint_ops structure
+   for all exception catchpoint kinds.  */
+
+static void
+print_recreate_exception (enum exception_catchpoint_kind ex,
+			  struct breakpoint *b, struct ui_file *fp)
+{
+  switch (ex)
+    {
+      case ex_catch_exception:
+	fprintf_filtered (fp, "catch exception");
+	if (b->exp_string != NULL)
+	  fprintf_filtered (fp, " %s", b->exp_string);
+	break;
+
+      case ex_catch_exception_unhandled:
+	fprintf_filtered (fp, "catch unhandled");
+	break;
+
+      case ex_catch_assert:
+	fprintf_filtered (fp, "catch assert");
+	break;
+
+      default:
+	internal_error (__FILE__, __LINE__, _("unexpected catchpoint type"));
+    }
+}
+
 /* Virtual table for "catch exception" breakpoints.  */
 
 static enum print_stop_action
@@ -10368,6 +10396,12 @@ print_mention_catch_exception (struct br
   print_mention_exception (ex_catch_exception, b);
 }
 
+static void
+print_recreate_catch_exception (struct breakpoint *b, struct ui_file *fp)
+{
+  print_recreate_exception (ex_catch_exception, b, fp);
+}
+
 static struct breakpoint_ops catch_exception_breakpoint_ops =
 {
   NULL, /* insert */
@@ -10375,7 +10409,8 @@ static struct breakpoint_ops catch_excep
   NULL, /* breakpoint_hit */
   print_it_catch_exception,
   print_one_catch_exception,
-  print_mention_catch_exception
+  print_mention_catch_exception,
+  print_recreate_catch_exception
 };
 
 /* Virtual table for "catch exception unhandled" breakpoints.  */
@@ -10399,13 +10434,21 @@ print_mention_catch_exception_unhandled 
   print_mention_exception (ex_catch_exception_unhandled, b);
 }
 
+static void
+print_recreate_catch_exception_unhandled (struct breakpoint *b,
+					  struct ui_file *fp)
+{
+  print_recreate_exception (ex_catch_exception_unhandled, b, fp);
+}
+
 static struct breakpoint_ops catch_exception_unhandled_breakpoint_ops = {
   NULL, /* insert */
   NULL, /* remove */
   NULL, /* breakpoint_hit */
   print_it_catch_exception_unhandled,
   print_one_catch_exception_unhandled,
-  print_mention_catch_exception_unhandled
+  print_mention_catch_exception_unhandled,
+  print_recreate_catch_exception_unhandled
 };
 
 /* Virtual table for "catch assert" breakpoints.  */
@@ -10428,13 +10471,20 @@ print_mention_catch_assert (struct break
   print_mention_exception (ex_catch_assert, b);
 }
 
+static void
+print_recreate_catch_assert (struct breakpoint *b, struct ui_file *fp)
+{
+  print_recreate_exception (ex_catch_assert, b, fp);
+}
+
 static struct breakpoint_ops catch_assert_breakpoint_ops = {
   NULL, /* insert */
   NULL, /* remove */
   NULL, /* breakpoint_hit */
   print_it_catch_assert,
   print_one_catch_assert,
-  print_mention_catch_assert
+  print_mention_catch_assert,
+  print_recreate_catch_assert
 };
 
 /* Return non-zero if B is an Ada exception catchpoint.  */
Index: src/gdb/doc/gdb.texinfo
===================================================================
--- src.orig/gdb/doc/gdb.texinfo	2010-04-12 09:58:41.000000000 +0100
+++ src/gdb/doc/gdb.texinfo	2010-04-12 19:00:11.000000000 +0100
@@ -3247,6 +3247,7 @@ all breakpoints in that range are operat
 * Disabling::                   Disabling breakpoints
 * Conditions::                  Break conditions
 * Break Commands::              Breakpoint command lists
+* Save Breakpoints::            How to save breakpoints in a file
 * Error in Breakpoints::        ``Cannot insert breakpoints''
 * Breakpoint-related Warnings:: ``Breakpoint address adjusted...''
 @end menu
@@ -4401,6 +4402,23 @@ cont
 end
 @end smallexample
 
+@node Save Breakpoints
+@subsection How to save breakpoints to a file
+
+To save breakpoint definitions to a file use the @w{@code{save
+breakpoints}} command.
+
+@table @code
+@kindex save breakpoints
+@cindex save breakpoints to a file for future sessions
+@item save breakpoints [@var{filename}]
+This command saves all current breakpoint definitions together with
+their commands and ignore counts, into a file @file{@var{filename}}
+suitable for use in a later debugging session.  To read the saved
+breakpoint definitions, use the @code{source} command (@pxref{Command
+Files}).
+@end table
+
 @c  @ifclear BARETARGET
 @node Error in Breakpoints
 @subsection ``Cannot insert breakpoints''
@@ -9971,7 +9989,7 @@ the buffer will fail.
 @menu
 * tfind::                       How to select a trace snapshot
 * tdump::                       How to display all data for a snapshot
-* save-tracepoints::            How to save tracepoints for a future run
+* save tracepoints::            How to save tracepoints for a future run
 @end menu
 
 @node tfind
@@ -10166,8 +10184,9 @@ list, and may fail if a while-stepping f
 same data that is collected at the tracepoint hit.
 @c This is getting pretty arcane, example would be good.
 
-@node save-tracepoints
-@subsection @code{save-tracepoints @var{filename}}
+@node save tracepoints
+@subsection @code{save tracepoints @var{filename}}
+@kindex save tracepoints
 @kindex save-tracepoints
 @cindex save tracepoints for future sessions
 
@@ -10175,7 +10194,8 @@ This command saves all current tracepoin
 their actions and passcounts, into a file @file{@var{filename}}
 suitable for use in a later debugging session.  To read the saved
 tracepoint definitions, use the @code{source} command (@pxref{Command
-Files}).
+Files}).  The @w{@code{save-tracepoints}} command is a deprecated
+alias for @w{@code{save tracepoints}}
 
 @node Tracepoint Variables
 @section Convenience Variables for Tracepoints
Index: src/gdb/NEWS
===================================================================
--- src.orig/gdb/NEWS	2010-04-12 09:47:53.000000000 +0100
+++ src/gdb/NEWS	2010-04-12 18:53:31.000000000 +0100
@@ -28,6 +28,16 @@
   single `break' command creates multiple breakpoints (e.g.,
   breakpoints on overloaded c++ functions).
 
+* New commands
+
+save breakpoints <filename>
+  Save all current breakpoint definitions to a file suitable for use
+  in a later debugging session.  To read the saved breakpoint
+  definitions, use the `source' command.
+
+`save tracepoints' is a new alias for `save-tracepoints'.  The latter
+is now deprecated.
+
 * Python scripting
 
 ** The GDB Python API now has access to breakpoints, symbols, symbol
Index: src/gdb/testsuite/gdb.trace/save-trace.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.trace/save-trace.exp	2010-04-12 09:47:53.000000000 +0100
+++ src/gdb/testsuite/gdb.trace/save-trace.exp	2010-04-12 18:51:30.000000000 +0100
@@ -129,7 +129,7 @@ gdb_verify_tracepoints "10.x: verify tra
 
 remote_file host delete savetrace.tr
 gdb_test "save-tracepoints savetrace.tr" \
-	"Tracepoints saved to file 'savetrace.tr'." \
+	"Saved to file 'savetrace.tr'." \
 	"10.1: save tracepoint definitions"
 
 # 10.2 Read back tracepoint definitions
@@ -148,7 +148,7 @@ remote_file host delete savetrace.tr
 set escapedfilename [string_to_regexp $objdir/savetrace.tr]
 remote_file host delete $objdir/savetrace.tr
 gdb_test "save-tracepoints $objdir/savetrace.tr" \
-	"Tracepoints saved to file '${escapedfilename}'." \
+	"Saved to file '${escapedfilename}'." \
 	"10.3: save tracepoint definitions, full path"
 
 gdb_delete_tracepoints


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: PR8554: New command to save breakpoints to a file
  2010-04-12 18:15     ` Pedro Alves
@ 2010-04-15 17:52       ` Michael Snyder
  2010-04-15 18:27         ` Pedro Alves
  0 siblings, 1 reply; 23+ messages in thread
From: Michael Snyder @ 2010-04-15 17:52 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, tromey

Pedro Alves wrote:
> On Friday 09 April 2010 18:23:31, Pedro Alves wrote:
>> On Friday 09 April 2010 17:17:31, Tom Tromey wrote:
>>>>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:
>>> Pedro> (I finished this instead of dumping it.)
>>>
>>> Thanks.  I do like this approach.
>>>
>>> Pedro> Add a new save-breakpoints command to save breakpoint definitions
>>> Pedro> to a file.
>>>
>>> I'd personally prefer "save breakpoints", with a space, and make
>>> save-tracepoints a deprecated alias for "save tracepoints".  What do you
>>> (and others) think of this?  I tend to like simple commands with spaces,
>>> especially when a subcommand comes along.
>> Fine with me.
> 
> Done now.

What about watchpoints?  Do we get all the context info right?


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: PR8554: New command to save breakpoints to a file
  2010-04-15 17:52       ` Michael Snyder
@ 2010-04-15 18:27         ` Pedro Alves
  2010-04-15 18:46           ` Michael Snyder
  2010-04-15 18:59           ` Stan Shebs
  0 siblings, 2 replies; 23+ messages in thread
From: Pedro Alves @ 2010-04-15 18:27 UTC (permalink / raw)
  To: Michael Snyder; +Cc: gdb-patches, tromey

On Thursday 15 April 2010 18:51:55, Michael Snyder wrote:
> What about watchpoints?  Do we get all the context info right?

Watchpoints on globals, as right as breakpoints.  Watchpoints on
locals, no, you're just out of luck.  I don't think there's
much to do there; GDB gets rid of those on process start/exit,
so users are used to those not being very "persistanteable".
OTOH, it may be useful to be able to dump watchpoints on locals,
and be able to load them up when you know it's okay, so never
dumping those isn't that great either.  So, IMO, we shouldn't
worry much about those, at least, in this first patch.  :-)  I
could add a note to the manual, perhaps.

-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: PR8554: New command to save breakpoints to a file
  2010-04-15 18:27         ` Pedro Alves
@ 2010-04-15 18:46           ` Michael Snyder
  2010-04-15 18:53             ` Pedro Alves
  2010-04-15 18:59           ` Stan Shebs
  1 sibling, 1 reply; 23+ messages in thread
From: Michael Snyder @ 2010-04-15 18:46 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, tromey

Pedro Alves wrote:
> On Thursday 15 April 2010 18:51:55, Michael Snyder wrote:
>> What about watchpoints?  Do we get all the context info right?
> 
> Watchpoints on globals, as right as breakpoints.  Watchpoints on
> locals, no, you're just out of luck.  I don't think there's
> much to do there; GDB gets rid of those on process start/exit,
> so users are used to those not being very "persistanteable".
> OTOH, it may be useful to be able to dump watchpoints on locals,
> and be able to load them up when you know it's okay, so never
> dumping those isn't that great either.  So, IMO, we shouldn't
> worry much about those, at least, in this first patch.  :-)  I
> could add a note to the manual, perhaps.

That's cool.  So what do we do now?  Just skip them?
Save the global ones, skip the local ones?


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: PR8554: New command to save breakpoints to a file
  2010-04-15 18:46           ` Michael Snyder
@ 2010-04-15 18:53             ` Pedro Alves
  2010-04-15 18:58               ` Michael Snyder
  0 siblings, 1 reply; 23+ messages in thread
From: Pedro Alves @ 2010-04-15 18:53 UTC (permalink / raw)
  To: Michael Snyder; +Cc: gdb-patches, tromey

On Thursday 15 April 2010 19:46:15, Michael Snyder wrote:
> Pedro Alves wrote:
> > On Thursday 15 April 2010 18:51:55, Michael Snyder wrote:
> >> What about watchpoints?  Do we get all the context info right?
> > 
> > Watchpoints on globals, as right as breakpoints.  Watchpoints on
> > locals, no, you're just out of luck.  I don't think there's
> > much to do there; GDB gets rid of those on process start/exit,
> > so users are used to those not being very "persistanteable".
> > OTOH, it may be useful to be able to dump watchpoints on locals,
> > and be able to load them up when you know it's okay, so never
> > dumping those isn't that great either.  So, IMO, we shouldn't
> > worry much about those, at least, in this first patch.  :-)  I
> > could add a note to the manual, perhaps.
> 
> That's cool.  So what do we do now?  Just skip them?
> Save the global ones, skip the local ones?

We save them.

-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: PR8554: New command to save breakpoints to a file
  2010-04-15 18:53             ` Pedro Alves
@ 2010-04-15 18:58               ` Michael Snyder
  2010-04-15 19:58                 ` Pedro Alves
  0 siblings, 1 reply; 23+ messages in thread
From: Michael Snyder @ 2010-04-15 18:58 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, tromey

Pedro Alves wrote:
> On Thursday 15 April 2010 19:46:15, Michael Snyder wrote:
>> Pedro Alves wrote:
>>> On Thursday 15 April 2010 18:51:55, Michael Snyder wrote:
>>>> What about watchpoints?  Do we get all the context info right?
>>> Watchpoints on globals, as right as breakpoints.  Watchpoints on
>>> locals, no, you're just out of luck.  I don't think there's
>>> much to do there; GDB gets rid of those on process start/exit,
>>> so users are used to those not being very "persistanteable".
>>> OTOH, it may be useful to be able to dump watchpoints on locals,
>>> and be able to load them up when you know it's okay, so never
>>> dumping those isn't that great either.  So, IMO, we shouldn't
>>> worry much about those, at least, in this first patch.  :-)  I
>>> could add a note to the manual, perhaps.
>> That's cool.  So what do we do now?  Just skip them?
>> Save the global ones, skip the local ones?
> 
> We save them.
> 

Oh -- and on reload, some of them fail?

Shouldn't be difficult to skip saving all of them, but what about
skipping the locals and saving the globals?  Hard?


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: PR8554: New command to save breakpoints to a file
  2010-04-15 18:27         ` Pedro Alves
  2010-04-15 18:46           ` Michael Snyder
@ 2010-04-15 18:59           ` Stan Shebs
  2010-04-15 19:10             ` Pedro Alves
  1 sibling, 1 reply; 23+ messages in thread
From: Stan Shebs @ 2010-04-15 18:59 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Michael Snyder, gdb-patches, tromey

Pedro Alves wrote:
> On Thursday 15 April 2010 18:51:55, Michael Snyder wrote:
>   
>> What about watchpoints?  Do we get all the context info right?
>>     
>
> Watchpoints on globals, as right as breakpoints.  Watchpoints on
> locals, no, you're just out of luck.  I don't think there's
> much to do there; GDB gets rid of those on process start/exit,
> so users are used to those not being very "persistanteable".
> OTOH, it may be useful to be able to dump watchpoints on locals,
> and be able to load them up when you know it's okay, so never
> dumping those isn't that great either.  So, IMO, we shouldn't
> worry much about those, at least, in this first patch.  :-)  I
> could add a note to the manual, perhaps.
>
>   

I can't remember if we've discussed this idea before, but a simple way 
to handle in the future might be simply to introduce a syntax for 
referring to specific locals - function:::local, file:line:::local, for 
instance.  You get a sal that maps to a block, then can find the block's 
symbol with that name.  If the name also occurs in superblocks, you 
could make a policy decision as to whether to include the first found, 
or all found.

Stan


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: PR8554: New command to save breakpoints to a file
  2010-04-15 18:59           ` Stan Shebs
@ 2010-04-15 19:10             ` Pedro Alves
  0 siblings, 0 replies; 23+ messages in thread
From: Pedro Alves @ 2010-04-15 19:10 UTC (permalink / raw)
  To: Stan Shebs; +Cc: Michael Snyder, gdb-patches, tromey

On Thursday 15 April 2010 19:59:17, Stan Shebs wrote:
> I can't remember if we've discussed this idea before, but a simple way 
> to handle in the future might be simply to introduce a syntax for 
> referring to specific locals - function:::local, file:line:::local, for 
> instance.  You get a sal that maps to a block, then can find the block's 
> symbol with that name.  If the name also occurs in superblocks, you 
> could make a policy decision as to whether to include the first found, 
> or all found.

Dunno.  Maybe we've read it over in HPDF's parallel debugger's "standard",
or a PR or something.  Maybe Frysk implemented it?  It's certainly not
a novel idea.  ;-)

Anyway, that handles the symbol scoping, not the fact that to set
a watchpoint, the frame where the local is defined needs to be 
active, otherwise, you can't read the initial value for
comparision and change detection.  Certainly more would have
to be changed to make it work.

I certainly think that the feature is quite useful even
with the locals caveat.

-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: PR8554: New command to save breakpoints to a file
  2010-04-15 18:58               ` Michael Snyder
@ 2010-04-15 19:58                 ` Pedro Alves
  2010-04-15 22:49                   ` Michael Snyder
  0 siblings, 1 reply; 23+ messages in thread
From: Pedro Alves @ 2010-04-15 19:58 UTC (permalink / raw)
  To: gdb-patches; +Cc: Michael Snyder, tromey

On Thursday 15 April 2010 19:58:44, Michael Snyder wrote:
> >>> OTOH, it may be useful to be able to dump watchpoints on locals,
> >>> and be able to load them up when you know it's okay, so never
> >>> dumping those isn't that great either.  So, IMO, we shouldn't
> >>> worry much about those, at least, in this first patch.  :-)  I
> >>> could add a note to the manual, perhaps.
> >> That's cool.  So what do we do now?  Just skip them?
> >> Save the global ones, skip the local ones?
> > 
> > We save them.
> > 
> 
> Oh -- and on reload, some of them fail?
> 
> Shouldn't be difficult to skip saving all of them, but what about
> skipping the locals and saving the globals?  Hard?

As I said above, it may be useful to be able to dump watchpoints
on locals, and be able to load them up when you know it's okay,
so never dumping those isn't that great either.  It's not hard,
it's just not always the right thing.  As is, the user can edit the
script if the wants to zap some breakpoints before sourcing it.  It's
just a CLI script.  If you want to add a new command switch to
tune the behaviour, that'd be cool.

-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: PR8554: New command to save breakpoints to a file
  2010-04-15 19:58                 ` Pedro Alves
@ 2010-04-15 22:49                   ` Michael Snyder
  2010-04-15 22:52                     ` Michael Snyder
  2010-04-15 23:01                     ` Pedro Alves
  0 siblings, 2 replies; 23+ messages in thread
From: Michael Snyder @ 2010-04-15 22:49 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, tromey

Pedro Alves wrote:
> On Thursday 15 April 2010 19:58:44, Michael Snyder wrote:
>>>>> OTOH, it may be useful to be able to dump watchpoints on locals,
>>>>> and be able to load them up when you know it's okay, so never
>>>>> dumping those isn't that great either.  So, IMO, we shouldn't
>>>>> worry much about those, at least, in this first patch.  :-)  I
>>>>> could add a note to the manual, perhaps.
>>>> That's cool.  So what do we do now?  Just skip them?
>>>> Save the global ones, skip the local ones?
>>> We save them.
>>>
>> Oh -- and on reload, some of them fail?
>>
>> Shouldn't be difficult to skip saving all of them, but what about
>> skipping the locals and saving the globals?  Hard?
> 
> As I said above, it may be useful to be able to dump watchpoints
> on locals, and be able to load them up when you know it's okay,
> so never dumping those isn't that great either.  It's not hard,
> it's just not always the right thing.  As is, the user can edit the
> script if the wants to zap some breakpoints before sourcing it.  It's
> just a CLI script.  If you want to add a new command switch to
> tune the behaviour, that'd be cool.
> 

Fair enough.

BTW. "save_command" needs to print an error or something, not
simply return without doing anything.  If I type "save<return>",
I get a silent failure.



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: PR8554: New command to save breakpoints to a file
  2010-04-15 22:49                   ` Michael Snyder
@ 2010-04-15 22:52                     ` Michael Snyder
  2010-04-15 22:58                       ` Michael Snyder
  2010-04-15 23:01                     ` Pedro Alves
  1 sibling, 1 reply; 23+ messages in thread
From: Michael Snyder @ 2010-04-15 22:52 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, tromey

Michael Snyder wrote:
> Pedro Alves wrote:
>> On Thursday 15 April 2010 19:58:44, Michael Snyder wrote:
>>>>>> OTOH, it may be useful to be able to dump watchpoints on locals,
>>>>>> and be able to load them up when you know it's okay, so never
>>>>>> dumping those isn't that great either.  So, IMO, we shouldn't
>>>>>> worry much about those, at least, in this first patch.  :-)  I
>>>>>> could add a note to the manual, perhaps.
>>>>> That's cool.  So what do we do now?  Just skip them?
>>>>> Save the global ones, skip the local ones?
>>>> We save them.
>>>>
>>> Oh -- and on reload, some of them fail?
>>>
>>> Shouldn't be difficult to skip saving all of them, but what about
>>> skipping the locals and saving the globals?  Hard?
>> As I said above, it may be useful to be able to dump watchpoints
>> on locals, and be able to load them up when you know it's okay,
>> so never dumping those isn't that great either.  It's not hard,
>> it's just not always the right thing.  As is, the user can edit the
>> script if the wants to zap some breakpoints before sourcing it.  It's
>> just a CLI script.  If you want to add a new command switch to
>> tune the behaviour, that'd be cool.
>>
> 
> Fair enough.
> 
> BTW. "save_command" needs to print an error or something, not
> simply return without doing anything.  If I type "save<return>",
> I get a silent failure.
> 
> 

... or perhaps "save" should be aliased to "save-tracepoints",
to mimic the previous behavior.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: PR8554: New command to save breakpoints to a file
  2010-04-15 22:52                     ` Michael Snyder
@ 2010-04-15 22:58                       ` Michael Snyder
  2010-04-15 23:05                         ` Pedro Alves
  2010-04-15 23:14                         ` Michael Snyder
  0 siblings, 2 replies; 23+ messages in thread
From: Michael Snyder @ 2010-04-15 22:58 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, tromey

Michael Snyder wrote:
> Michael Snyder wrote:
>> Pedro Alves wrote:
>>> On Thursday 15 April 2010 19:58:44, Michael Snyder wrote:
>>>>>>> OTOH, it may be useful to be able to dump watchpoints on locals,
>>>>>>> and be able to load them up when you know it's okay, so never
>>>>>>> dumping those isn't that great either.  So, IMO, we shouldn't
>>>>>>> worry much about those, at least, in this first patch.  :-)  I
>>>>>>> could add a note to the manual, perhaps.
>>>>>> That's cool.  So what do we do now?  Just skip them?
>>>>>> Save the global ones, skip the local ones?
>>>>> We save them.
>>>>>
>>>> Oh -- and on reload, some of them fail?
>>>>
>>>> Shouldn't be difficult to skip saving all of them, but what about
>>>> skipping the locals and saving the globals?  Hard?
>>> As I said above, it may be useful to be able to dump watchpoints
>>> on locals, and be able to load them up when you know it's okay,
>>> so never dumping those isn't that great either.  It's not hard,
>>> it's just not always the right thing.  As is, the user can edit the
>>> script if the wants to zap some breakpoints before sourcing it.  It's
>>> just a CLI script.  If you want to add a new command switch to
>>> tune the behaviour, that'd be cool.
>>>
>> Fair enough.
>>
>> BTW. "save_command" needs to print an error or something, not
>> simply return without doing anything.  If I type "save<return>",
>> I get a silent failure.
>>
>>
> 
> ... or perhaps "save" should be aliased to "save-tracepoints",
> to mimic the previous behavior.

Also maybe this should be milder than a warning:

   if (!any)
     {
       if (from_tty)
         printf_filtered (_("Nothing to save."));
       return;
     }


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: PR8554: New command to save breakpoints to a file
  2010-04-15 22:49                   ` Michael Snyder
  2010-04-15 22:52                     ` Michael Snyder
@ 2010-04-15 23:01                     ` Pedro Alves
  1 sibling, 0 replies; 23+ messages in thread
From: Pedro Alves @ 2010-04-15 23:01 UTC (permalink / raw)
  To: Michael Snyder; +Cc: gdb-patches, tromey

On Thursday 15 April 2010 23:49:32, Michael Snyder wrote:
> BTW. "save_command" needs to print an error or something, not
> simply return without doing anything.  If I type "save<return>",
> I get a silent failure.

I think the common practice is to have it print "help save".
At least, there are over 30 calls to help_list under gdb/
that look like that.  E.g.,

(gdb) set print
"set print" must be followed by the name of a print subcommand.
List of set print subcommands:

set print address -- Set printing of addresses
...

I'll do that.


I don't think we should bother with aliasing save to 
save-tracepoints, and it may even be annoying at some
point.

-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: PR8554: New command to save breakpoints to a file
  2010-04-15 22:58                       ` Michael Snyder
@ 2010-04-15 23:05                         ` Pedro Alves
  2010-04-15 23:15                           ` Michael Snyder
  2010-04-15 23:14                         ` Michael Snyder
  1 sibling, 1 reply; 23+ messages in thread
From: Pedro Alves @ 2010-04-15 23:05 UTC (permalink / raw)
  To: Michael Snyder; +Cc: gdb-patches, tromey

On Thursday 15 April 2010 23:58:23, Michael Snyder wrote:
> Also maybe this should be milder than a warning:
> 
>    if (!any)
>      {
>        if (from_tty)
>          printf_filtered (_("Nothing to save."));
>        return;
>      }

(sorry, lost in translation)  What do you mean by
milder?  Get rid of the print at all?

That is what the current implementation of "save-tracepoints"
does, I didn't want to change the whole world with a single
patch.

-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: PR8554: New command to save breakpoints to a file
  2010-04-15 22:58                       ` Michael Snyder
  2010-04-15 23:05                         ` Pedro Alves
@ 2010-04-15 23:14                         ` Michael Snyder
  2010-04-15 23:20                           ` Pedro Alves
  1 sibling, 1 reply; 23+ messages in thread
From: Michael Snyder @ 2010-04-15 23:14 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, tromey

Michael Snyder wrote:
> Michael Snyder wrote:
>> Michael Snyder wrote:
>>> Pedro Alves wrote:
>>>> On Thursday 15 April 2010 19:58:44, Michael Snyder wrote:
>>>>>>>> OTOH, it may be useful to be able to dump watchpoints on locals,
>>>>>>>> and be able to load them up when you know it's okay, so never
>>>>>>>> dumping those isn't that great either.  So, IMO, we shouldn't
>>>>>>>> worry much about those, at least, in this first patch.  :-)  I
>>>>>>>> could add a note to the manual, perhaps.
>>>>>>> That's cool.  So what do we do now?  Just skip them?
>>>>>>> Save the global ones, skip the local ones?
>>>>>> We save them.
>>>>>>
>>>>> Oh -- and on reload, some of them fail?
>>>>>
>>>>> Shouldn't be difficult to skip saving all of them, but what about
>>>>> skipping the locals and saving the globals?  Hard?
>>>> As I said above, it may be useful to be able to dump watchpoints
>>>> on locals, and be able to load them up when you know it's okay,
>>>> so never dumping those isn't that great either.  It's not hard,
>>>> it's just not always the right thing.  As is, the user can edit the
>>>> script if the wants to zap some breakpoints before sourcing it.  It's
>>>> just a CLI script.  If you want to add a new command switch to
>>>> tune the behaviour, that'd be cool.
>>>>
>>> Fair enough.
>>>
>>> BTW. "save_command" needs to print an error or something, not
>>> simply return without doing anything.  If I type "save<return>",
>>> I get a silent failure.
>>>
>>>
>> ... or perhaps "save" should be aliased to "save-tracepoints",
>> to mimic the previous behavior.
> 
> Also maybe this should be milder than a warning:
> 
>    if (!any)
>      {
>        if (from_tty)
>          printf_filtered (_("Nothing to save."));
>        return;
>      }
> 

Also, it doesn't seem right to me that "save tracepoints" will ignore
breakpoints, but "save breakpoints" doesn't ignore tracepoints (but
rather saves them both).

Since you have this filter mechanism in place already, why don't you
arrange to have "save breakpoints" save only breakpoints?




^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: PR8554: New command to save breakpoints to a file
  2010-04-15 23:05                         ` Pedro Alves
@ 2010-04-15 23:15                           ` Michael Snyder
  2010-04-15 23:25                             ` Pedro Alves
  0 siblings, 1 reply; 23+ messages in thread
From: Michael Snyder @ 2010-04-15 23:15 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, tromey

Pedro Alves wrote:
> On Thursday 15 April 2010 23:58:23, Michael Snyder wrote:
>> Also maybe this should be milder than a warning:
>>
>>    if (!any)
>>      {
>>        if (from_tty)
>>          printf_filtered (_("Nothing to save."));
>>        return;
>>      }
> 
> (sorry, lost in translation)  What do you mean by
> milder?  Get rid of the print at all?
> 
> That is what the current implementation of "save-tracepoints"
> does, I didn't want to change the whole world with a single
> patch.
> 

No, the current implementation calls "warning", with the same
message.  So it comes out at the console as:

   Warning: Nothing to save.


Just sounds a little draconian, that's all.   ;-)


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: PR8554: New command to save breakpoints to a file
  2010-04-15 23:14                         ` Michael Snyder
@ 2010-04-15 23:20                           ` Pedro Alves
  0 siblings, 0 replies; 23+ messages in thread
From: Pedro Alves @ 2010-04-15 23:20 UTC (permalink / raw)
  To: Michael Snyder; +Cc: gdb-patches, tromey

On Friday 16 April 2010 00:14:11, Michael Snyder wrote:
> Also, it doesn't seem right to me that "save tracepoints" will ignore
> breakpoints, but "save breakpoints" doesn't ignore tracepoints (but
> rather saves them both).
> 
> Since you have this filter mechanism in place already, why don't you
> arrange to have "save breakpoints" save only breakpoints?

Because "info breakpoints" also lists all kinds of breakpoints
(real-breakpoints, watchpoints, tracepoints, catchpoints), and
"info tracepoints" only lists tracepoints.  Please refer to
last January's discution about "actionpoints" over at gdb@.

-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: PR8554: New command to save breakpoints to a file
  2010-04-15 23:15                           ` Michael Snyder
@ 2010-04-15 23:25                             ` Pedro Alves
  2010-05-05  8:28                               ` Hui Zhu
  0 siblings, 1 reply; 23+ messages in thread
From: Pedro Alves @ 2010-04-15 23:25 UTC (permalink / raw)
  To: Michael Snyder; +Cc: gdb-patches, tromey

On Friday 16 April 2010 00:15:42, Michael Snyder wrote:
> No, the current implementation calls "warning", with the same
> message.  So it comes out at the console as:
> 
>    Warning: Nothing to save.
> 

I meant, "warning" is what the current code already does:

-  if (!any_tp)
+
+  if (!any)
     {
-      warning (_("save-tracepoints: no tracepoints to save."));
+      warning (_("Nothing to save."));
       return;
     }

> Just sounds a little draconian, that's all.   ;-)

It's actually correct to be a warning.  If there's nothing
to save, the command does not overwrite a previous file,
so a follow up "source" may read a stale breakpoint|tracepoint
list.

When writing the patch I had considered that this could
be seen as a bug, and, hence we should either `error' out,
or, proceed and write an empty file.  But as I said, I didn't
want to change the whole world with a single patch, so I
left that part out of the discussion on purpose...

-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: PR8554: New command to save breakpoints to a file
  2010-04-15 23:25                             ` Pedro Alves
@ 2010-05-05  8:28                               ` Hui Zhu
  0 siblings, 0 replies; 23+ messages in thread
From: Hui Zhu @ 2010-05-05  8:28 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Michael Snyder, gdb-patches, tromey

Hi,

I think this idea is very cool.
Now, more and more function have a special switch to set a lot of
things.  So setup them to fit every time will very hard thing.

I suggest we design a new frame "save".
That every feature can add item to it.
The customer just use "save" or "save break" to set all of the items
or just break items.

Thanks,
Hui

On Fri, Apr 16, 2010 at 07:25, Pedro Alves <pedro@codesourcery.com> wrote:
> On Friday 16 April 2010 00:15:42, Michael Snyder wrote:
>> No, the current implementation calls "warning", with the same
>> message.  So it comes out at the console as:
>>
>>    Warning: Nothing to save.
>>
>
> I meant, "warning" is what the current code already does:
>
> -  if (!any_tp)
> +
> +  if (!any)
>     {
> -      warning (_("save-tracepoints: no tracepoints to save."));
> +      warning (_("Nothing to save."));
>       return;
>     }
>
>> Just sounds a little draconian, that's all.   ;-)
>
> It's actually correct to be a warning.  If there's nothing
> to save, the command does not overwrite a previous file,
> so a follow up "source" may read a stale breakpoint|tracepoint
> list.
>
> When writing the patch I had considered that this could
> be seen as a bug, and, hence we should either `error' out,
> or, proceed and write an empty file.  But as I said, I didn't
> want to change the whole world with a single patch, so I
> left that part out of the discussion on purpose...
>
> --
> Pedro Alves
>


^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2010-05-05  8:28 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-04-09  2:41 PR8554: New command to save breakpoints to a file Pedro Alves
2010-04-09 16:17 ` Tom Tromey
2010-04-09 17:23   ` Pedro Alves
2010-04-10  4:10     ` Sergio Durigan Junior
2010-04-12 18:15     ` Pedro Alves
2010-04-15 17:52       ` Michael Snyder
2010-04-15 18:27         ` Pedro Alves
2010-04-15 18:46           ` Michael Snyder
2010-04-15 18:53             ` Pedro Alves
2010-04-15 18:58               ` Michael Snyder
2010-04-15 19:58                 ` Pedro Alves
2010-04-15 22:49                   ` Michael Snyder
2010-04-15 22:52                     ` Michael Snyder
2010-04-15 22:58                       ` Michael Snyder
2010-04-15 23:05                         ` Pedro Alves
2010-04-15 23:15                           ` Michael Snyder
2010-04-15 23:25                             ` Pedro Alves
2010-05-05  8:28                               ` Hui Zhu
2010-04-15 23:14                         ` Michael Snyder
2010-04-15 23:20                           ` Pedro Alves
2010-04-15 23:01                     ` Pedro Alves
2010-04-15 18:59           ` Stan Shebs
2010-04-15 19:10             ` Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox