public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [RFA v2 11/17] Use scoped_restore in more places
  2017-04-11 15:02 [RFA v2 00/17] miscellaneous C++-ification Tom Tromey
                   ` (3 preceding siblings ...)
  2017-04-11 15:01 ` [RFA v2 16/17] Add a constructor and destructor to linespec_result Tom Tromey
@ 2017-04-11 15:01 ` Tom Tromey
  2017-04-11 15:01 ` [RFA v2 05/17] Change increment_reading_symtab to return a scoped_restore Tom Tromey
                   ` (12 subsequent siblings)
  17 siblings, 0 replies; 36+ messages in thread
From: Tom Tromey @ 2017-04-11 15:01 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes a few more places to use scoped_restore, allowing some
cleanup removals.

gdb/ChangeLog
2017-04-11  Tom Tromey  <tom@tromey.com>

	* mi/mi-main.c (exec_direction_forward): Remove.
	(exec_reverse_continue, mi_execute_command): Use scoped_restore.
	* guile/scm-ports.c (ioscm_with_output_to_port_worker): Use
	scoped_restore.
	* guile/guile.c (guile_repl_command, guile_command)
	(gdbscm_execute_gdb_command): Use scoped_restore.
	* go-exp.y (go_parse): Use scoped_restore.
	* d-exp.y (d_parse): Use scoped_restore.
	* cli/cli-decode.c (cmd_func): Use scoped_restore.
	* c-exp.y (c_parse): Use scoped_restore.
---
 gdb/ChangeLog         | 13 +++++++++++++
 gdb/c-exp.y           |  4 ++--
 gdb/cli/cli-decode.c  | 10 +++-------
 gdb/d-exp.y           |  4 ++--
 gdb/go-exp.y          |  4 ++--
 gdb/guile/guile.c     | 24 ++++++------------------
 gdb/guile/scm-ports.c |  3 +--
 gdb/mi/mi-main.c      | 25 +++++++------------------
 8 files changed, 36 insertions(+), 51 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index a6febff..f582694 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,18 @@
 2017-04-11  Tom Tromey  <tom@tromey.com>
 
+	* mi/mi-main.c (exec_direction_forward): Remove.
+	(exec_reverse_continue, mi_execute_command): Use scoped_restore.
+	* guile/scm-ports.c (ioscm_with_output_to_port_worker): Use
+	scoped_restore.
+	* guile/guile.c (guile_repl_command, guile_command)
+	(gdbscm_execute_gdb_command): Use scoped_restore.
+	* go-exp.y (go_parse): Use scoped_restore.
+	* d-exp.y (d_parse): Use scoped_restore.
+	* cli/cli-decode.c (cmd_func): Use scoped_restore.
+	* c-exp.y (c_parse): Use scoped_restore.
+
+2017-04-11  Tom Tromey  <tom@tromey.com>
+
 	* mi/mi-parse.h (struct mi_parse): Add constructor, destructor.
 	(mi_parse): Update return type..
 	(mi_parse_free): Remove.
diff --git a/gdb/c-exp.y b/gdb/c-exp.y
index b2fc195..283b737 100644
--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -3188,8 +3188,8 @@ c_parse (struct parser_state *par_state)
   gdb_assert (! macro_original_text);
   make_cleanup (scan_macro_cleanup, 0);
 
-  make_cleanup_restore_integer (&yydebug);
-  yydebug = parser_debug;
+  scoped_restore restore_yydebug = make_scoped_restore (&yydebug,
+							parser_debug);
 
   /* Initialize some state used by the lexer.  */
   last_was_structop = 0;
diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index fc14465..d45733e 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -23,6 +23,7 @@
 #include "ui-out.h"
 #include "cli/cli-cmds.h"
 #include "cli/cli-decode.h"
+#include "common/gdb_optional.h"
 
 /* Prototypes for local functions.  */
 
@@ -1878,17 +1879,12 @@ cmd_func (struct cmd_list_element *cmd, char *args, int from_tty)
 {
   if (cmd_func_p (cmd))
     {
-      struct cleanup *cleanups = make_cleanup (null_cleanup, NULL);
+      gdb::optional<scoped_restore_tmpl<int>> restore_suppress;
 
       if (cmd->suppress_notification != NULL)
-	{
-	  make_cleanup_restore_integer (cmd->suppress_notification);
-	  *cmd->suppress_notification = 1;
-	}
+	restore_suppress.emplace (cmd->suppress_notification, 1);
 
       (*cmd->func) (cmd, args, from_tty);
-
-      do_cleanups (cleanups);
     }
   else
     error (_("Invalid command"));
diff --git a/gdb/d-exp.y b/gdb/d-exp.y
index 06eef5f..62df737 100644
--- a/gdb/d-exp.y
+++ b/gdb/d-exp.y
@@ -1629,9 +1629,9 @@ d_parse (struct parser_state *par_state)
 
   back_to = make_cleanup (null_cleanup, NULL);
 
-  make_cleanup_restore_integer (&yydebug);
+  scoped_restore restore_yydebug = make_scoped_restore (&yydebug,
+							parser_debug);
   make_cleanup_clear_parser_state (&pstate);
-  yydebug = parser_debug;
 
   /* Initialize some state used by the lexer.  */
   last_was_structop = 0;
diff --git a/gdb/go-exp.y b/gdb/go-exp.y
index 1906e68..057e227 100644
--- a/gdb/go-exp.y
+++ b/gdb/go-exp.y
@@ -1569,9 +1569,9 @@ go_parse (struct parser_state *par_state)
 
   back_to = make_cleanup (null_cleanup, NULL);
 
-  make_cleanup_restore_integer (&yydebug);
+  scoped_restore restore_yydebug = make_scoped_restore (&yydebug,
+							parser_debug);
   make_cleanup_clear_parser_state (&pstate);
-  yydebug = parser_debug;
 
   /* Initialize some state used by the lexer.  */
   last_was_structop = 0;
diff --git a/gdb/guile/guile.c b/gdb/guile/guile.c
index 9bb2487..0dadc3c 100644
--- a/gdb/guile/guile.c
+++ b/gdb/guile/guile.c
@@ -163,10 +163,7 @@ const struct extension_language_ops guile_extension_ops =
 static void
 guile_repl_command (char *arg, int from_tty)
 {
-  struct cleanup *cleanup;
-
-  cleanup = make_cleanup_restore_integer (&current_ui->async);
-  current_ui->async = 0;
+  scoped_restore restore_async = make_scoped_restore (&current_ui->async, 0);
 
   arg = skip_spaces (arg);
 
@@ -183,8 +180,6 @@ guile_repl_command (char *arg, int from_tty)
       dont_repeat ();
       gdbscm_enter_repl ();
     }
-
-  do_cleanups (cleanup);
 }
 
 /* Implementation of the gdb "guile" command.
@@ -196,10 +191,7 @@ guile_repl_command (char *arg, int from_tty)
 static void
 guile_command (char *arg, int from_tty)
 {
-  struct cleanup *cleanup;
-
-  cleanup = make_cleanup_restore_integer (&current_ui->async);
-  current_ui->async = 0;
+  scoped_restore restore_async = make_scoped_restore (&current_ui->async, 0);
 
   arg = skip_spaces (arg);
 
@@ -209,6 +201,8 @@ guile_command (char *arg, int from_tty)
 
       if (msg != NULL)
 	{
+	  /* It is ok that this is a "dangling cleanup" because we
+	     throw immediately.  */
 	  make_cleanup (xfree, msg);
 	  error ("%s", msg);
 	}
@@ -219,8 +213,6 @@ guile_command (char *arg, int from_tty)
 
       execute_control_command_untraced (l.get ());
     }
-
-  do_cleanups (cleanup);
 }
 
 /* Given a command_line, return a command string suitable for passing
@@ -326,10 +318,8 @@ gdbscm_execute_gdb_command (SCM command_scm, SCM rest)
 
   TRY
     {
-      struct cleanup *inner_cleanups;
-
-      inner_cleanups = make_cleanup_restore_integer (&current_ui->async);
-      current_ui->async = 0;
+      scoped_restore restore_async = make_scoped_restore (&current_ui->async,
+							  0);
 
       scoped_restore preventer = prevent_dont_repeat ();
       if (to_string)
@@ -339,8 +329,6 @@ gdbscm_execute_gdb_command (SCM command_scm, SCM rest)
 
       /* Do any commands attached to breakpoint we stopped at.  */
       bpstat_do_actions ();
-
-      do_cleanups (inner_cleanups);
     }
   CATCH (ex, RETURN_MASK_ALL)
     {
diff --git a/gdb/guile/scm-ports.c b/gdb/guile/scm-ports.c
index fb3a47b..735abc2 100644
--- a/gdb/guile/scm-ports.c
+++ b/gdb/guile/scm-ports.c
@@ -470,8 +470,7 @@ ioscm_with_output_to_port_worker (SCM port, SCM thunk, enum oport oport,
 
   cleanups = set_batch_flag_and_make_cleanup_restore_page_info ();
 
-  make_cleanup_restore_integer (&current_ui->async);
-  current_ui->async = 0;
+  scoped_restore restore_async = make_scoped_restore (&current_ui->async, 0);
 
   ui_file_up port_file (new ioscm_file_port (port));
 
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index d99c40e..c3e7bf7 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -54,6 +54,7 @@
 #include "extension.h"
 #include "gdbcmd.h"
 #include "observer.h"
+#include "common/gdb_optional.h"
 
 #include <ctype.h>
 #include "run-time-clock.h"
@@ -312,16 +313,9 @@ exec_continue (char **argv, int argc)
 }
 
 static void
-exec_direction_forward (void *notused)
-{
-  execution_direction = EXEC_FORWARD;
-}
-
-static void
 exec_reverse_continue (char **argv, int argc)
 {
   enum exec_direction_kind dir = execution_direction;
-  struct cleanup *old_chain;
 
   if (dir == EXEC_REVERSE)
     error (_("Already in reverse mode."));
@@ -329,10 +323,9 @@ exec_reverse_continue (char **argv, int argc)
   if (!target_can_execute_reverse)
     error (_("Target %s does not support this command."), target_shortname);
 
-  old_chain = make_cleanup (exec_direction_forward, NULL);
-  execution_direction = EXEC_REVERSE;
+  scoped_restore save_exec_dir = make_scoped_restore (&execution_direction,
+						      EXEC_REVERSE);
   exec_continue (argv, argc);
-  do_cleanups (old_chain);
 }
 
 void
@@ -2140,15 +2133,13 @@ mi_execute_command (const char *cmd, int from_tty)
   if (command != NULL)
     {
       ptid_t previous_ptid = inferior_ptid;
-      struct cleanup *cleanup = make_cleanup (null_cleanup, NULL);
 
-      command->token = token;
+      gdb::optional<scoped_restore_tmpl<int>> restore_suppress;
 
       if (command->cmd != NULL && command->cmd->suppress_notification != NULL)
-        {
-          make_cleanup_restore_integer (command->cmd->suppress_notification);
-          *command->cmd->suppress_notification = 1;
-        }
+	restore_suppress.emplace (command->cmd->suppress_notification, 1);
+
+      command->token = token;
 
       if (do_timings)
 	{
@@ -2210,8 +2201,6 @@ mi_execute_command (const char *cmd, int from_tty)
 		  (USER_SELECTED_THREAD | USER_SELECTED_FRAME);
 	    }
 	}
-
-      do_cleanups (cleanup);
     }
 }
 
-- 
2.9.3

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

* [RFA v2 10/17] C++ify mi_parse
  2017-04-11 15:02 [RFA v2 00/17] miscellaneous C++-ification Tom Tromey
  2017-04-11 15:01 ` [RFA v2 09/17] Remove some cleanups from location.c Tom Tromey
@ 2017-04-11 15:01 ` Tom Tromey
  2017-04-12 11:26   ` Pedro Alves
  2017-04-11 15:01 ` [RFA v2 17/17] Change linespec_result::location to be an event_location_up Tom Tromey
                   ` (15 subsequent siblings)
  17 siblings, 1 reply; 36+ messages in thread
From: Tom Tromey @ 2017-04-11 15:01 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes mi_parse to return a unique_ptr, and to use "new"; then
fixes up the users.  This allows removing one cleanup.

gdb/ChangeLog
2017-04-11  Tom Tromey  <tom@tromey.com>

	* mi/mi-parse.h (struct mi_parse): Add constructor, destructor.
	(mi_parse): Update return type..
	(mi_parse_free): Remove.
	* mi/mi-parse.c (mi_parse::~mi_parse): Rename from mi_parse_free.
	(mi_parse_cleanup): Remove.
	(mi_parse): Return a unique_ptr.  Use new.
	* mi/mi-main.c (mi_execute_command): Update.
---
 gdb/ChangeLog     | 10 ++++++++++
 gdb/mi/mi-main.c  |  8 +++-----
 gdb/mi/mi-parse.c | 40 +++++++++-------------------------------
 gdb/mi/mi-parse.h | 14 +++++++++-----
 4 files changed, 31 insertions(+), 41 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index f0c9a7b..a6febff 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,15 @@
 2017-04-11  Tom Tromey  <tom@tromey.com>
 
+	* mi/mi-parse.h (struct mi_parse): Add constructor, destructor.
+	(mi_parse): Update return type..
+	(mi_parse_free): Remove.
+	* mi/mi-parse.c (mi_parse::~mi_parse): Rename from mi_parse_free.
+	(mi_parse_cleanup): Remove.
+	(mi_parse): Return a unique_ptr.  Use new.
+	* mi/mi-main.c (mi_execute_command): Update.
+
+2017-04-11  Tom Tromey  <tom@tromey.com>
+
 	* location.c (explicit_location_lex_one): Return a
 	unique_xmalloc_ptr.
 	(string_to_explicit_location): Update.  Remove cleanups.
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 91fe104..d99c40e 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -2117,7 +2117,7 @@ void
 mi_execute_command (const char *cmd, int from_tty)
 {
   char *token;
-  struct mi_parse *command = NULL;
+  std::unique_ptr<struct mi_parse> command;
 
   /* This is to handle EOF (^D). We just quit gdb.  */
   /* FIXME: we should call some API function here.  */
@@ -2158,7 +2158,7 @@ mi_execute_command (const char *cmd, int from_tty)
 
       TRY
 	{
-	  captured_mi_execute_command (current_uiout, command);
+	  captured_mi_execute_command (current_uiout, command.get ());
 	}
       CATCH (result, RETURN_MASK_ALL)
 	{
@@ -2186,7 +2186,7 @@ mi_execute_command (const char *cmd, int from_tty)
 	  && thread_count () != 0
 	  /* If the command already reports the thread change, no need to do it
 	     again.  */
-	  && !command_notifies_uscc_observer (command))
+	  && !command_notifies_uscc_observer (command.get ()))
 	{
 	  struct mi_interp *mi = (struct mi_interp *) top_level_interpreter ();
 	  int report_change = 0;
@@ -2211,8 +2211,6 @@ mi_execute_command (const char *cmd, int from_tty)
 	    }
 	}
 
-      mi_parse_free (command);
-
       do_cleanups (cleanup);
     }
 }
diff --git a/gdb/mi/mi-parse.c b/gdb/mi/mi-parse.c
index 19cbb14..198d5c6 100644
--- a/gdb/mi/mi-parse.c
+++ b/gdb/mi/mi-parse.c
@@ -208,46 +208,28 @@ mi_parse_argv (const char *args, struct mi_parse *parse)
     }
 }
 
-void
-mi_parse_free (struct mi_parse *parse)
+mi_parse::~mi_parse ()
 {
-  if (parse == NULL)
-    return;
-  if (parse->command != NULL)
-    xfree (parse->command);
-  if (parse->token != NULL)
-    xfree (parse->token);
-  if (parse->args != NULL)
-    xfree (parse->args);
-  if (parse->argv != NULL)
-    freeargv (parse->argv);
-  xfree (parse);
+  xfree (command);
+  xfree (token);
+  xfree (args);
+  freeargv (argv);
 }
 
-/* A cleanup that calls mi_parse_free.  */
-
-static void
-mi_parse_cleanup (void *arg)
-{
-  mi_parse_free ((struct mi_parse *) arg);
-}
-
-struct mi_parse *
+std::unique_ptr<struct mi_parse>
 mi_parse (const char *cmd, char **token)
 {
   const char *chp;
-  struct mi_parse *parse = XNEW (struct mi_parse);
   struct cleanup *cleanup;
 
-  memset (parse, 0, sizeof (*parse));
+  std::unique_ptr<struct mi_parse> parse (new struct mi_parse);
+  memset (parse.get (), 0, sizeof (struct mi_parse));
   parse->all = 0;
   parse->thread_group = -1;
   parse->thread = -1;
   parse->frame = -1;
   parse->language = language_unknown;
 
-  cleanup = make_cleanup (mi_parse_cleanup, parse);
-
   /* Before starting, skip leading white space.  */
   cmd = skip_spaces_const (cmd);
 
@@ -265,8 +247,6 @@ mi_parse (const char *cmd, char **token)
       parse->command = xstrdup (chp);
       parse->op = CLI_COMMAND;
 
-      discard_cleanups (cleanup);
-
       return parse;
     }
 
@@ -383,7 +363,7 @@ mi_parse (const char *cmd, char **token)
      list.  */
   if (parse->cmd->argv_func != NULL)
     {
-      mi_parse_argv (chp, parse);
+      mi_parse_argv (chp, parse.get ());
       if (parse->argv == NULL)
 	error (_("Problem parsing arguments: %s %s"), parse->command, chp);
     }
@@ -394,8 +374,6 @@ mi_parse (const char *cmd, char **token)
   if (parse->cmd->cli.cmd != NULL)
     parse->args = xstrdup (chp);
 
-  discard_cleanups (cleanup);
-
   /* Fully parsed, flag as an MI command.  */
   parse->op = MI_COMMAND;
   return parse;
diff --git a/gdb/mi/mi-parse.h b/gdb/mi/mi-parse.h
index b11e5d3..a09e704 100644
--- a/gdb/mi/mi-parse.h
+++ b/gdb/mi/mi-parse.h
@@ -41,6 +41,13 @@ enum mi_command_type
 
 struct mi_parse
   {
+    mi_parse () = default;
+
+    ~mi_parse ();
+
+    mi_parse (const mi_parse &) = delete;
+    mi_parse &operator= (const mi_parse &) = delete;
+
     enum mi_command_type op;
     char *command;
     char *token;
@@ -67,11 +74,8 @@ struct mi_parse
    the TOKEN field of the resultant mi_parse object, to be freed by
    mi_parse_free.  */
 
-extern struct mi_parse *mi_parse (const char *cmd, char **token);
-
-/* Free a command returned by mi_parse_command.  */
-
-extern void mi_parse_free (struct mi_parse *cmd);
+extern std::unique_ptr<struct mi_parse> mi_parse (const char *cmd,
+						  char **token);
 
 /* Parse a string argument into a print_values value.  */
 
-- 
2.9.3

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

* [RFA v2 17/17] Change linespec_result::location to be an event_location_up
  2017-04-11 15:02 [RFA v2 00/17] miscellaneous C++-ification Tom Tromey
  2017-04-11 15:01 ` [RFA v2 09/17] Remove some cleanups from location.c Tom Tromey
  2017-04-11 15:01 ` [RFA v2 10/17] C++ify mi_parse Tom Tromey
@ 2017-04-11 15:01 ` Tom Tromey
  2017-04-12  2:39   ` Simon Marchi
  2017-04-11 15:01 ` [RFA v2 16/17] Add a constructor and destructor to linespec_result Tom Tromey
                   ` (14 subsequent siblings)
  17 siblings, 1 reply; 36+ messages in thread
From: Tom Tromey @ 2017-04-11 15:01 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This is a follow-up to another patch.  It changes
linespec_result::location to be an event_location_up.

gdb/ChangeLog
2017-04-11  Tom Tromey  <tom@tromey.com>

	* probe.c (parse_probes): Update.
	* location.h (delete_event_location): Don't declare.
	(event_location_deleter::operator()): Update.
	* location.c (event_location_deleter::operator()): Rename from
	delete_event_location.
	* linespec.h (linespec_result) <location>: Change type to
	event_location_up.
	* linespec.c (canonicalize_linespec, event_location_to_sals)
	(decode_objc): Update.
	(linespec_result): Don't call delete_event_location.
	* breakpoint.c (create_breakpoints_sal)
	(bkpt_probe_create_sals_from_location)
	(strace_marker_create_sals_from_location): Update.
---
 gdb/ChangeLog    | 16 ++++++++++++++++
 gdb/breakpoint.c | 12 +++++++-----
 gdb/linespec.c   | 10 +++++-----
 gdb/linespec.h   |  6 +++---
 gdb/location.c   |  4 +---
 gdb/location.h   |  9 +--------
 gdb/probe.c      |  2 +-
 7 files changed, 34 insertions(+), 25 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index fc600b1..b842af0 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,21 @@
 2017-04-11  Tom Tromey  <tom@tromey.com>
 
+	* probe.c (parse_probes): Update.
+	* location.h (delete_event_location): Don't declare.
+	(event_location_deleter::operator()): Update.
+	* location.c (event_location_deleter::operator()): Rename from
+	delete_event_location.
+	* linespec.h (linespec_result) <location>: Change type to
+	event_location_up.
+	* linespec.c (canonicalize_linespec, event_location_to_sals)
+	(decode_objc): Update.
+	(linespec_result): Don't call delete_event_location.
+	* breakpoint.c (create_breakpoints_sal)
+	(bkpt_probe_create_sals_from_location)
+	(strace_marker_create_sals_from_location): Update.
+
+2017-04-11  Tom Tromey  <tom@tromey.com>
+
 	* linespec.h (struct linespec_result): Add constructor and
 	destructor.
 	(init_linespec_result, destroy_linespec_result)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 42f344a..42e2e6d 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -9429,7 +9429,7 @@ create_breakpoints_sal (struct gdbarch *gdbarch,
 	 'break', without arguments.  */
       event_location_up location
 	= (canonical->location != NULL
-	   ? copy_event_location (canonical->location) : NULL);
+	   ? copy_event_location (canonical->location.get ()) : NULL);
       char *filter_string = lsal->canonical ? xstrdup (lsal->canonical) : NULL;
 
       make_cleanup (xfree, filter_string);
@@ -13390,7 +13390,8 @@ bkpt_probe_create_sals_from_location (const struct event_location *location,
   struct linespec_sals lsal;
 
   lsal.sals = parse_probes (location, NULL, canonical);
-  lsal.canonical = xstrdup (event_location_to_string (canonical->location));
+  lsal.canonical
+    = xstrdup (event_location_to_string (canonical->location.get ()));
   VEC_safe_push (linespec_sals, canonical->sals, &lsal);
 }
 
@@ -13646,10 +13647,11 @@ strace_marker_create_sals_from_location (const struct event_location *location,
 
   str = savestring (arg_start, arg - arg_start);
   cleanup = make_cleanup (xfree, str);
-  canonical->location = new_linespec_location (&str).release ();
+  canonical->location = new_linespec_location (&str);
   do_cleanups (cleanup);
 
-  lsal.canonical = xstrdup (event_location_to_string (canonical->location));
+  lsal.canonical
+    = xstrdup (event_location_to_string (canonical->location.get ()));
   VEC_safe_push (linespec_sals, canonical->sals, &lsal);
 }
 
@@ -13686,7 +13688,7 @@ strace_marker_create_breakpoints_sal (struct gdbarch *gdbarch,
       expanded.nelts = 1;
       expanded.sals = &lsal->sals.sals[i];
 
-      location = copy_event_location (canonical->location);
+      location = copy_event_location (canonical->location.get ());
 
       tp = new tracepoint ();
       init_breakpoint_sal (&tp->base, gdbarch, expanded,
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 51fa128..acf4900 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -1750,8 +1750,9 @@ canonicalize_linespec (struct linespec_state *state, const linespec_p ls)
     return;
 
   /* Save everything as an explicit location.  */
-  canon = state->canonical->location
-    = new_explicit_location (&ls->explicit_loc).release ();
+  state->canonical->location
+    = new_explicit_location (&ls->explicit_loc);
+  canon = state->canonical->location.get ();
   explicit_loc = get_explicit_location (canon);
 
   if (explicit_loc->label_name != NULL)
@@ -2491,7 +2492,7 @@ event_location_to_sals (linespec_parser *parser,
 	    addr = linespec_expression_to_pc (&const_expr);
 	    if (PARSER_STATE (parser)->canonical != NULL)
 	      PARSER_STATE (parser)->canonical->location
-		= copy_event_location (location).release ();
+		= copy_event_location (location);
 
 	    do_cleanups (cleanup);
 	  }
@@ -2780,7 +2781,7 @@ decode_objc (struct linespec_state *self, linespec_p ls, const char *arg)
 	    str = xstrdup (saved_arg);
 
 	  make_cleanup (xfree, str);
-	  self->canonical->location = new_linespec_location (&str).release ();
+	  self->canonical->location = new_linespec_location (&str);
 	}
     }
 
@@ -3892,7 +3893,6 @@ linespec_result::~linespec_result ()
   int i;
   struct linespec_sals *lsal;
 
-  delete_event_location (location);
   for (i = 0; VEC_iterate (linespec_sals, sals, i, lsal); ++i)
     {
       xfree (lsal->canonical);
diff --git a/gdb/linespec.h b/gdb/linespec.h
index 7459ff8..ee8b9ae 100644
--- a/gdb/linespec.h
+++ b/gdb/linespec.h
@@ -19,6 +19,7 @@
 
 struct symtab;
 
+#include "location.h"
 #include "vec.h"
 
 /* Flags to pass to decode_line_1 and decode_line_full.  */
@@ -61,7 +62,6 @@ struct linespec_result
   linespec_result ()
     : special_display (0),
       pre_expanded (0),
-      location (NULL),
       sals (NULL)
   {
   }
@@ -83,8 +83,8 @@ struct linespec_result
   int pre_expanded;
 
   /* If PRE_EXPANDED is non-zero, this is set to the location entered
-     by the user.  This will be freed by destroy_linespec_result.  */
-  struct event_location *location;
+     by the user.  */
+  event_location_up location;
 
   /* The sals.  The vector will be freed by
      destroy_linespec_result.  */
diff --git a/gdb/location.c b/gdb/location.c
index 8aa8bd5..8796320 100644
--- a/gdb/location.c
+++ b/gdb/location.c
@@ -342,10 +342,8 @@ copy_event_location (const struct event_location *src)
   return event_location_up (dst);
 }
 
-/* See description in location.h.  */
-
 void
-delete_event_location (struct event_location *location)
+event_location_deleter::operator() (event_location *location) const
 {
   if (location != NULL)
     {
diff --git a/gdb/location.h b/gdb/location.h
index 5a5adb5..7e1f012 100644
--- a/gdb/location.h
+++ b/gdb/location.h
@@ -114,18 +114,11 @@ extern char *
 extern const char *
   event_location_to_string (struct event_location *location);
 
-/* Free an event location and any associated data.  */
-
-extern void delete_event_location (struct event_location *location);
-
 /* A deleter for a struct event_location.  */
 
 struct event_location_deleter
 {
-  void operator() (event_location *location) const
-  {
-    delete_event_location (location);
-  }
+  void operator() (event_location *location) const;
 };
 
 /* A unique pointer for event_location.  */
diff --git a/gdb/probe.c b/gdb/probe.c
index 4f4a5a3..c147810 100644
--- a/gdb/probe.c
+++ b/gdb/probe.c
@@ -205,7 +205,7 @@ parse_probes (const struct event_location *location,
       make_cleanup (xfree, canon);
       canonical->special_display = 1;
       canonical->pre_expanded = 1;
-      canonical->location = new_probe_location (canon).release ();
+      canonical->location = new_probe_location (canon);
     }
 
   do_cleanups (cleanup);
-- 
2.9.3

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

* [RFA v2 05/17] Change increment_reading_symtab to return a scoped_restore
  2017-04-11 15:02 [RFA v2 00/17] miscellaneous C++-ification Tom Tromey
                   ` (4 preceding siblings ...)
  2017-04-11 15:01 ` [RFA v2 11/17] Use scoped_restore in more places Tom Tromey
@ 2017-04-11 15:01 ` Tom Tromey
  2017-04-12 11:16   ` Pedro Alves
  2017-04-11 15:01 ` [RFA v2 12/17] Use std::vector in reread_symbols Tom Tromey
                   ` (11 subsequent siblings)
  17 siblings, 1 reply; 36+ messages in thread
From: Tom Tromey @ 2017-04-11 15:01 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes increment_reading_symtab to return a scoped_restore, then
fixes up the users.

gdb/ChangeLog
2017-04-11  Tom Tromey  <tom@tromey.com>

	* symfile.h (increment_reading_symtab): Update type.
	* symfile.c (decrement_reading_symtab): Remove.
	(increment_reading_symtab): Return a scoped_restore_tmpl<int>.
	* psymtab.c (psymtab_to_symtab): Update.
	* dwarf2read.c (dw2_instantiate_symtab): Update.
---
 gdb/ChangeLog    |  8 ++++++++
 gdb/dwarf2read.c |  2 +-
 gdb/psymtab.c    |  3 +--
 gdb/symfile.c    | 15 +++------------
 gdb/symfile.h    |  2 +-
 5 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index a888174..1c76edb 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,13 @@
 2017-04-11  Tom Tromey  <tom@tromey.com>
 
+	* symfile.h (increment_reading_symtab): Update type.
+	* symfile.c (decrement_reading_symtab): Remove.
+	(increment_reading_symtab): Return a scoped_restore_tmpl<int>.
+	* psymtab.c (psymtab_to_symtab): Update.
+	* dwarf2read.c (dw2_instantiate_symtab): Update.
+
+2017-04-11  Tom Tromey  <tom@tromey.com>
+
 	* jit.c (struct jit_reader): Declare separately.  Add constructor
 	and destructor.  Change type of "handle".
 	(loaded_jit_reader): Define separately.
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 966e1ee..091d665 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -2885,7 +2885,7 @@ dw2_instantiate_symtab (struct dwarf2_per_cu_data *per_cu)
   if (!per_cu->v.quick->compunit_symtab)
     {
       struct cleanup *back_to = make_cleanup (free_cached_comp_units, NULL);
-      increment_reading_symtab ();
+      scoped_restore decrementer = increment_reading_symtab ();
       dw2_do_instantiate_symtab (per_cu);
       process_cu_includes ();
       do_cleanups (back_to);
diff --git a/gdb/psymtab.c b/gdb/psymtab.c
index bdce8f2..bb482ee 100644
--- a/gdb/psymtab.c
+++ b/gdb/psymtab.c
@@ -770,10 +770,9 @@ psymtab_to_symtab (struct objfile *objfile, struct partial_symtab *pst)
   /* If it has not yet been read in, read it.  */
   if (!pst->readin)
     {
-      struct cleanup *back_to = increment_reading_symtab ();
+      scoped_restore decrementer = increment_reading_symtab ();
 
       (*pst->read_symtab) (pst, objfile);
-      do_cleanups (back_to);
     }
 
   return pst->compunit_symtab;
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 750039d..4d9fe54 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -91,8 +91,6 @@ static void add_symbol_file_command (char *, int);
 
 static const struct sym_fns *find_sym_fns (bfd *);
 
-static void decrement_reading_symtab (void *);
-
 static void overlay_invalidate_all (void);
 
 static void overlay_auto_command (char *, int);
@@ -193,22 +191,15 @@ print_symbol_loading_p (int from_tty, int exec, int full)
 
 int currently_reading_symtab = 0;
 
-static void
-decrement_reading_symtab (void *dummy)
-{
-  currently_reading_symtab--;
-  gdb_assert (currently_reading_symtab >= 0);
-}
-
 /* Increment currently_reading_symtab and return a cleanup that can be
    used to decrement it.  */
 
-struct cleanup *
+scoped_restore_tmpl<int>
 increment_reading_symtab (void)
 {
-  ++currently_reading_symtab;
+  return make_scoped_restore (&currently_reading_symtab,
+			      currently_reading_symtab + 1);
   gdb_assert (currently_reading_symtab > 0);
-  return make_cleanup (decrement_reading_symtab, NULL);
 }
 
 /* Remember the lowest-addressed loadable section we've seen.
diff --git a/gdb/symfile.h b/gdb/symfile.h
index 6066481..ab536e8 100644
--- a/gdb/symfile.h
+++ b/gdb/symfile.h
@@ -556,7 +556,7 @@ extern int symfile_map_offsets_to_segments (bfd *,
 struct symfile_segment_data *get_symfile_segment_data (bfd *abfd);
 void free_symfile_segment_data (struct symfile_segment_data *data);
 
-extern struct cleanup *increment_reading_symtab (void);
+extern scoped_restore_tmpl<int> increment_reading_symtab (void);
 
 void expand_symtabs_matching
   (gdb::function_view<expand_symtabs_file_matcher_ftype> file_matcher,
-- 
2.9.3

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

* [RFA v2 01/17] Introduce event_location_up
  2017-04-11 15:02 [RFA v2 00/17] miscellaneous C++-ification Tom Tromey
                   ` (6 preceding siblings ...)
  2017-04-11 15:01 ` [RFA v2 12/17] Use std::vector in reread_symbols Tom Tromey
@ 2017-04-11 15:01 ` Tom Tromey
  2017-04-11 15:01 ` [RFA v2 04/17] Introduce gdb_dlhandle_up Tom Tromey
                   ` (9 subsequent siblings)
  17 siblings, 0 replies; 36+ messages in thread
From: Tom Tromey @ 2017-04-11 15:01 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This removes make_cleanup_delete_event_location and instead changes
the various location functions to return an event_location_up, a new
unique_ptr typedef.

This is largely straightforward, but be sure to examine the
init_breakpoint_sal change.  I believe the code I deleted there is
dead, because "location != NULL" can never be true in that branch; but
you should double-check.

gdb/ChangeLog
2017-04-11  Tom Tromey  <tom@tromey.com>

	* tracepoint.c (scope_info): Update.
	* spu-tdep.c (spu_catch_start): Update.
	* python/python.c (gdbpy_decode_line): Update.
	* python/py-finishbreakpoint.c (bpfinishpy_init): Update.
	* python/py-breakpoint.c (bppy_init): Update.
	* probe.c (parse_probes): Update.
	* mi/mi-cmd-break.c (mi_cmd_break_insert_1): Update.
	* location.h (event_location_deleter): New struct.
	(event_location_up): New typedef.
	(new_linespec_location, new_address_location, new_probe_location)
	(new_explicit_location, copy_event_location)
	(string_to_event_location, string_to_event_location_basic)
	(string_to_explicit_location): Update return type.
	(make_cleanup_delete_event_location): Remove.
	* location.c (new_linespec_location, new_address_location)
	(new_probe_location, new_explicit_location, copy_event_location):
	Return event_location_up.
	(delete_event_location_cleanup)
	(make_cleanup_delete_event_location): Remove.
	(string_to_explicit_location, string_to_event_location_basic)
	(string_to_event_location): Return event_location_up.
	* linespec.c (canonicalize_linespec, event_location_to_sals)
	(decode_line_with_current_source)
	(decode_line_with_last_displayed, decode_objc): Update.
	* guile/scm-breakpoint.c (gdbscm_register_breakpoint_x): Update.
	* completer.c (location_completer): Update.
	* cli/cli-cmds.c (edit_command, list_command): Update.
	* breakpoint.c (create_overlay_event_breakpoint)
	(create_longjmp_master_breakpoint)
	(create_std_terminate_master_breakpoint)
	(create_exception_master_breakpoint)
	(create_thread_event_breakpoint): Update.
	(init_breakpoint_sal): Update.  Remove some dead code.
	(create_breakpoint_sal): Change type of "location".  Update.
	(create_breakpoints_sal, create_breakpoint, break_command_1)
	(dprintf_command, break_range_command, until_break_command)
	(init_ada_exception_breakpoint)
	(strace_marker_create_sals_from_location)
	(update_static_tracepoint, trace_command, ftrace_command)
	(strace_command, create_tracepoint_from_upload): Update.
	* break-catch-throw.c (re_set_exception_catchpoint): Update.
	* ax-gdb.c (agent_command_1): Update.
---
 gdb/ChangeLog                    |  45 ++++++++++++
 gdb/ax-gdb.c                     |   8 +--
 gdb/break-catch-throw.c          |  14 ++--
 gdb/breakpoint.c                 | 152 ++++++++++++++-------------------------
 gdb/cli/cli-cmds.c               |  43 ++++-------
 gdb/completer.c                  |  14 ++--
 gdb/guile/scm-breakpoint.c       |   9 +--
 gdb/linespec.c                   |  27 +++----
 gdb/location.c                   |  53 ++++----------
 gdb/location.h                   |  62 ++++++++--------
 gdb/mi/mi-cmd-break.c            |  11 +--
 gdb/probe.c                      |   2 +-
 gdb/python/py-breakpoint.c       |  10 +--
 gdb/python/py-finishbreakpoint.c |  10 +--
 gdb/python/python.c              |   9 +--
 gdb/spu-tdep.c                   |   8 +--
 gdb/tracepoint.c                 |  11 ++-
 17 files changed, 202 insertions(+), 286 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index a5d07e6..f7aefaa 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,48 @@
+2017-04-11  Tom Tromey  <tom@tromey.com>
+
+	* tracepoint.c (scope_info): Update.
+	* spu-tdep.c (spu_catch_start): Update.
+	* python/python.c (gdbpy_decode_line): Update.
+	* python/py-finishbreakpoint.c (bpfinishpy_init): Update.
+	* python/py-breakpoint.c (bppy_init): Update.
+	* probe.c (parse_probes): Update.
+	* mi/mi-cmd-break.c (mi_cmd_break_insert_1): Update.
+	* location.h (event_location_deleter): New struct.
+	(event_location_up): New typedef.
+	(new_linespec_location, new_address_location, new_probe_location)
+	(new_explicit_location, copy_event_location)
+	(string_to_event_location, string_to_event_location_basic)
+	(string_to_explicit_location): Update return type.
+	(make_cleanup_delete_event_location): Remove.
+	* location.c (new_linespec_location, new_address_location)
+	(new_probe_location, new_explicit_location, copy_event_location):
+	Return event_location_up.
+	(delete_event_location_cleanup)
+	(make_cleanup_delete_event_location): Remove.
+	(string_to_explicit_location, string_to_event_location_basic)
+	(string_to_event_location): Return event_location_up.
+	* linespec.c (canonicalize_linespec, event_location_to_sals)
+	(decode_line_with_current_source)
+	(decode_line_with_last_displayed, decode_objc): Update.
+	* guile/scm-breakpoint.c (gdbscm_register_breakpoint_x): Update.
+	* completer.c (location_completer): Update.
+	* cli/cli-cmds.c (edit_command, list_command): Update.
+	* breakpoint.c (create_overlay_event_breakpoint)
+	(create_longjmp_master_breakpoint)
+	(create_std_terminate_master_breakpoint)
+	(create_exception_master_breakpoint)
+	(create_thread_event_breakpoint): Update.
+	(init_breakpoint_sal): Update.  Remove some dead code.
+	(create_breakpoint_sal): Change type of "location".  Update.
+	(create_breakpoints_sal, create_breakpoint, break_command_1)
+	(dprintf_command, break_range_command, until_break_command)
+	(init_ada_exception_breakpoint)
+	(strace_marker_create_sals_from_location)
+	(update_static_tracepoint, trace_command, ftrace_command)
+	(strace_command, create_tracepoint_from_upload): Update.
+	* break-catch-throw.c (re_set_exception_catchpoint): Update.
+	* ax-gdb.c (agent_command_1): Update.
+
 2017-04-11  Pedro Alves  <palves@redhat.com>
 
 	* thread.c: Fix whitespace throughout.
diff --git a/gdb/ax-gdb.c b/gdb/ax-gdb.c
index 05b1dd7..7206022 100644
--- a/gdb/ax-gdb.c
+++ b/gdb/ax-gdb.c
@@ -2604,16 +2604,14 @@ agent_command_1 (char *exp, int eval)
       int ix;
       struct linespec_sals *iter;
       struct cleanup *old_chain;
-      struct event_location *location;
 
       exp = skip_spaces (exp);
       init_linespec_result (&canonical);
-      location = new_linespec_location (&exp);
-      old_chain = make_cleanup_delete_event_location (location);
-      decode_line_full (location, DECODE_LINE_FUNFIRSTLINE, NULL,
+      event_location_up location = new_linespec_location (&exp);
+      decode_line_full (location.get (), DECODE_LINE_FUNFIRSTLINE, NULL,
 			(struct symtab *) NULL, 0, &canonical,
 			NULL, NULL);
-      make_cleanup_destroy_linespec_result (&canonical);
+      old_chain = make_cleanup_destroy_linespec_result (&canonical);
       exp = skip_spaces (exp);
       if (exp[0] == ',')
         {
diff --git a/gdb/break-catch-throw.c b/gdb/break-catch-throw.c
index 2e18d2a..2714bbf 100644
--- a/gdb/break-catch-throw.c
+++ b/gdb/break-catch-throw.c
@@ -205,17 +205,14 @@ re_set_exception_catchpoint (struct breakpoint *self)
   struct symtabs_and_lines sals_end = {0};
   struct cleanup *cleanup;
   enum exception_event_kind kind = classify_exception_breakpoint (self);
-  struct event_location *location;
   struct program_space *filter_pspace = current_program_space;
 
   /* We first try to use the probe interface.  */
   TRY
     {
-      location
+      event_location_up location
 	= new_probe_location (exception_functions[kind].probe);
-      cleanup = make_cleanup_delete_event_location (location);
-      sals = parse_probes (location, filter_pspace, NULL);
-      do_cleanups (cleanup);
+      sals = parse_probes (location.get (), filter_pspace, NULL);
     }
   CATCH (e, RETURN_MASK_ERROR)
     {
@@ -228,10 +225,9 @@ re_set_exception_catchpoint (struct breakpoint *self)
 	  initialize_explicit_location (&explicit_loc);
 	  explicit_loc.function_name
 	    = ASTRDUP (exception_functions[kind].function);
-	  location = new_explicit_location (&explicit_loc);
-	  cleanup = make_cleanup_delete_event_location (location);
-	  self->ops->decode_location (self, location, filter_pspace, &sals);
-	  do_cleanups (cleanup);
+	  event_location_up location = new_explicit_location (&explicit_loc);
+	  self->ops->decode_location (self, location.get (), filter_pspace,
+				      &sals);
 	}
       CATCH (ex, RETURN_MASK_ERROR)
 	{
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 3925ec6..9f7db91 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -3472,7 +3472,7 @@ create_overlay_event_breakpoint (void)
 				      &internal_breakpoint_ops);
       initialize_explicit_location (&explicit_loc);
       explicit_loc.function_name = ASTRDUP (func_name);
-      b->location = new_explicit_location (&explicit_loc);
+      b->location = new_explicit_location (&explicit_loc).release ();
 
       if (overlay_debugging == ovly_auto)
         {
@@ -3553,7 +3553,7 @@ create_longjmp_master_breakpoint (void)
 					      bp_longjmp_master,
 					      &internal_breakpoint_ops);
 	      b->location
-		= new_probe_location ("-probe-stap libc:longjmp");
+		= new_probe_location ("-probe-stap libc:longjmp").release ();
 	      b->enable_state = bp_disabled;
 	    }
 
@@ -3593,7 +3593,7 @@ create_longjmp_master_breakpoint (void)
 					  &internal_breakpoint_ops);
 	  initialize_explicit_location (&explicit_loc);
 	  explicit_loc.function_name = ASTRDUP (func_name);
-	  b->location = new_explicit_location (&explicit_loc);
+	  b->location = new_explicit_location (&explicit_loc).release ();
 	  b->enable_state = bp_disabled;
 	}
     }
@@ -3651,7 +3651,7 @@ create_std_terminate_master_breakpoint (void)
 				      &internal_breakpoint_ops);
       initialize_explicit_location (&explicit_loc);
       explicit_loc.function_name = ASTRDUP (func_name);
-      b->location = new_explicit_location (&explicit_loc);
+      b->location = new_explicit_location (&explicit_loc).release ();
       b->enable_state = bp_disabled;
     }
   }
@@ -3721,7 +3721,7 @@ create_exception_master_breakpoint (void)
 					      bp_exception_master,
 					      &internal_breakpoint_ops);
 	      b->location
-		= new_probe_location ("-probe-stap libgcc:unwind");
+		= new_probe_location ("-probe-stap libgcc:unwind").release ();
 	      b->enable_state = bp_disabled;
 	    }
 
@@ -3756,7 +3756,7 @@ create_exception_master_breakpoint (void)
 				      &internal_breakpoint_ops);
       initialize_explicit_location (&explicit_loc);
       explicit_loc.function_name = ASTRDUP (func_name);
-      b->location = new_explicit_location (&explicit_loc);
+      b->location = new_explicit_location (&explicit_loc).release ();
       b->enable_state = bp_disabled;
     }
 }
@@ -7796,7 +7796,7 @@ create_thread_event_breakpoint (struct gdbarch *gdbarch, CORE_ADDR address)
 
   b->enable_state = bp_enabled;
   /* location has to be used or breakpoint_re_set will delete me.  */
-  b->location = new_address_location (b->loc->address, NULL, 0);
+  b->location = new_address_location (b->loc->address, NULL, 0).release ();
 
   update_global_location_list_nothrow (UGLL_MAY_INSERT);
 
@@ -9219,7 +9219,7 @@ update_dprintf_commands (char *args, int from_tty,
 static void
 init_breakpoint_sal (struct breakpoint *b, struct gdbarch *gdbarch,
 		     struct symtabs_and_lines sals,
-		     struct event_location *location,
+		     event_location_up &&location,
 		     char *filter, char *cond_string,
 		     char *extra_string,
 		     enum bptype type, enum bpdisp disposition,
@@ -9348,27 +9348,16 @@ init_breakpoint_sal (struct breakpoint *b, struct gdbarch *gdbarch,
 
   b->display_canonical = display_canonical;
   if (location != NULL)
-    b->location = location;
+    b->location = location.release ();
   else
-    {
-      const char *addr_string = NULL;
-      int addr_string_len = 0;
-
-      if (location != NULL)
-	addr_string = event_location_to_string (location);
-      if (addr_string != NULL)
-	addr_string_len = strlen (addr_string);
-
-      b->location = new_address_location (b->loc->address,
-					  addr_string, addr_string_len);
-    }
+    b->location = new_address_location (b->loc->address, NULL, 0).release ();
   b->filter = filter;
 }
 
 static void
 create_breakpoint_sal (struct gdbarch *gdbarch,
 		       struct symtabs_and_lines sals,
-		       struct event_location *location,
+		       event_location_up &&location,
 		       char *filter, char *cond_string,
 		       char *extra_string,
 		       enum bptype type, enum bpdisp disposition,
@@ -9393,7 +9382,7 @@ create_breakpoint_sal (struct gdbarch *gdbarch,
   old_chain = make_cleanup (xfree, b);
 
   init_breakpoint_sal (b, gdbarch,
-		       sals, location,
+		       sals, std::move (location),
 		       filter, cond_string, extra_string,
 		       type, disposition,
 		       thread, task, ignore_count,
@@ -9439,22 +9428,20 @@ create_breakpoints_sal (struct gdbarch *gdbarch,
     {
       /* Note that 'location' can be NULL in the case of a plain
 	 'break', without arguments.  */
-      struct event_location *location
+      event_location_up location
 	= (canonical->location != NULL
 	   ? copy_event_location (canonical->location) : NULL);
       char *filter_string = lsal->canonical ? xstrdup (lsal->canonical) : NULL;
-      struct cleanup *inner = make_cleanup_delete_event_location (location);
 
       make_cleanup (xfree, filter_string);
       create_breakpoint_sal (gdbarch, lsal->sals,
-			     location,
+			     std::move (location),
 			     filter_string,
 			     cond_string, extra_string,
 			     type, disposition,
 			     thread, task, ignore_count, ops,
 			     from_tty, enabled, internal, flags,
 			     canonical->special_display);
-      discard_cleanups (inner);
     }
 }
 
@@ -9892,7 +9879,7 @@ create_breakpoint (struct gdbarch *gdbarch,
 	b = new breakpoint ();
 
       init_raw_breakpoint_without_location (b, gdbarch, type_wanted, ops);
-      b->location = copy_event_location (location);
+      b->location = copy_event_location (location).release ();
 
       if (parse_extra)
 	b->cond_string = NULL;
@@ -9960,21 +9947,18 @@ break_command_1 (char *arg, int flag, int from_tty)
 			     ? bp_hardware_breakpoint
 			     : bp_breakpoint);
   struct breakpoint_ops *ops;
-  struct event_location *location;
-  struct cleanup *cleanup;
 
-  location = string_to_event_location (&arg, current_language);
-  cleanup = make_cleanup_delete_event_location (location);
+  event_location_up location = string_to_event_location (&arg, current_language);
 
   /* Matching breakpoints on probes.  */
   if (location != NULL
-      && event_location_type (location) == PROBE_LOCATION)
+      && event_location_type (location.get ()) == PROBE_LOCATION)
     ops = &bkpt_probe_breakpoint_ops;
   else
     ops = &bkpt_breakpoint_ops;
 
   create_breakpoint (get_current_arch (),
-		     location,
+		     location.get (),
 		     NULL, 0, arg, 1 /* parse arg */,
 		     tempflag, type_wanted,
 		     0 /* Ignore count */,
@@ -9984,7 +9968,6 @@ break_command_1 (char *arg, int flag, int from_tty)
 		     1 /* enabled */,
 		     0 /* internal */,
 		     0);
-  do_cleanups (cleanup);
 }
 
 /* Helper function for break_command_1 and disassemble_command.  */
@@ -10151,11 +10134,7 @@ stopat_command (char *arg, int from_tty)
 static void
 dprintf_command (char *arg, int from_tty)
 {
-  struct event_location *location;
-  struct cleanup *cleanup;
-
-  location = string_to_event_location (&arg, current_language);
-  cleanup = make_cleanup_delete_event_location (location);
+  event_location_up location = string_to_event_location (&arg, current_language);
 
   /* If non-NULL, ARG should have been advanced past the location;
      the next character must be ','.  */
@@ -10171,7 +10150,7 @@ dprintf_command (char *arg, int from_tty)
     }
 
   create_breakpoint (get_current_arch (),
-		     location,
+		     location.get (),
 		     NULL, 0, arg, 1 /* parse arg */,
 		     0, bp_dprintf,
 		     0 /* Ignore count */,
@@ -10181,7 +10160,6 @@ dprintf_command (char *arg, int from_tty)
 		     1 /* enabled */,
 		     0 /* internal */,
 		     0);
-  do_cleanups (cleanup);
 }
 
 static void
@@ -10379,7 +10357,6 @@ break_range_command (char *arg, int from_tty)
   struct symtab_and_line sal_start, sal_end;
   struct cleanup *cleanup_bkpt;
   struct linespec_sals *lsal_start, *lsal_end;
-  struct event_location *start_location, *end_location;
 
   /* We don't support software ranged breakpoints.  */
   if (target_ranged_break_num_registers () < 0)
@@ -10399,10 +10376,10 @@ break_range_command (char *arg, int from_tty)
   init_linespec_result (&canonical_start);
 
   arg_start = arg;
-  start_location = string_to_event_location (&arg, current_language);
-  cleanup_bkpt = make_cleanup_delete_event_location (start_location);
-  parse_breakpoint_sals (start_location, &canonical_start);
-  make_cleanup_destroy_linespec_result (&canonical_start);
+  event_location_up start_location = string_to_event_location (&arg,
+							       current_language);
+  parse_breakpoint_sals (start_location.get (), &canonical_start);
+  cleanup_bkpt = make_cleanup_destroy_linespec_result (&canonical_start);
 
   if (arg[0] != ',')
     error (_("Too few arguments."));
@@ -10432,9 +10409,9 @@ break_range_command (char *arg, int from_tty)
      symtab and line as the default symtab and line for the end of the
      range.  This makes it possible to have ranges like "foo.c:27, +14",
      where +14 means 14 lines from the start location.  */
-  end_location = string_to_event_location (&arg, current_language);
-  make_cleanup_delete_event_location (end_location);
-  decode_line_full (end_location, DECODE_LINE_FUNFIRSTLINE, NULL,
+  event_location_up end_location = string_to_event_location (&arg,
+							     current_language);
+  decode_line_full (end_location.get (), DECODE_LINE_FUNFIRSTLINE, NULL,
 		    sal_start.symtab, sal_start.line,
 		    &canonical_end, NULL, NULL);
 
@@ -10475,8 +10452,8 @@ break_range_command (char *arg, int from_tty)
   set_breakpoint_count (breakpoint_count + 1);
   b->number = breakpoint_count;
   b->disposition = disp_donttouch;
-  b->location = copy_event_location (start_location);
-  b->location_range_end = copy_event_location (end_location);
+  b->location = start_location.release ();
+  b->location_range_end = end_location.release ();
   b->loc->length = length;
 
   do_cleanups (cleanup_bkpt);
@@ -11676,10 +11653,9 @@ until_break_command (char *arg, int from_tty, int anywhere)
   struct frame_id caller_frame_id;
   struct breakpoint *location_breakpoint;
   struct breakpoint *caller_breakpoint = NULL;
-  struct cleanup *old_chain, *cleanup;
+  struct cleanup *old_chain;
   int thread;
   struct thread_info *tp;
-  struct event_location *location;
   struct until_break_fsm *sm;
 
   clear_proceed_status (0);
@@ -11687,15 +11663,14 @@ until_break_command (char *arg, int from_tty, int anywhere)
   /* Set a breakpoint where the user wants it and at return from
      this function.  */
 
-  location = string_to_event_location (&arg, current_language);
-  cleanup = make_cleanup_delete_event_location (location);
+  event_location_up location = string_to_event_location (&arg, current_language);
 
   if (last_displayed_sal_is_valid ())
-    sals = decode_line_1 (location, DECODE_LINE_FUNFIRSTLINE, NULL,
+    sals = decode_line_1 (location.get (), DECODE_LINE_FUNFIRSTLINE, NULL,
 			  get_last_displayed_symtab (),
 			  get_last_displayed_line ());
   else
-    sals = decode_line_1 (location, DECODE_LINE_FUNFIRSTLINE,
+    sals = decode_line_1 (location.get (), DECODE_LINE_FUNFIRSTLINE,
 			  NULL, (struct symtab *) NULL, 0);
 
   if (sals.nelts != 1)
@@ -11767,8 +11742,6 @@ until_break_command (char *arg, int from_tty, int anywhere)
   discard_cleanups (old_chain);
 
   proceed (-1, GDB_SIGNAL_DEFAULT);
-
-  do_cleanups (cleanup);
 }
 
 /* This function attempts to parse an optional "if <cond>" clause
@@ -11926,8 +11899,9 @@ init_ada_exception_breakpoint (struct breakpoint *b,
 
   b->enable_state = enabled ? bp_enabled : bp_disabled;
   b->disposition = tempflag ? disp_del : disp_donttouch;
-  b->location = string_to_event_location (&addr_string,
-					  language_def (language_ada));
+  b->location
+    = string_to_event_location (&addr_string,
+				language_def (language_ada)).release ();
   b->language = language_ada;
 }
 
@@ -13690,7 +13664,7 @@ strace_marker_create_sals_from_location (const struct event_location *location,
 
   str = savestring (arg_start, arg - arg_start);
   cleanup = make_cleanup (xfree, str);
-  canonical->location = new_linespec_location (&str);
+  canonical->location = new_linespec_location (&str).release ();
   do_cleanups (cleanup);
 
   lsal.canonical = xstrdup (event_location_to_string (canonical->location));
@@ -13725,18 +13699,16 @@ strace_marker_create_breakpoints_sal (struct gdbarch *gdbarch,
     {
       struct symtabs_and_lines expanded;
       struct tracepoint *tp;
-      struct cleanup *old_chain;
-      struct event_location *location;
+      event_location_up location;
 
       expanded.nelts = 1;
       expanded.sals = &lsal->sals.sals[i];
 
       location = copy_event_location (canonical->location);
-      old_chain = make_cleanup_delete_event_location (location);
 
       tp = new tracepoint ();
       init_breakpoint_sal (&tp->base, gdbarch, expanded,
-			   location, NULL,
+			   std::move (location), NULL,
 			   cond_string, extra_string,
 			   type_wanted, disposition,
 			   thread, task, ignore_count, ops,
@@ -13751,8 +13723,6 @@ strace_marker_create_breakpoints_sal (struct gdbarch *gdbarch,
       tp->static_trace_marker_id_idx = i;
 
       install_breakpoint (internal, &tp->base, 0);
-
-      discard_cleanups (old_chain);
     }
 }
 
@@ -14155,7 +14125,7 @@ update_static_tracepoint (struct breakpoint *b, struct symtab_and_line sal)
 	    = ASTRDUP (symtab_to_filename_for_display (sal2.symtab));
 	  explicit_loc.line_offset.offset = b->loc->line_number;
 	  explicit_loc.line_offset.sign = LINE_OFFSET_NONE;
-	  b->location = new_explicit_location (&explicit_loc);
+	  b->location = new_explicit_location (&explicit_loc).release ();
 
 	  /* Might be nice to check if function changed, and warn if
 	     so.  */
@@ -15225,19 +15195,17 @@ static void
 trace_command (char *arg, int from_tty)
 {
   struct breakpoint_ops *ops;
-  struct event_location *location;
-  struct cleanup *back_to;
 
-  location = string_to_event_location (&arg, current_language);
-  back_to = make_cleanup_delete_event_location (location);
+  event_location_up location = string_to_event_location (&arg,
+							 current_language);
   if (location != NULL
-      && event_location_type (location) == PROBE_LOCATION)
+      && event_location_type (location.get ()) == PROBE_LOCATION)
     ops = &tracepoint_probe_breakpoint_ops;
   else
     ops = &tracepoint_breakpoint_ops;
 
   create_breakpoint (get_current_arch (),
-		     location,
+		     location.get (),
 		     NULL, 0, arg, 1 /* parse arg */,
 		     0 /* tempflag */,
 		     bp_tracepoint /* type_wanted */,
@@ -15247,19 +15215,15 @@ trace_command (char *arg, int from_tty)
 		     from_tty,
 		     1 /* enabled */,
 		     0 /* internal */, 0);
-  do_cleanups (back_to);
 }
 
 static void
 ftrace_command (char *arg, int from_tty)
 {
-  struct event_location *location;
-  struct cleanup *back_to;
-
-  location = string_to_event_location (&arg, current_language);
-  back_to = make_cleanup_delete_event_location (location);
+  event_location_up location = string_to_event_location (&arg,
+							 current_language);
   create_breakpoint (get_current_arch (),
-		     location,
+		     location.get (),
 		     NULL, 0, arg, 1 /* parse arg */,
 		     0 /* tempflag */,
 		     bp_fast_tracepoint /* type_wanted */,
@@ -15269,7 +15233,6 @@ ftrace_command (char *arg, int from_tty)
 		     from_tty,
 		     1 /* enabled */,
 		     0 /* internal */, 0);
-  do_cleanups (back_to);
 }
 
 /* strace command implementation.  Creates a static tracepoint.  */
@@ -15278,7 +15241,7 @@ static void
 strace_command (char *arg, int from_tty)
 {
   struct breakpoint_ops *ops;
-  struct event_location *location;
+  event_location_up location;
   struct cleanup *back_to;
 
   /* Decide if we are dealing with a static tracepoint marker (`-m'),
@@ -15294,9 +15257,8 @@ strace_command (char *arg, int from_tty)
       location = string_to_event_location (&arg, current_language);
     }
 
-  back_to = make_cleanup_delete_event_location (location);
   create_breakpoint (get_current_arch (),
-		     location,
+		     location.get (),
 		     NULL, 0, arg, 1 /* parse arg */,
 		     0 /* tempflag */,
 		     bp_static_tracepoint /* type_wanted */,
@@ -15306,7 +15268,6 @@ strace_command (char *arg, int from_tty)
 		     from_tty,
 		     1 /* enabled */,
 		     0 /* internal */, 0);
-  do_cleanups (back_to);
 }
 
 /* Set up a fake reader function that gets command lines from a linked
@@ -15338,8 +15299,6 @@ create_tracepoint_from_upload (struct uploaded_tp *utp)
 {
   char *addr_str, small_buf[100];
   struct tracepoint *tp;
-  struct event_location *location;
-  struct cleanup *cleanup;
 
   if (utp->at_string)
     addr_str = utp->at_string;
@@ -15362,10 +15321,10 @@ create_tracepoint_from_upload (struct uploaded_tp *utp)
 	       "has no source form, ignoring it"),
 	     utp->number);
 
-  location = string_to_event_location (&addr_str, current_language);
-  cleanup = make_cleanup_delete_event_location (location);
+  event_location_up location = string_to_event_location (&addr_str,
+							 current_language);
   if (!create_breakpoint (get_current_arch (),
-			  location,
+			  location.get (),
 			  utp->cond_string, -1, addr_str,
 			  0 /* parse cond/thread */,
 			  0 /* tempflag */,
@@ -15377,12 +15336,7 @@ create_tracepoint_from_upload (struct uploaded_tp *utp)
 			  utp->enabled /* enabled */,
 			  0 /* internal */,
 			  CREATE_BREAKPOINT_FLAGS_INSERTED))
-    {
-      do_cleanups (cleanup);
-      return NULL;
-    }
-
-  do_cleanups (cleanup);
+    return NULL;
 
   /* Get the tracepoint we just created.  */
   tp = get_tracepoint (tracepoint_count);
diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 4956ba7..2a5b128 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -828,28 +828,25 @@ edit_command (char *arg, int from_tty)
     }
   else
     {
-      struct cleanup *cleanup;
-      struct event_location *location;
       char *arg1;
 
       /* Now should only be one argument -- decode it in SAL.  */
       arg1 = arg;
-      location = string_to_event_location (&arg1, current_language);
-      cleanup = make_cleanup_delete_event_location (location);
-      sals = decode_line_1 (location, DECODE_LINE_LIST_MODE, NULL, NULL, 0);
+      event_location_up location = string_to_event_location (&arg1,
+							     current_language);
+      sals = decode_line_1 (location.get (), DECODE_LINE_LIST_MODE,
+			    NULL, NULL, 0);
 
       filter_sals (&sals);
       if (! sals.nelts)
 	{
 	  /*  C++  */
-	  do_cleanups (cleanup);
 	  return;
 	}
       if (sals.nelts > 1)
 	{
 	  ambiguous_line_spec (&sals);
 	  xfree (sals.sals);
-	  do_cleanups (cleanup);
 	  return;
 	}
 
@@ -891,7 +888,6 @@ edit_command (char *arg, int from_tty)
 
       if (sal.symtab == 0)
         error (_("No line number known for %s."), arg);
-      do_cleanups (cleanup);
     }
 
   if ((editor = (char *) getenv ("EDITOR")) == NULL)
@@ -920,9 +916,6 @@ list_command (char *arg, int from_tty)
   int dummy_beg = 0;
   int linenum_beg = 0;
   char *p;
-  struct cleanup *cleanup;
-
-  cleanup = make_cleanup (null_cleanup, NULL);
 
   /* Pull in the current default source line if necessary.  */
   if (arg == NULL || ((arg[0] == '+' || arg[0] == '-') && arg[1] == '\0'))
@@ -985,24 +978,21 @@ list_command (char *arg, int from_tty)
     dummy_beg = 1;
   else
     {
-      struct event_location *location;
-
-      location = string_to_event_location (&arg1, current_language);
-      make_cleanup_delete_event_location (location);
-      sals = decode_line_1 (location, DECODE_LINE_LIST_MODE, NULL, NULL, 0);
+      event_location_up location = string_to_event_location (&arg1,
+							     current_language);
+      sals = decode_line_1 (location.get (), DECODE_LINE_LIST_MODE,
+			    NULL, NULL, 0);
 
       filter_sals (&sals);
       if (!sals.nelts)
 	{
 	  /*  C++  */
-	  do_cleanups (cleanup);
 	  return;
 	}
       if (sals.nelts > 1)
 	{
 	  ambiguous_line_spec (&sals);
 	  xfree (sals.sals);
-	  do_cleanups (cleanup);
 	  return;
 	}
 
@@ -1027,28 +1017,22 @@ list_command (char *arg, int from_tty)
 	dummy_end = 1;
       else
 	{
-	  struct event_location *location;
-
-	  location = string_to_event_location (&arg1, current_language);
-	  make_cleanup_delete_event_location (location);
+	  event_location_up location
+	    = string_to_event_location (&arg1, current_language);
 	  if (dummy_beg)
-	    sals_end = decode_line_1 (location,
+	    sals_end = decode_line_1 (location.get (),
 				      DECODE_LINE_LIST_MODE, NULL, NULL, 0);
 	  else
-	    sals_end = decode_line_1 (location, DECODE_LINE_LIST_MODE,
+	    sals_end = decode_line_1 (location.get (), DECODE_LINE_LIST_MODE,
 				      NULL, sal.symtab, sal.line);
 
 	  filter_sals (&sals_end);
 	  if (sals_end.nelts == 0)
-	    {
-	      do_cleanups (cleanup);
-	      return;
-	    }
+	    return;
 	  if (sals_end.nelts > 1)
 	    {
 	      ambiguous_line_spec (&sals_end);
 	      xfree (sals_end.sals);
-	      do_cleanups (cleanup);
 	      return;
 	    }
 	  sal_end = sals_end.sals[0];
@@ -1129,7 +1113,6 @@ list_command (char *arg, int from_tty)
 			 ? sal.line + get_lines_to_list ()
 			 : sal_end.line + 1),
 			0);
-  do_cleanups (cleanup);
 }
 
 /* Subroutine of disassemble_command to simplify it.
diff --git a/gdb/completer.c b/gdb/completer.c
index 7e46416..6acf115 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -507,17 +507,13 @@ location_completer (struct cmd_list_element *ignore,
 {
   VEC (char_ptr) *matches = NULL;
   const char *copy = text;
-  struct event_location *location;
 
-  location = string_to_explicit_location (&copy, current_language, 1);
+  event_location_up location = string_to_explicit_location (&copy,
+							    current_language,
+							    1);
   if (location != NULL)
-    {
-      struct cleanup *cleanup;
-
-      cleanup = make_cleanup_delete_event_location (location);
-      matches = explicit_location_completer (ignore, location, text, word);
-      do_cleanups (cleanup);
-    }
+    matches = explicit_location_completer (ignore, location.get (),
+					   text, word);
   else
     {
       /* This is an address or linespec location.
diff --git a/gdb/guile/scm-breakpoint.c b/gdb/guile/scm-breakpoint.c
index 9189446..3a1e4d3 100644
--- a/gdb/guile/scm-breakpoint.c
+++ b/gdb/guile/scm-breakpoint.c
@@ -413,8 +413,6 @@ gdbscm_register_breakpoint_x (SCM self)
     = bpscm_get_breakpoint_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
   struct gdb_exception except = exception_none;
   char *location, *copy;
-  struct event_location *eloc;
-  struct cleanup *cleanup;
 
   /* We only support registering breakpoints created with make-breakpoint.  */
   if (!bp_smob->is_scheme_bkpt)
@@ -426,8 +424,8 @@ gdbscm_register_breakpoint_x (SCM self)
   pending_breakpoint_scm = self;
   location = bp_smob->spec.location;
   copy = skip_spaces (location);
-  eloc = string_to_event_location_basic (&copy, current_language);
-  cleanup = make_cleanup_delete_event_location (eloc);
+  event_location_up eloc = string_to_event_location_basic (&copy,
+							   current_language);
 
   TRY
     {
@@ -438,7 +436,7 @@ gdbscm_register_breakpoint_x (SCM self)
 	case bp_breakpoint:
 	  {
 	    create_breakpoint (get_current_arch (),
-			       eloc, NULL, -1, NULL,
+			       eloc.get (), NULL, -1, NULL,
 			       0,
 			       0, bp_breakpoint,
 			       0,
@@ -474,7 +472,6 @@ gdbscm_register_breakpoint_x (SCM self)
   /* Ensure this gets reset, even if there's an error.  */
   pending_breakpoint_scm = SCM_BOOL_F;
   GDBSCM_HANDLE_GDB_EXCEPTION (except);
-  do_cleanups (cleanup);
 
   return SCM_UNSPECIFIED;
 }
diff --git a/gdb/linespec.c b/gdb/linespec.c
index dcbe253..41b82d7 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -1751,7 +1751,7 @@ canonicalize_linespec (struct linespec_state *state, const linespec_p ls)
 
   /* Save everything as an explicit location.  */
   canon = state->canonical->location
-    = new_explicit_location (&ls->explicit_loc);
+    = new_explicit_location (&ls->explicit_loc).release ();
   explicit_loc = get_explicit_location (canon);
 
   if (explicit_loc->label_name != NULL)
@@ -2491,7 +2491,7 @@ event_location_to_sals (linespec_parser *parser,
 	    addr = linespec_expression_to_pc (&const_expr);
 	    if (PARSER_STATE (parser)->canonical != NULL)
 	      PARSER_STATE (parser)->canonical->location
-		= copy_event_location (location);
+		= copy_event_location (location).release ();
 
 	    do_cleanups (cleanup);
 	  }
@@ -2630,8 +2630,6 @@ decode_line_with_current_source (char *string, int flags)
 {
   struct symtabs_and_lines sals;
   struct symtab_and_line cursal;
-  struct event_location *location;
-  struct cleanup *cleanup;
 
   if (string == 0)
     error (_("Empty line specification."));
@@ -2640,15 +2638,14 @@ decode_line_with_current_source (char *string, int flags)
      and get a default source symtab+line or it will recursively call us!  */
   cursal = get_current_source_symtab_and_line ();
 
-  location = string_to_event_location (&string, current_language);
-  cleanup = make_cleanup_delete_event_location (location);
-  sals = decode_line_1 (location, flags, NULL,
+  event_location_up location = string_to_event_location (&string,
+							 current_language);
+  sals = decode_line_1 (location.get (), flags, NULL,
 			cursal.symtab, cursal.line);
 
   if (*string)
     error (_("Junk at end of line specification: %s"), string);
 
-  do_cleanups (cleanup);
   return sals;
 }
 
@@ -2658,25 +2655,23 @@ struct symtabs_and_lines
 decode_line_with_last_displayed (char *string, int flags)
 {
   struct symtabs_and_lines sals;
-  struct event_location *location;
-  struct cleanup *cleanup;
 
   if (string == 0)
     error (_("Empty line specification."));
 
-  location = string_to_event_location (&string, current_language);
-  cleanup = make_cleanup_delete_event_location (location);
+  event_location_up location = string_to_event_location (&string,
+							 current_language);
   if (last_displayed_sal_is_valid ())
-    sals = decode_line_1 (location, flags, NULL,
+    sals = decode_line_1 (location.get (), flags, NULL,
 			  get_last_displayed_symtab (),
 			  get_last_displayed_line ());
   else
-    sals = decode_line_1 (location, flags, NULL, (struct symtab *) NULL, 0);
+    sals = decode_line_1 (location.get (), flags, NULL,
+			  (struct symtab *) NULL, 0);
 
   if (*string)
     error (_("Junk at end of line specification: %s"), string);
 
-  do_cleanups (cleanup);
   return sals;
 }
 
@@ -2785,7 +2780,7 @@ decode_objc (struct linespec_state *self, linespec_p ls, const char *arg)
 	    str = xstrdup (saved_arg);
 
 	  make_cleanup (xfree, str);
-	  self->canonical->location = new_linespec_location (&str);
+	  self->canonical->location = new_linespec_location (&str).release ();
 	}
     }
 
diff --git a/gdb/location.c b/gdb/location.c
index fbd09e2..877c1ed 100644
--- a/gdb/location.c
+++ b/gdb/location.c
@@ -81,7 +81,7 @@ initialize_explicit_location (struct explicit_location *explicit_loc)
 
 /* See description in location.h.  */
 
-struct event_location *
+event_location_up
 new_linespec_location (char **linespec)
 {
   struct event_location *location;
@@ -98,7 +98,7 @@ new_linespec_location (char **linespec)
       if ((p - orig) > 0)
 	EL_LINESPEC (location) = savestring (orig, p - orig);
     }
-  return location;
+  return event_location_up (location);
 }
 
 /* See description in location.h.  */
@@ -112,7 +112,7 @@ get_linespec_location (const struct event_location *location)
 
 /* See description in location.h.  */
 
-struct event_location *
+event_location_up
 new_address_location (CORE_ADDR addr, const char *addr_string,
 		      int addr_string_len)
 {
@@ -123,7 +123,7 @@ new_address_location (CORE_ADDR addr, const char *addr_string,
   EL_ADDRESS (location) = addr;
   if (addr_string != NULL)
     EL_STRING (location) = xstrndup (addr_string, addr_string_len);
-  return location;
+  return event_location_up (location);
 }
 
 /* See description in location.h.  */
@@ -146,7 +146,7 @@ get_address_string_location (const struct event_location *location)
 
 /* See description in location.h.  */
 
-struct event_location *
+event_location_up
 new_probe_location (const char *probe)
 {
   struct event_location *location;
@@ -155,7 +155,7 @@ new_probe_location (const char *probe)
   EL_TYPE (location) = PROBE_LOCATION;
   if (probe != NULL)
     EL_PROBE (location) = xstrdup (probe);
-  return location;
+  return event_location_up (location);
 }
 
 /* See description in location.h.  */
@@ -169,7 +169,7 @@ get_probe_location (const struct event_location *location)
 
 /* See description in location.h.  */
 
-struct event_location *
+event_location_up
 new_explicit_location (const struct explicit_location *explicit_loc)
 {
   struct event_location tmp;
@@ -293,7 +293,7 @@ explicit_location_to_linespec (const struct explicit_location *explicit_loc)
 
 /* See description in location.h.  */
 
-struct event_location *
+event_location_up
 copy_event_location (const struct event_location *src)
 {
   struct event_location *dst;
@@ -339,25 +339,7 @@ copy_event_location (const struct event_location *src)
       gdb_assert_not_reached ("unknown event location type");
     }
 
-  return dst;
-}
-
-/* A cleanup function for struct event_location.  */
-
-static void
-delete_event_location_cleanup (void *data)
-{
-  struct event_location *location = (struct event_location *) data;
-
-  delete_event_location (location);
-}
-
-/* See description in location.h.  */
-
-struct cleanup *
-make_cleanup_delete_event_location (struct event_location *location)
-{
-  return make_cleanup (delete_event_location_cleanup, location);
+  return event_location_up (dst);
 }
 
 /* See description in location.h.  */
@@ -505,13 +487,12 @@ explicit_location_lex_one (const char **inp,
 
 /* See description in location.h.  */
 
-struct event_location *
+event_location_up
 string_to_explicit_location (const char **argp,
 			     const struct language_defn *language,
 			     int dont_throw)
 {
-  struct cleanup *cleanup;
-  struct event_location *location;
+  event_location_up location;
 
   /* It is assumed that input beginning with '-' and a non-digit
      character is an explicit location.  "-p" is reserved, though,
@@ -524,7 +505,6 @@ string_to_explicit_location (const char **argp,
     return NULL;
 
   location = new_explicit_location (NULL);
-  cleanup = make_cleanup_delete_event_location (location);
 
   /* Process option/argument pairs.  dprintf_command
      requires that processing stop on ','.  */
@@ -591,7 +571,6 @@ string_to_explicit_location (const char **argp,
 	  *argp = start;
 	  discard_cleanups (oarg_cleanup);
 	  do_cleanups (opt_cleanup);
-	  discard_cleanups (cleanup);
 	  return location;
 	}
 
@@ -621,17 +600,16 @@ string_to_explicit_location (const char **argp,
 	       "line offset."));
     }
 
-  discard_cleanups (cleanup);
   return location;
 }
 
 /* See description in location.h.  */
 
-struct event_location *
+event_location_up
 string_to_event_location_basic (char **stringp,
 				const struct language_defn *language)
 {
-  struct event_location *location;
+  event_location_up location;
   const char *cs;
 
   /* Try the input as a probe spec.  */
@@ -666,16 +644,15 @@ string_to_event_location_basic (char **stringp,
 
 /* See description in location.h.  */
 
-struct event_location *
+event_location_up
 string_to_event_location (char **stringp,
 			  const struct language_defn *language)
 {
-  struct event_location *location;
   const char *arg, *orig;
 
   /* Try an explicit location.  */
   orig = arg = *stringp;
-  location = string_to_explicit_location (&arg, language, 0);
+  event_location_up location = string_to_explicit_location (&arg, language, 0);
   if (location != NULL)
     {
       /* It was a valid explicit location.  Advance STRINGP to
diff --git a/gdb/location.h b/gdb/location.h
index e8a3d20..5a5adb5 100644
--- a/gdb/location.h
+++ b/gdb/location.h
@@ -114,11 +114,27 @@ extern char *
 extern const char *
   event_location_to_string (struct event_location *location);
 
-/* Create a new linespec location.  The return result is malloc'd
-   and should be freed with delete_event_location.  */
+/* Free an event location and any associated data.  */
+
+extern void delete_event_location (struct event_location *location);
+
+/* A deleter for a struct event_location.  */
+
+struct event_location_deleter
+{
+  void operator() (event_location *location) const
+  {
+    delete_event_location (location);
+  }
+};
+
+/* A unique pointer for event_location.  */
+typedef std::unique_ptr<event_location, event_location_deleter>
+     event_location_up;
+
+/* Create a new linespec location.  */
 
-extern struct event_location *
-  new_linespec_location (char **linespec);
+extern event_location_up new_linespec_location (char **linespec);
 
 /* Return the linespec location (a string) of the given event_location
    (which must be of type LINESPEC_LOCATION).  */
@@ -131,9 +147,9 @@ extern const char *
    ADDR_STRING, a string of ADDR_STRING_LEN characters, is
    the expression that was parsed to determine the address ADDR.  */
 
-extern struct event_location *
-  new_address_location (CORE_ADDR addr, const char *addr_string,
-			int addr_string_len);
+extern event_location_up new_address_location (CORE_ADDR addr,
+					       const char *addr_string,
+					       int addr_string_len);
 
 /* Return the address location (a CORE_ADDR) of the given event_location
    (which must be of type ADDRESS_LOCATION).  */
@@ -147,11 +163,9 @@ extern CORE_ADDR
 extern const char *
   get_address_string_location (const struct event_location *location);
 
-/* Create a new probe location.  The return result is malloc'd
-   and should be freed with delete_event_location.  */
+/* Create a new probe location.  */
 
-extern struct event_location *
-  new_probe_location (const char *probe);
+extern event_location_up new_probe_location (const char *probe);
 
 /* Return the probe location (a string) of the given event_location
    (which must be of type PROBE_LOCATION).  */
@@ -165,12 +179,9 @@ extern void
   initialize_explicit_location (struct explicit_location *explicit_loc);
 
 /* Create a new explicit location.  If not NULL, EXPLICIT is checked for
-   validity.  If invalid, an exception is thrown.
+   validity.  If invalid, an exception is thrown.  */
 
-   The return result is malloc'd and should be freed with
-   delete_event_location.  */
-
-extern struct event_location *
+extern event_location_up
   new_explicit_location (const struct explicit_location *explicit_loc);
 
 /* Return the explicit location of the given event_location
@@ -184,18 +195,9 @@ extern struct explicit_location *
 extern const struct explicit_location *
   get_explicit_location_const (const struct event_location *location);
 
-/* Free an event location and any associated data.  */
-
-extern void delete_event_location (struct event_location *location);
-
-/* Make a cleanup to free LOCATION.  */
-
-extern struct cleanup *
-  make_cleanup_delete_event_location (struct event_location *location);
-
 /* Return a copy of the given SRC location.  */
 
-extern struct event_location *
+extern event_location_up
   copy_event_location (const struct event_location *src);
 
 /* Attempt to convert the input string in *ARGP into an event_location.
@@ -207,21 +209,19 @@ extern struct event_location *
    but invalid, input, e.g., if it is called with missing argument parameters
    or invalid options.
 
-   The return result must be freed with delete_event_location.
-
    This function is intended to be used by CLI commands and will parse
    explicit locations in a CLI-centric way.  Other interfaces should use
    string_to_event_location_basic if they want to maintain support for
    legacy specifications of probe, address, and linespec locations.  */
 
-extern struct event_location *
+extern event_location_up
   string_to_event_location (char **argp,
 			    const struct language_defn *langauge);
 
 /* Like string_to_event_location, but does not attempt to parse explicit
    locations.  */
 
-extern struct event_location *
+extern event_location_up
   string_to_event_location_basic (char **argp,
 				  const struct language_defn *language);
 
@@ -235,7 +235,7 @@ extern struct event_location *
    parameters or invalid options.  If DONT_THROW is non-zero, this function
    will not throw any exceptions.  */
 
-extern struct event_location *
+extern event_location_up
   string_to_explicit_location (const char **argp,
 			       const struct language_defn *langauge,
 			       int dont_throw);
diff --git a/gdb/mi/mi-cmd-break.c b/gdb/mi/mi-cmd-break.c
index 8cc6034..0780f7b 100644
--- a/gdb/mi/mi-cmd-break.c
+++ b/gdb/mi/mi-cmd-break.c
@@ -180,7 +180,7 @@ mi_cmd_break_insert_1 (int dprintf, const char *command, char **argv, int argc)
   int tracepoint = 0;
   struct cleanup *back_to = make_cleanup (null_cleanup, NULL);
   enum bptype type_wanted;
-  struct event_location *location;
+  event_location_up location;
   struct breakpoint_ops *ops;
   int is_explicit = 0;
   struct explicit_location explicit_loc;
@@ -343,15 +343,10 @@ mi_cmd_break_insert_1 (int dprintf, const char *command, char **argv, int argc)
     {
       location = string_to_event_location_basic (&address, current_language);
       if (*address)
-	{
-	  delete_event_location (location);
-	  error (_("Garbage '%s' at end of location"), address);
-	}
+	error (_("Garbage '%s' at end of location"), address);
     }
 
-  make_cleanup_delete_event_location (location);
-
-  create_breakpoint (get_current_arch (), location, condition, thread,
+  create_breakpoint (get_current_arch (), location.get (), condition, thread,
 		     extra_string,
 		     0 /* condition and thread are valid.  */,
 		     temp_p, type_wanted,
diff --git a/gdb/probe.c b/gdb/probe.c
index c147810..4f4a5a3 100644
--- a/gdb/probe.c
+++ b/gdb/probe.c
@@ -205,7 +205,7 @@ parse_probes (const struct event_location *location,
       make_cleanup (xfree, canon);
       canonical->special_display = 1;
       canonical->pre_expanded = 1;
-      canonical->location = new_probe_location (canon);
+      canonical->location = new_probe_location (canon).release ();
     }
 
   do_cleanups (cleanup);
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index 6bf517c..d63f458 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -680,22 +680,16 @@ bppy_init (PyObject *self, PyObject *args, PyObject *kwargs)
 	{
 	case bp_breakpoint:
 	  {
-	    struct event_location *location;
-	    struct cleanup *cleanup;
-
-	    location
+	    event_location_up location
 	      = string_to_event_location_basic (&copy, current_language);
-	    cleanup = make_cleanup_delete_event_location (location);
 	    create_breakpoint (python_gdbarch,
-			       location, NULL, -1, NULL,
+			       location.get (), NULL, -1, NULL,
 			       0,
 			       temporary_bp, bp_breakpoint,
 			       0,
 			       AUTO_BOOLEAN_TRUE,
 			       &bkpt_breakpoint_ops,
 			       0, 1, internal_bp, 0);
-
-	    do_cleanups (cleanup);
 	    break;
 	  }
         case bp_watchpoint:
diff --git a/gdb/python/py-finishbreakpoint.c b/gdb/python/py-finishbreakpoint.c
index 797ca84..204d818 100644
--- a/gdb/python/py-finishbreakpoint.c
+++ b/gdb/python/py-finishbreakpoint.c
@@ -293,14 +293,11 @@ bpfinishpy_init (PyObject *self, PyObject *args, PyObject *kwargs)
 
   TRY
     {
-      struct event_location *location;
-      struct cleanup *back_to;
-
       /* Set a breakpoint on the return address.  */
-      location = new_address_location (get_frame_pc (prev_frame), NULL, 0);
-      back_to = make_cleanup_delete_event_location (location);
+      event_location_up location
+	= new_address_location (get_frame_pc (prev_frame), NULL, 0);
       create_breakpoint (python_gdbarch,
-                         location, NULL, thread, NULL,
+                         location.get (), NULL, thread, NULL,
                          0,
                          1 /*temp_flag*/,
                          bp_breakpoint,
@@ -308,7 +305,6 @@ bpfinishpy_init (PyObject *self, PyObject *args, PyObject *kwargs)
                          AUTO_BOOLEAN_TRUE,
                          &bkpt_breakpoint_ops,
                          0, 1, internal_bp, 0);
-      do_cleanups (back_to);
     }
   CATCH (except, RETURN_MASK_ALL)
     {
diff --git a/gdb/python/python.c b/gdb/python/python.c
index e0e24ac..ec202de 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -672,7 +672,7 @@ gdbpy_decode_line (PyObject *self, PyObject *args)
   struct cleanup *cleanups;
   gdbpy_ref<> result;
   gdbpy_ref<> unparsed;
-  struct event_location *location = NULL;
+  event_location_up location;
 
   if (! PyArg_ParseTuple (args, "|s", &arg))
     return NULL;
@@ -682,15 +682,12 @@ gdbpy_decode_line (PyObject *self, PyObject *args)
   sals.sals = NULL;
 
   if (arg != NULL)
-    {
-      location = string_to_event_location_basic (&arg, python_language);
-      make_cleanup_delete_event_location (location);
-    }
+    location = string_to_event_location_basic (&arg, python_language);
 
   TRY
     {
       if (location != NULL)
-	sals = decode_line_1 (location, 0, NULL, NULL, 0);
+	sals = decode_line_1 (location.get (), 0, NULL, NULL, 0);
       else
 	{
 	  set_default_source_symtab_and_line ();
diff --git a/gdb/spu-tdep.c b/gdb/spu-tdep.c
index 00e59c2..91b1a0e 100644
--- a/gdb/spu-tdep.c
+++ b/gdb/spu-tdep.c
@@ -1934,8 +1934,6 @@ spu_catch_start (struct objfile *objfile)
   struct bound_minimal_symbol minsym;
   struct compunit_symtab *cust;
   CORE_ADDR pc;
-  struct event_location *location;
-  struct cleanup *back_to;
 
   /* Do this only if requested by "set spu stop-on-load on".  */
   if (!spu_stop_on_load_p)
@@ -1979,9 +1977,8 @@ spu_catch_start (struct objfile *objfile)
 
   /* Use a numerical address for the set_breakpoint command to avoid having
      the breakpoint re-set incorrectly.  */
-  location = new_address_location (pc, NULL, 0);
-  back_to = make_cleanup_delete_event_location (location);
-  create_breakpoint (get_objfile_arch (objfile), location,
+  event_location_up location = new_address_location (pc, NULL, 0);
+  create_breakpoint (get_objfile_arch (objfile), location.get (),
 		     NULL /* cond_string */, -1 /* thread */,
 		     NULL /* extra_string */,
 		     0 /* parse_condition_and_thread */, 1 /* tempflag */,
@@ -1990,7 +1987,6 @@ spu_catch_start (struct objfile *objfile)
 		     AUTO_BOOLEAN_FALSE /* pending_break_support */,
 		     &bkpt_breakpoint_ops /* ops */, 0 /* from_tty */,
 		     1 /* enabled */, 0 /* internal  */, 0);
-  do_cleanups (back_to);
 }
 
 
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index c28aa38..dac1657 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -2591,20 +2591,18 @@ scope_info (char *args, int from_tty)
   int j, count = 0;
   struct gdbarch *gdbarch;
   int regno;
-  struct event_location *location;
-  struct cleanup *back_to;
 
   if (args == 0 || *args == 0)
     error (_("requires an argument (function, "
 	     "line or *addr) to define a scope"));
 
-  location = string_to_event_location (&args, current_language);
-  back_to = make_cleanup_delete_event_location (location);
-  sals = decode_line_1 (location, DECODE_LINE_FUNFIRSTLINE, NULL, NULL, 0);
+  event_location_up location = string_to_event_location (&args,
+							 current_language);
+  sals = decode_line_1 (location.get (), DECODE_LINE_FUNFIRSTLINE,
+			NULL, NULL, 0);
   if (sals.nelts == 0)
     {
       /* Presumably decode_line_1 has already warned.  */
-      do_cleanups (back_to);
       return;
     }
 
@@ -2743,7 +2741,6 @@ scope_info (char *args, int from_tty)
   if (count <= 0)
     printf_filtered ("Scope for %s contains no locals or arguments.\n",
 		     save_args);
-  do_cleanups (back_to);
 }
 
 /* Helper for trace_dump_command.  Dump the action list starting at
-- 
2.9.3

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

* [RFA v2 09/17] Remove some cleanups from location.c
  2017-04-11 15:02 [RFA v2 00/17] miscellaneous C++-ification Tom Tromey
@ 2017-04-11 15:01 ` Tom Tromey
  2017-04-11 15:01 ` [RFA v2 10/17] C++ify mi_parse Tom Tromey
                   ` (16 subsequent siblings)
  17 siblings, 0 replies; 36+ messages in thread
From: Tom Tromey @ 2017-04-11 15:01 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This removes some more cleanups from location.c by using
unique_xmalloc_ptr.

gdb/ChangeLog
2017-04-11  Tom Tromey  <tom@tromey.com>

	* location.c (explicit_location_lex_one): Return a
	unique_xmalloc_ptr.
	(string_to_explicit_location): Update.  Remove cleanups.
---
 gdb/ChangeLog  |  6 ++++++
 gdb/location.c | 64 ++++++++++++++++++++++++----------------------------------
 2 files changed, 32 insertions(+), 38 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index d47f796..f0c9a7b 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
 2017-04-11  Tom Tromey  <tom@tromey.com>
 
+	* location.c (explicit_location_lex_one): Return a
+	unique_xmalloc_ptr.
+	(string_to_explicit_location): Update.  Remove cleanups.
+
+2017-04-11  Tom Tromey  <tom@tromey.com>
+
 	* gnu-v3-abi.c (value_and_voffset_p): Remove typedef.
 	(compare_value_and_voffset): Change type.  Update.
 	(compute_vtable_size): Change type of "offset_vec".
diff --git a/gdb/location.c b/gdb/location.c
index 877c1ed..8aa8bd5 100644
--- a/gdb/location.c
+++ b/gdb/location.c
@@ -420,7 +420,7 @@ event_location_to_string (struct event_location *location)
    past any strings that it lexes.  Returns a malloc'd copy of the
    lexed string or NULL if no lexing was done.  */
 
-static char *
+static gdb::unique_xmalloc_ptr<char>
 explicit_location_lex_one (const char **inp,
 			   const struct language_defn *language)
 {
@@ -444,7 +444,8 @@ explicit_location_lex_one (const char **inp,
 	  if (end == NULL)
 	    error (_("Unmatched quote, %s."), start);
 	  *inp = end + 1;
-	  return savestring (start + 1, *inp - start - 2);
+	  return gdb::unique_xmalloc_ptr<char> (savestring (start + 1,
+							    *inp - start - 2));
 	}
     }
 
@@ -461,7 +462,8 @@ explicit_location_lex_one (const char **inp,
       while (isdigit (*inp[0]))
 	++(*inp);
       if (*inp[0] == '\0' || isspace (*inp[0]) || *inp[0] == ',')
-	return savestring (start, *inp - start);
+	return gdb::unique_xmalloc_ptr<char> (savestring (start,
+							  *inp - start));
 
       /* Otherwise stop at the next occurrence of whitespace, '\0',
 	 keyword, or ','.  */
@@ -480,7 +482,7 @@ explicit_location_lex_one (const char **inp,
     }
 
   if (*inp - start > 0)
-    return savestring (start, *inp - start);
+    return gdb::unique_xmalloc_ptr<char> (savestring (start, *inp - start));
 
   return NULL;
 }
@@ -511,9 +513,7 @@ string_to_explicit_location (const char **argp,
   while ((*argp)[0] != '\0' && (*argp)[0] != ',')
     {
       int len;
-      char *opt, *oarg;
       const char *start;
-      struct cleanup *opt_cleanup, *oarg_cleanup;
 
       /* If *ARGP starts with a keyword, stop processing
 	 options.  */
@@ -524,44 +524,40 @@ string_to_explicit_location (const char **argp,
       start = *argp;
 
       /* Get the option string.  */
-      opt = explicit_location_lex_one (argp, language);
-      opt_cleanup = make_cleanup (xfree, opt);
+      gdb::unique_xmalloc_ptr<char> opt
+	= explicit_location_lex_one (argp, language);
 
       *argp = skip_spaces_const (*argp);
 
       /* Get the argument string.  */
-      oarg = explicit_location_lex_one (argp, language);
-      oarg_cleanup = make_cleanup (xfree, oarg);
+      gdb::unique_xmalloc_ptr<char> oarg
+	= explicit_location_lex_one (argp, language);
+      bool have_oarg = oarg != NULL;
       *argp = skip_spaces_const (*argp);
 
       /* Use the length of the option to allow abbreviations.  */
-      len = strlen (opt);
+      len = strlen (opt.get ());
 
       /* All options have a required argument.  Checking for this required
 	 argument is deferred until later.  */
-      if (strncmp (opt, "-source", len) == 0)
-	EL_EXPLICIT (location)->source_filename = oarg;
-      else if (strncmp (opt, "-function", len) == 0)
-	EL_EXPLICIT (location)->function_name = oarg;
-      else if (strncmp (opt, "-line", len) == 0)
+      if (strncmp (opt.get (), "-source", len) == 0)
+	EL_EXPLICIT (location)->source_filename = oarg.release ();
+      else if (strncmp (opt.get (), "-function", len) == 0)
+	EL_EXPLICIT (location)->function_name = oarg.release ();
+      else if (strncmp (opt.get (), "-line", len) == 0)
 	{
-	  if (oarg != NULL)
-	    {
-	      EL_EXPLICIT (location)->line_offset
-		= linespec_parse_line_offset (oarg);
-	      do_cleanups (oarg_cleanup);
-	      do_cleanups (opt_cleanup);
-	      continue;
-	    }
+	  if (have_oarg)
+	    EL_EXPLICIT (location)->line_offset
+	      = linespec_parse_line_offset (oarg.get ());
 	}
-      else if (strncmp (opt, "-label", len) == 0)
-	EL_EXPLICIT (location)->label_name = oarg;
+      else if (strncmp (opt.get (), "-label", len) == 0)
+	EL_EXPLICIT (location)->label_name = oarg.release ();
       /* Only emit an "invalid argument" error for options
 	 that look like option strings.  */
-      else if (opt[0] == '-' && !isdigit (opt[1]))
+      else if (opt.get ()[0] == '-' && !isdigit (opt.get ()[1]))
 	{
 	  if (!dont_throw)
-	    error (_("invalid explicit location argument, \"%s\""), opt);
+	    error (_("invalid explicit location argument, \"%s\""), opt.get ());
 	}
       else
 	{
@@ -569,8 +565,6 @@ string_to_explicit_location (const char **argp,
 	     Stop parsing and return whatever explicit location was
 	     parsed.  */
 	  *argp = start;
-	  discard_cleanups (oarg_cleanup);
-	  do_cleanups (opt_cleanup);
 	  return location;
 	}
 
@@ -578,14 +572,8 @@ string_to_explicit_location (const char **argp,
 	 case, it provides a much better user experience to issue
 	 the "invalid argument" error before any missing
 	 argument error.  */
-      if (oarg == NULL && !dont_throw)
-	error (_("missing argument for \"%s\""), opt);
-
-      /* The option/argument pair was successfully processed;
-	 oarg belongs to the explicit location, and opt should
-	 be freed.  */
-      discard_cleanups (oarg_cleanup);
-      do_cleanups (opt_cleanup);
+      if (!have_oarg && !dont_throw)
+	error (_("missing argument for \"%s\""), opt.get ());
     }
 
   /* One special error check:  If a source filename was given
-- 
2.9.3

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

* [RFA v2 04/17] Introduce gdb_dlhandle_up
  2017-04-11 15:02 [RFA v2 00/17] miscellaneous C++-ification Tom Tromey
                   ` (7 preceding siblings ...)
  2017-04-11 15:01 ` [RFA v2 01/17] Introduce event_location_up Tom Tromey
@ 2017-04-11 15:01 ` Tom Tromey
  2017-04-11 15:01 ` [RFA v2 02/17] Introduce command_line_up Tom Tromey
                   ` (8 subsequent siblings)
  17 siblings, 0 replies; 36+ messages in thread
From: Tom Tromey @ 2017-04-11 15:01 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This introduces gdb_dlhandle_up, a unique_ptr that can close a
dlopen'd library.  All the functions working with dlopen handles are
updated to use this new type.

I did not try to build this on Windows.

gdb/ChangeLog
2017-04-11  Tom Tromey  <tom@tromey.com>

	* jit.c (struct jit_reader): Declare separately.  Add constructor
	and destructor.  Change type of "handle".
	(loaded_jit_reader): Define separately.
	(jit_reader_load): Update.  New "new".
	(jit_reader_unload_command): Use "delete".
	* gdb-dlfcn.h (struct dlclose_deleter): New.
	(gdb_dlhandle_up): New typedef.
	(gdb_dlopen, gdb_dlsym): Update types.
	(gdb_dlclose): Remove.
	* gdb-dlfcn.c (gdb_dlopen): Return a gdb_dlhandle_up.
	(gdb_dlsym): Change type of "handle".
	(make_cleanup_dlclose): Remove.
	(dlclose_deleter::operator()): Rename from gdb_dlclose.
	* compile/compile-c-support.c (load_libcc): Update.
---
 gdb/ChangeLog                   | 17 ++++++++++++++++
 gdb/compile/compile-c-support.c |  6 ++++--
 gdb/gdb-dlfcn.c                 | 45 ++++++++++++-----------------------------
 gdb/gdb-dlfcn.h                 | 24 ++++++++++++----------
 gdb/jit.c                       | 42 +++++++++++++++++++++-----------------
 5 files changed, 70 insertions(+), 64 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index a71de4a..a888174 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,22 @@
 2017-04-11  Tom Tromey  <tom@tromey.com>
 
+	* jit.c (struct jit_reader): Declare separately.  Add constructor
+	and destructor.  Change type of "handle".
+	(loaded_jit_reader): Define separately.
+	(jit_reader_load): Update.  New "new".
+	(jit_reader_unload_command): Use "delete".
+	* gdb-dlfcn.h (struct dlclose_deleter): New.
+	(gdb_dlhandle_up): New typedef.
+	(gdb_dlopen, gdb_dlsym): Update types.
+	(gdb_dlclose): Remove.
+	* gdb-dlfcn.c (gdb_dlopen): Return a gdb_dlhandle_up.
+	(gdb_dlsym): Change type of "handle".
+	(make_cleanup_dlclose): Remove.
+	(dlclose_deleter::operator()): Rename from gdb_dlclose.
+	* compile/compile-c-support.c (load_libcc): Update.
+
+2017-04-11  Tom Tromey  <tom@tromey.com>
+
 	* symtab.h (find_pcs_for_symtab_line): Change return type.
 	* symtab.c (find_pcs_for_symtab_line): Change return type.
 	* python/py-linetable.c (build_line_table_tuple_from_pcs): Change
diff --git a/gdb/compile/compile-c-support.c b/gdb/compile/compile-c-support.c
index c969b42..2614570 100644
--- a/gdb/compile/compile-c-support.c
+++ b/gdb/compile/compile-c-support.c
@@ -75,12 +75,11 @@ c_get_range_decl_name (const struct dynamic_prop *prop)
 static gcc_c_fe_context_function *
 load_libcc (void)
 {
-  void *handle;
   gcc_c_fe_context_function *func;
 
    /* gdb_dlopen will call error () on an error, so no need to check
       value.  */
-  handle = gdb_dlopen (STRINGIFY (GCC_C_FE_LIBCC));
+  gdb_dlhandle_up handle = gdb_dlopen (STRINGIFY (GCC_C_FE_LIBCC));
   func = (gcc_c_fe_context_function *) gdb_dlsym (handle,
 						  STRINGIFY (GCC_C_FE_CONTEXT));
 
@@ -88,6 +87,9 @@ load_libcc (void)
     error (_("could not find symbol %s in library %s"),
 	   STRINGIFY (GCC_C_FE_CONTEXT),
 	   STRINGIFY (GCC_C_FE_LIBCC));
+
+  /* Leave the library open.  */
+  handle.release ();
   return func;
 }
 
diff --git a/gdb/gdb-dlfcn.c b/gdb/gdb-dlfcn.c
index a11e396..7485a38 100644
--- a/gdb/gdb-dlfcn.c
+++ b/gdb/gdb-dlfcn.c
@@ -31,27 +31,20 @@
 
 #ifdef NO_SHARED_LIB
 
-void *
+gdb_dlhandle_up
 gdb_dlopen (const char *filename)
 {
   gdb_assert_not_reached ("gdb_dlopen should not be called on this platform.");
 }
 
 void *
-gdb_dlsym (void *handle, const char *symbol)
+gdb_dlsym (const gdb_dlhandle_up &handle, const char *symbol)
 {
   gdb_assert_not_reached ("gdb_dlsym should not be called on this platform.");
 }
 
-struct cleanup *
-make_cleanup_dlclose (void *handle)
-{
-  gdb_assert_not_reached ("make_cleanup_dlclose should not be called on this "
-                          "platform.");
-}
-
-int
-gdb_dlclose (void *handle)
+void
+dlclose_deleter::operator() (void *handle) const
 {
   gdb_assert_not_reached ("gdb_dlclose should not be called on this platform.");
 }
@@ -64,7 +57,7 @@ is_dl_available (void)
 
 #else /* NO_SHARED_LIB */
 
-void *
+gdb_dlhandle_up
 gdb_dlopen (const char *filename)
 {
   void *result;
@@ -74,7 +67,7 @@ gdb_dlopen (const char *filename)
   result = (void *) LoadLibrary (filename);
 #endif
   if (result != NULL)
-    return result;
+    return gdb_dlhandle_up (result);
 
 #ifdef HAVE_DLFCN_H
   error (_("Could not load %s: %s"), filename, dlerror());
@@ -97,37 +90,25 @@ gdb_dlopen (const char *filename)
 }
 
 void *
-gdb_dlsym (void *handle, const char *symbol)
+gdb_dlsym (const gdb_dlhandle_up &handle, const char *symbol)
 {
 #ifdef HAVE_DLFCN_H
-  return dlsym (handle, symbol);
+  return dlsym (handle.get (), symbol);
 #elif __MINGW32__
-  return (void *) GetProcAddress ((HMODULE) handle, symbol);
+  return (void *) GetProcAddress ((HMODULE) handle.get (), symbol);
 #endif
 }
 
-int
-gdb_dlclose (void *handle)
+void
+dlclose_deleter::operator() (void *handle) const
 {
 #ifdef HAVE_DLFCN_H
-  return dlclose (handle);
+  dlclose (handle);
 #elif __MINGW32__
-  return !((int) FreeLibrary ((HMODULE) handle));
+  FreeLibrary ((HMODULE) handle);
 #endif
 }
 
-static void
-do_dlclose_cleanup (void *handle)
-{
-  gdb_dlclose (handle);
-}
-
-struct cleanup *
-make_cleanup_dlclose (void *handle)
-{
-  return make_cleanup (do_dlclose_cleanup, handle);
-}
-
 int
 is_dl_available (void)
 {
diff --git a/gdb/gdb-dlfcn.h b/gdb/gdb-dlfcn.h
index 31709a9..021f759 100644
--- a/gdb/gdb-dlfcn.h
+++ b/gdb/gdb-dlfcn.h
@@ -20,26 +20,28 @@
 #ifndef GDB_DLFCN_H
 #define GDB_DLFCN_H
 
+/* A deleter that closes an open dynamic library.  */
+
+struct dlclose_deleter
+{
+  void operator() (void *handle) const;
+};
+
+/* A unique pointer that points to a dynamic library.  */
+
+typedef std::unique_ptr<void, dlclose_deleter> gdb_dlhandle_up;
+
 /* Load the dynamic library file named FILENAME, and return a handle
    for that dynamic library.  Return NULL if the loading fails for any
    reason.  */
 
-void *gdb_dlopen (const char *filename);
+gdb_dlhandle_up gdb_dlopen (const char *filename);
 
 /* Return the address of the symbol named SYMBOL inside the shared
    library whose handle is HANDLE.  Return NULL when the symbol could
    not be found.  */
 
-void *gdb_dlsym (void *handle, const char *symbol);
-
-/* Install a cleanup routine which closes the handle HANDLE.  */
-
-struct cleanup *make_cleanup_dlclose (void *handle);
-
-/* Cleanup the shared object pointed to by HANDLE. Return 0 on success
-   and nonzero on failure.  */
-
-int gdb_dlclose (void *handle);
+void *gdb_dlsym (const gdb_dlhandle_up &handle, const char *symbol);
 
 /* Return non-zero if the dynamic library functions are available on
    this platform.  */
diff --git a/gdb/jit.c b/gdb/jit.c
index 158d6d8..ddf1005 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -151,14 +151,29 @@ bfd_open_from_target_memory (CORE_ADDR addr, ULONGEST size, char *target)
 			      mem_bfd_iovec_stat);
 }
 
+struct jit_reader
+{
+  jit_reader (struct gdb_reader_funcs *f, gdb_dlhandle_up &&h)
+    : functions (f), handle (std::move (h))
+  {
+  }
+
+  ~jit_reader ()
+  {
+    functions->destroy (functions);
+  }
+
+  jit_reader (const jit_reader &) = delete;
+  jit_reader &operator= (const jit_reader &) = delete;
+
+  struct gdb_reader_funcs *functions;
+  gdb_dlhandle_up handle;
+};
+
 /* One reader that has been loaded successfully, and can potentially be used to
    parse debug info.  */
 
-static struct jit_reader
-{
-  struct gdb_reader_funcs *functions;
-  void *handle;
-} *loaded_jit_reader = NULL;
+static struct jit_reader *loaded_jit_reader = NULL;
 
 typedef struct gdb_reader_funcs * (reader_init_fn_type) (void);
 static const char *reader_init_fn_sym = "gdb_init_reader";
@@ -168,17 +183,13 @@ static const char *reader_init_fn_sym = "gdb_init_reader";
 static struct jit_reader *
 jit_reader_load (const char *file_name)
 {
-  void *so;
   reader_init_fn_type *init_fn;
-  struct jit_reader *new_reader = NULL;
   struct gdb_reader_funcs *funcs = NULL;
-  struct cleanup *old_cleanups;
 
   if (jit_debug)
     fprintf_unfiltered (gdb_stdlog, _("Opening shared object %s.\n"),
                         file_name);
-  so = gdb_dlopen (file_name);
-  old_cleanups = make_cleanup_dlclose (so);
+  gdb_dlhandle_up so = gdb_dlopen (file_name);
 
   init_fn = (reader_init_fn_type *) gdb_dlsym (so, reader_init_fn_sym);
   if (!init_fn)
@@ -192,12 +203,7 @@ jit_reader_load (const char *file_name)
   if (funcs->reader_version != GDB_READER_INTERFACE_VERSION)
     error (_("Reader version does not match GDB version."));
 
-  new_reader = XCNEW (struct jit_reader);
-  new_reader->functions = funcs;
-  new_reader->handle = so;
-
-  discard_cleanups (old_cleanups);
-  return new_reader;
+  return new jit_reader (funcs, std::move (so));
 }
 
 /* Provides the jit-reader-load command.  */
@@ -240,10 +246,8 @@ jit_reader_unload_command (char *args, int from_tty)
 
   reinit_frame_cache ();
   jit_inferior_exit_hook (current_inferior ());
-  loaded_jit_reader->functions->destroy (loaded_jit_reader->functions);
 
-  gdb_dlclose (loaded_jit_reader->handle);
-  xfree (loaded_jit_reader);
+  delete loaded_jit_reader;
   loaded_jit_reader = NULL;
 }
 
-- 
2.9.3

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

* [RFA v2 16/17] Add a constructor and destructor to linespec_result
  2017-04-11 15:02 [RFA v2 00/17] miscellaneous C++-ification Tom Tromey
                   ` (2 preceding siblings ...)
  2017-04-11 15:01 ` [RFA v2 17/17] Change linespec_result::location to be an event_location_up Tom Tromey
@ 2017-04-11 15:01 ` Tom Tromey
  2017-04-12  2:25   ` Simon Marchi
  2017-04-11 15:01 ` [RFA v2 11/17] Use scoped_restore in more places Tom Tromey
                   ` (13 subsequent siblings)
  17 siblings, 1 reply; 36+ messages in thread
From: Tom Tromey @ 2017-04-11 15:01 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

linespec_result is only ever allocated on the stack, so it's
relatively easy to convert to having a constructor and a destructor.
This patch makes this change.  This removes some cleanups.

2017-04-11  Tom Tromey  <tom@tromey.com>

	* linespec.h (struct linespec_result): Add constructor and
	destructor.
	(init_linespec_result, destroy_linespec_result)
	(make_cleanup_destroy_linespec_result): Don't declare.
	* linespec.c (init_linespec_result): Remove.
	(linespec_result::~linespec_result): Rename from
	destroy_linespec_result.  Update.
	(cleanup_linespec_result, make_cleanup_destroy_linespec_result):
	Remove.
	* breakpoint.c (create_breakpoint, break_range_command)
	(decode_location_default): Update.
	* ax-gdb.c (agent_command_1): Update.
---
 gdb/ChangeLog    | 15 +++++++++++++++
 gdb/ax-gdb.c     |  5 +----
 gdb/breakpoint.c | 19 +------------------
 gdb/linespec.c   | 35 ++++-------------------------------
 gdb/linespec.h   | 30 ++++++++++++++----------------
 5 files changed, 35 insertions(+), 69 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 3faadc2..fc600b1 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,20 @@
 2017-04-11  Tom Tromey  <tom@tromey.com>
 
+	* linespec.h (struct linespec_result): Add constructor and
+	destructor.
+	(init_linespec_result, destroy_linespec_result)
+	(make_cleanup_destroy_linespec_result): Don't declare.
+	* linespec.c (init_linespec_result): Remove.
+	(linespec_result::~linespec_result): Rename from
+	destroy_linespec_result.  Update.
+	(cleanup_linespec_result, make_cleanup_destroy_linespec_result):
+	Remove.
+	* breakpoint.c (create_breakpoint, break_range_command)
+	(decode_location_default): Update.
+	* ax-gdb.c (agent_command_1): Update.
+
+2017-04-11  Tom Tromey  <tom@tromey.com>
+
 	* remote.c (remote_download_tracepoint): Update.
 	* python/py-breakpoint.c (bppy_get_location): Update.
 	* guile/scm-breakpoint.c (bpscm_print_breakpoint_smob)
diff --git a/gdb/ax-gdb.c b/gdb/ax-gdb.c
index 7206022..fae2e2d 100644
--- a/gdb/ax-gdb.c
+++ b/gdb/ax-gdb.c
@@ -2603,15 +2603,13 @@ agent_command_1 (char *exp, int eval)
       struct linespec_result canonical;
       int ix;
       struct linespec_sals *iter;
-      struct cleanup *old_chain;
 
       exp = skip_spaces (exp);
-      init_linespec_result (&canonical);
+
       event_location_up location = new_linespec_location (&exp);
       decode_line_full (location.get (), DECODE_LINE_FUNFIRSTLINE, NULL,
 			(struct symtab *) NULL, 0, &canonical,
 			NULL, NULL);
-      old_chain = make_cleanup_destroy_linespec_result (&canonical);
       exp = skip_spaces (exp);
       if (exp[0] == ',')
         {
@@ -2625,7 +2623,6 @@ agent_command_1 (char *exp, int eval)
 	  for (i = 0; i < iter->sals.nelts; i++)
 	    agent_eval_command_one (exp, eval, iter->sals.sals[i].pc);
         }
-      do_cleanups (old_chain);
     }
   else
     agent_eval_command_one (exp, eval, get_frame_pc (get_current_frame ()));
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index afc8309..42f344a 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -9726,7 +9726,6 @@ create_breakpoint (struct gdbarch *gdbarch,
 		   unsigned flags)
 {
   struct linespec_result canonical;
-  struct cleanup *old_chain;
   struct cleanup *bkpt_chain = NULL;
   int pending = 0;
   int task = 0;
@@ -9738,8 +9737,6 @@ create_breakpoint (struct gdbarch *gdbarch,
   if (extra_string != NULL && *extra_string == '\0')
     extra_string = NULL;
 
-  init_linespec_result (&canonical);
-
   TRY
     {
       ops->create_sals_from_location (location, &canonical, type_wanted);
@@ -9779,9 +9776,6 @@ create_breakpoint (struct gdbarch *gdbarch,
   if (!pending && VEC_empty (linespec_sals, canonical.sals))
     return 0;
 
-  /* Create a chain of things that always need to be cleaned up.  */
-  old_chain = make_cleanup_destroy_linespec_result (&canonical);
-
   /* ----------------------------- SNIP -----------------------------
      Anything added to the cleanup chain beyond this point is assumed
      to be part of a breakpoint.  If the breakpoint create succeeds
@@ -9922,8 +9916,6 @@ create_breakpoint (struct gdbarch *gdbarch,
   /* That's it.  Discard the cleanups for data inserted into the
      breakpoint.  */
   discard_cleanups (bkpt_chain);
-  /* But cleanup everything else.  */
-  do_cleanups (old_chain);
 
   /* error call may happen here - have BKPT_CHAIN already discarded.  */
   update_global_location_list (UGLL_MAY_INSERT);
@@ -10372,13 +10364,10 @@ break_range_command (char *arg, int from_tty)
   if (arg == NULL || arg[0] == '\0')
     error(_("No address range specified."));
 
-  init_linespec_result (&canonical_start);
-
   arg_start = arg;
   event_location_up start_location = string_to_event_location (&arg,
 							       current_language);
   parse_breakpoint_sals (start_location.get (), &canonical_start);
-  cleanup_bkpt = make_cleanup_destroy_linespec_result (&canonical_start);
 
   if (arg[0] != ',')
     error (_("Too few arguments."));
@@ -10393,14 +10382,13 @@ break_range_command (char *arg, int from_tty)
 
   sal_start = lsal_start->sals.sals[0];
   addr_string_start = savestring (arg_start, arg - arg_start);
-  make_cleanup (xfree, addr_string_start);
+  cleanup_bkpt = make_cleanup (xfree, addr_string_start);
 
   arg++;	/* Skip the comma.  */
   arg = skip_spaces (arg);
 
   /* Parse the end location.  */
 
-  init_linespec_result (&canonical_end);
   arg_start = arg;
 
   /* We call decode_line_full directly here instead of using
@@ -10414,8 +10402,6 @@ break_range_command (char *arg, int from_tty)
 		    sal_start.symtab, sal_start.line,
 		    &canonical_end, NULL, NULL);
 
-  make_cleanup_destroy_linespec_result (&canonical_end);
-
   if (VEC_empty (linespec_sals, canonical_end.sals))
     error (_("Could not find location of the end of the range."));
 
@@ -14487,7 +14473,6 @@ decode_location_default (struct breakpoint *b,
 {
   struct linespec_result canonical;
 
-  init_linespec_result (&canonical);
   decode_line_full (location, DECODE_LINE_FUNFIRSTLINE, search_pspace,
 		    (struct symtab *) NULL, 0,
 		    &canonical, multiple_symbols_all,
@@ -14506,8 +14491,6 @@ decode_location_default (struct breakpoint *b,
 	 contents.  */
       lsal->sals.sals = NULL;
     }
-
-  destroy_linespec_result (&canonical);
 }
 
 /* Prepare the global context for a re-set of breakpoint B.  */
diff --git a/gdb/linespec.c b/gdb/linespec.c
index bccabaf..51fa128 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -3887,45 +3887,18 @@ symbol_to_sal (struct symtab_and_line *result,
   return 0;
 }
 
-/* See the comment in linespec.h.  */
-
-void
-init_linespec_result (struct linespec_result *lr)
-{
-  memset (lr, 0, sizeof (*lr));
-}
-
-/* See the comment in linespec.h.  */
-
-void
-destroy_linespec_result (struct linespec_result *ls)
+linespec_result::~linespec_result ()
 {
   int i;
   struct linespec_sals *lsal;
 
-  delete_event_location (ls->location);
-  for (i = 0; VEC_iterate (linespec_sals, ls->sals, i, lsal); ++i)
+  delete_event_location (location);
+  for (i = 0; VEC_iterate (linespec_sals, sals, i, lsal); ++i)
     {
       xfree (lsal->canonical);
       xfree (lsal->sals.sals);
     }
-  VEC_free (linespec_sals, ls->sals);
-}
-
-/* Cleanup function for a linespec_result.  */
-
-static void
-cleanup_linespec_result (void *a)
-{
-  destroy_linespec_result ((struct linespec_result *) a);
-}
-
-/* See the comment in linespec.h.  */
-
-struct cleanup *
-make_cleanup_destroy_linespec_result (struct linespec_result *ls)
-{
-  return make_cleanup (cleanup_linespec_result, ls);
+  VEC_free (linespec_sals, sals);
 }
 
 /* Return the quote characters permitted by the linespec parser.  */
diff --git a/gdb/linespec.h b/gdb/linespec.h
index 4e69895..7459ff8 100644
--- a/gdb/linespec.h
+++ b/gdb/linespec.h
@@ -54,12 +54,23 @@ typedef struct linespec_sals linespec_sals;
 DEF_VEC_O (linespec_sals);
 
 /* An instance of this may be filled in by decode_line_1.  The caller
-   must call init_linespec_result to initialize it and
-   destroy_linespec_result to destroy it.  The caller must make copies
-   of any data that it needs to keep.  */
+   must make copies of any data that it needs to keep.  */
 
 struct linespec_result
 {
+  linespec_result ()
+    : special_display (0),
+      pre_expanded (0),
+      location (NULL),
+      sals (NULL)
+  {
+  }
+
+  ~linespec_result ();
+
+  linespec_result (const linespec_result &) = delete;
+  linespec_result &operator= (const linespec_result &) = delete;
+
   /* If non-zero, the linespec should be displayed to the user.  This
      is used by "unusual" linespecs where the ordinary `info break'
      display mechanism would do the wrong thing.  */
@@ -80,19 +91,6 @@ struct linespec_result
   VEC (linespec_sals) *sals;
 };
 
-/* Initialize a linespec_result.  */
-
-extern void init_linespec_result (struct linespec_result *);
-
-/* Destroy a linespec_result.  */
-
-extern void destroy_linespec_result (struct linespec_result *);
-
-/* Return a cleanup that destroys a linespec_result.  */
-
-extern struct cleanup *
-        make_cleanup_destroy_linespec_result (struct linespec_result *);
-
 /* Decode a linespec using the provided default symtab and line.  */
 
 extern struct symtabs_and_lines
-- 
2.9.3

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

* [RFA v2 12/17] Use std::vector in reread_symbols
  2017-04-11 15:02 [RFA v2 00/17] miscellaneous C++-ification Tom Tromey
                   ` (5 preceding siblings ...)
  2017-04-11 15:01 ` [RFA v2 05/17] Change increment_reading_symtab to return a scoped_restore Tom Tromey
@ 2017-04-11 15:01 ` Tom Tromey
  2017-04-11 15:01 ` [RFA v2 01/17] Introduce event_location_up Tom Tromey
                   ` (10 subsequent siblings)
  17 siblings, 0 replies; 36+ messages in thread
From: Tom Tromey @ 2017-04-11 15:01 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes reread_symbols to use std::vector, removing a cleanup.

gdb/ChangeLog
2017-04-11  Tom Tromey  <tom@tromey.com>

	* symfile.c (objfilep): Remove typedef.
	(reread_symbols): Use a std::vector.
---
 gdb/ChangeLog |  5 +++++
 gdb/symfile.c | 21 +++++----------------
 2 files changed, 10 insertions(+), 16 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index f582694..ca92e86 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2017-04-11  Tom Tromey  <tom@tromey.com>
 
+	* symfile.c (objfilep): Remove typedef.
+	(reread_symbols): Use a std::vector.
+
+2017-04-11  Tom Tromey  <tom@tromey.com>
+
 	* mi/mi-main.c (exec_direction_forward): Remove.
 	(exec_reverse_continue, mi_execute_command): Use scoped_restore.
 	* guile/scm-ports.c (ioscm_with_output_to_port_worker): Use
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 4d9fe54..0ef0029 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -2434,10 +2434,6 @@ remove_symbol_file_command (char *args, int from_tty)
   do_cleanups (my_cleanups);
 }
 
-typedef struct objfile *objfilep;
-
-DEF_VEC_P (objfilep);
-
 /* Re-read symbols if a symbol-file has changed.  */
 
 void
@@ -2447,10 +2443,7 @@ reread_symbols (void)
   long new_modtime;
   struct stat new_statbuf;
   int res;
-  VEC (objfilep) *new_objfiles = NULL;
-  struct cleanup *all_cleanups;
-
-  all_cleanups = make_cleanup (VEC_cleanup (objfilep), &new_objfiles);
+  std::vector<struct objfile *> new_objfiles;
 
   /* With the addition of shared libraries, this should be modified,
      the load time should be saved in the partial symbol tables, since
@@ -2661,14 +2654,12 @@ reread_symbols (void)
 	  objfile->mtime = new_modtime;
 	  init_entry_point_info (objfile);
 
-	  VEC_safe_push (objfilep, new_objfiles, objfile);
+	  new_objfiles.push_back (objfile);
 	}
     }
 
-  if (new_objfiles)
+  if (!new_objfiles.empty ())
     {
-      int ix;
-
       /* Notify objfiles that we've modified objfile sections.  */
       objfiles_changed ();
 
@@ -2677,15 +2668,13 @@ reread_symbols (void)
       /* clear_objfile_data for each objfile was called before freeing it and
 	 observer_notify_new_objfile (NULL) has been called by
 	 clear_symtab_users above.  Notify the new files now.  */
-      for (ix = 0; VEC_iterate (objfilep, new_objfiles, ix, objfile); ix++)
-	observer_notify_new_objfile (objfile);
+      for (auto iter : new_objfiles)
+	observer_notify_new_objfile (iter);
 
       /* At least one objfile has changed, so we can consider that
          the executable we're debugging has changed too.  */
       observer_notify_executable_changed ();
     }
-
-  do_cleanups (all_cleanups);
 }
 \f
 
-- 
2.9.3

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

* [RFA v2 02/17] Introduce command_line_up
  2017-04-11 15:02 [RFA v2 00/17] miscellaneous C++-ification Tom Tromey
                   ` (8 preceding siblings ...)
  2017-04-11 15:01 ` [RFA v2 04/17] Introduce gdb_dlhandle_up Tom Tromey
@ 2017-04-11 15:01 ` Tom Tromey
  2017-04-11 15:02 ` [RFA v2 08/17] Remove some cleanups from gnu-v3-abi.c Tom Tromey
                   ` (7 subsequent siblings)
  17 siblings, 0 replies; 36+ messages in thread
From: Tom Tromey @ 2017-04-11 15:01 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This introduces command_line_up, a unique_ptr for command_line
objects, and changes many places to use it.  This removes a number of
cleanups.

Command lines are funny in that sometimes they are reference counted.
Once there is more C++-ification of some of the users, perhaps all of
these can be changed to use shared_ptr instead.

gdb/ChangeLog
2017-04-11  Tom Tromey  <tom@tromey.com>

	* tracepoint.c (actions_command): Update.
	* python/python.c (python_command, python_interactive_command):
	Update.
	* mi/mi-cmd-break.c (mi_cmd_break_commands): Update.
	* guile/guile.c (guile_command): Update.
	* defs.h (read_command_lines, read_command_lines_1): Return
	command_line_up.
	(command_lines_deleter): New struct.
	(command_line_up): New typedef.
	* compile/compile.c (compile_code_command)
	(compile_print_command): Update.
	* cli/cli-script.h (get_command_line, copy_command_lines): Return
	command_line_up.
	(make_cleanup_free_command_lines): Remove.
	* cli/cli-script.c (get_command_line, read_command_lines_1)
	(copy_command_lines): Return command_line_up.
	(while_command, if_command, read_command_lines, define_command)
	(document_command): Update.
	(do_free_command_lines_cleanup, make_cleanup_free_command_lines):
	Remove.
	* breakpoint.h (breakpoint_set_commands): Change type of
	"commands".
	* breakpoint.c (breakpoint_set_commands): Change type of
	"commands".  Update.
	(do_map_commands_command, update_dprintf_command_list)
	(create_tracepoint_from_upload): Update.
---
 gdb/ChangeLog         | 29 ++++++++++++++++++
 gdb/breakpoint.c      | 16 +++++-----
 gdb/breakpoint.h      |  2 +-
 gdb/cli/cli-script.c  | 85 ++++++++++++++++-----------------------------------
 gdb/cli/cli-script.h  |  9 ++----
 gdb/compile/compile.c | 12 +++-----
 gdb/defs.h            | 28 ++++++++++++-----
 gdb/guile/guile.c     | 11 +++----
 gdb/mi/mi-cmd-break.c |  4 +--
 gdb/python/python.c   | 12 +++-----
 gdb/tracepoint.c      | 17 +++++------
 11 files changed, 109 insertions(+), 116 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index f7aefaa..7fa5099 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,34 @@
 2017-04-11  Tom Tromey  <tom@tromey.com>
 
+	* tracepoint.c (actions_command): Update.
+	* python/python.c (python_command, python_interactive_command):
+	Update.
+	* mi/mi-cmd-break.c (mi_cmd_break_commands): Update.
+	* guile/guile.c (guile_command): Update.
+	* defs.h (read_command_lines, read_command_lines_1): Return
+	command_line_up.
+	(command_lines_deleter): New struct.
+	(command_line_up): New typedef.
+	* compile/compile.c (compile_code_command)
+	(compile_print_command): Update.
+	* cli/cli-script.h (get_command_line, copy_command_lines): Return
+	command_line_up.
+	(make_cleanup_free_command_lines): Remove.
+	* cli/cli-script.c (get_command_line, read_command_lines_1)
+	(copy_command_lines): Return command_line_up.
+	(while_command, if_command, read_command_lines, define_command)
+	(document_command): Update.
+	(do_free_command_lines_cleanup, make_cleanup_free_command_lines):
+	Remove.
+	* breakpoint.h (breakpoint_set_commands): Change type of
+	"commands".
+	* breakpoint.c (breakpoint_set_commands): Change type of
+	"commands".  Update.
+	(do_map_commands_command, update_dprintf_command_list)
+	(create_tracepoint_from_upload): Update.
+
+2017-04-11  Tom Tromey  <tom@tromey.com>
+
 	* tracepoint.c (scope_info): Update.
 	* spu-tdep.c (spu_catch_start): Update.
 	* python/python.c (gdbpy_decode_line): Update.
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 9f7db91..3a3cd80 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1271,12 +1271,12 @@ static_tracepoints_here (CORE_ADDR addr)
 
 void
 breakpoint_set_commands (struct breakpoint *b, 
-			 struct command_line *commands)
+			 command_line_up &&commands)
 {
-  validate_commands_for_breakpoint (b, commands);
+  validate_commands_for_breakpoint (b, commands.get ());
 
   decref_counted_command_line (&b->commands);
-  b->commands = alloc_counted_command_line (commands);
+  b->commands = alloc_counted_command_line (commands.release ());
   observer_notify_breakpoint_modified (b);
 }
 
@@ -1358,7 +1358,7 @@ do_map_commands_command (struct breakpoint *b, void *data)
 
   if (info->cmd == NULL)
     {
-      struct command_line *l;
+      command_line_up l;
 
       if (info->control != NULL)
 	l = copy_command_lines (info->control->body_list[0]);
@@ -1382,7 +1382,7 @@ do_map_commands_command (struct breakpoint *b, void *data)
 	  do_cleanups (old_chain);
 	}
 
-      info->cmd = alloc_counted_command_line (l);
+      info->cmd = alloc_counted_command_line (l.release ());
     }
 
   /* If a breakpoint was on the list more than once, we don't need to
@@ -9191,7 +9191,7 @@ update_dprintf_command_list (struct breakpoint *b)
     printf_cmd_line->next = NULL;
     printf_cmd_line->line = printf_line;
 
-    breakpoint_set_commands (b, printf_cmd_line);
+    breakpoint_set_commands (b, command_line_up (printf_cmd_line));
   }
 }
 
@@ -15356,14 +15356,14 @@ create_tracepoint_from_upload (struct uploaded_tp *utp)
      function.  */
   if (!VEC_empty (char_ptr, utp->cmd_strings))
     {
-      struct command_line *cmd_list;
+      command_line_up cmd_list;
 
       this_utp = utp;
       next_cmd = 0;
 
       cmd_list = read_command_lines_1 (read_uploaded_action, 1, NULL, NULL);
 
-      breakpoint_set_commands (&tp->base, cmd_list);
+      breakpoint_set_commands (&tp->base, std::move (cmd_list));
     }
   else if (!VEC_empty (char_ptr, utp->actions)
 	   || !VEC_empty (char_ptr, utp->step_actions))
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index a7f2128..1301fb4 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -1475,7 +1475,7 @@ extern void disable_breakpoint (struct breakpoint *);
 extern void enable_breakpoint (struct breakpoint *);
 
 extern void breakpoint_set_commands (struct breakpoint *b, 
-				     struct command_line *commands);
+				     command_line_up &&commands);
 
 extern void breakpoint_set_silent (struct breakpoint *b, int silent);
 
diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
index 33b657d..e0e27ef 100644
--- a/gdb/cli/cli-script.c
+++ b/gdb/cli/cli-script.c
@@ -165,27 +165,20 @@ build_command_line (enum command_control_type type, const char *args)
 /* Build and return a new command structure for the control commands
    such as "if" and "while".  */
 
-struct command_line *
+command_line_up
 get_command_line (enum command_control_type type, const char *arg)
 {
-  struct command_line *cmd;
-  struct cleanup *old_chain = NULL;
-
   /* Allocate and build a new command line structure.  */
-  cmd = build_command_line (type, arg);
-
-  old_chain = make_cleanup_free_command_lines (&cmd);
+  command_line_up cmd (build_command_line (type, arg));
 
   /* Read in the body of this command.  */
-  if (recurse_read_control_structure (read_next_line, cmd, 0, 0)
+  if (recurse_read_control_structure (read_next_line, cmd.get (), 0, 0)
       == invalid_control)
     {
       warning (_("Error reading in canned sequence of commands."));
-      do_cleanups (old_chain);
       return NULL;
     }
 
-  discard_cleanups (old_chain);
   return cmd;
 }
 
@@ -677,18 +670,15 @@ execute_control_command_untraced (struct command_line *cmd)
 static void
 while_command (char *arg, int from_tty)
 {
-  struct command_line *command = NULL;
-
   control_level = 1;
-  command = get_command_line (while_control, arg);
+  command_line_up command = get_command_line (while_control, arg);
 
   if (command == NULL)
     return;
 
   scoped_restore save_async = make_scoped_restore (&current_ui->async, 0);
 
-  execute_control_command_untraced (command);
-  free_command_lines (&command);
+  execute_control_command_untraced (command.get ());
 }
 
 /* "if" command support.  Execute either the true or false arm depending
@@ -697,19 +687,15 @@ while_command (char *arg, int from_tty)
 static void
 if_command (char *arg, int from_tty)
 {
-  struct command_line *command = NULL;
-  struct cleanup *old_chain;
-
   control_level = 1;
-  command = get_command_line (if_control, arg);
+  command_line_up command = get_command_line (if_control, arg);
 
   if (command == NULL)
     return;
 
   scoped_restore save_async = make_scoped_restore (&current_ui->async, 0);
 
-  execute_control_command_untraced (command);
-  free_command_lines (&command);
+  execute_control_command_untraced (command.get ());
 }
 
 /* Bind the incoming arguments for a user defined command to $arg0,
@@ -1208,12 +1194,10 @@ restore_interp (void *arg)
 
 #define END_MESSAGE "End with a line saying just \"end\"."
 
-struct command_line *
+command_line_up
 read_command_lines (char *prompt_arg, int from_tty, int parse_commands,
 		    void (*validator)(char *, void *), void *closure)
 {
-  struct command_line *head;
-
   if (from_tty && input_interactive_p (current_ui))
     {
       if (deprecated_readline_begin_hook)
@@ -1232,6 +1216,7 @@ read_command_lines (char *prompt_arg, int from_tty, int parse_commands,
 
   /* Reading commands assumes the CLI behavior, so temporarily
      override the current interpreter with CLI.  */
+  command_line_up head;
   if (current_interp_named_p (INTERP_CONSOLE))
     head = read_command_lines_1 (read_next_line, parse_commands,
 				 validator, closure);
@@ -1256,17 +1241,17 @@ read_command_lines (char *prompt_arg, int from_tty, int parse_commands,
 /* Act the same way as read_command_lines, except that each new line is
    obtained using READ_NEXT_LINE_FUNC.  */
 
-struct command_line *
+command_line_up
 read_command_lines_1 (char * (*read_next_line_func) (void), int parse_commands,
 		      void (*validator)(char *, void *), void *closure)
 {
-  struct command_line *head, *tail, *next;
-  struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
+  struct command_line *tail, *next;
+  command_line_up head;
   enum command_control_type ret;
   enum misc_command_type val;
 
   control_level = 0;
-  head = tail = NULL;
+  tail = NULL;
 
   while (1)
     {
@@ -1307,18 +1292,15 @@ read_command_lines_1 (char * (*read_next_line_func) (void), int parse_commands,
 	}
       else
 	{
-	  head = next;
-	  make_cleanup_free_command_lines (&head);
+	  head.reset (next);
 	}
       tail = next;
     }
 
   dont_repeat ();
 
-  if (ret != invalid_control)
-    discard_cleanups (old_chain);
-  else
-    do_cleanups (old_chain);
+  if (ret == invalid_control)
+    return NULL;
 
   return head;
 }
@@ -1349,19 +1331,7 @@ free_command_lines (struct command_line **lptr)
   *lptr = NULL;
 }
 
-static void
-do_free_command_lines_cleanup (void *arg)
-{
-  free_command_lines ((struct command_line **) arg);
-}
-
-struct cleanup *
-make_cleanup_free_command_lines (struct command_line **arg)
-{
-  return make_cleanup (do_free_command_lines_cleanup, arg);
-}
-
-struct command_line *
+command_line_up
 copy_command_lines (struct command_line *cmds)
 {
   struct command_line *result = NULL;
@@ -1370,7 +1340,7 @@ copy_command_lines (struct command_line *cmds)
     {
       result = XNEW (struct command_line);
 
-      result->next = copy_command_lines (cmds->next);
+      result->next = copy_command_lines (cmds->next).release ();
       result->line = xstrdup (cmds->line);
       result->control_type = cmds->control_type;
       result->body_count = cmds->body_count;
@@ -1381,13 +1351,14 @@ copy_command_lines (struct command_line *cmds)
           result->body_list = XNEWVEC (struct command_line *, cmds->body_count);
 
           for (i = 0; i < cmds->body_count; i++)
-            result->body_list[i] = copy_command_lines (cmds->body_list[i]);
+            result->body_list[i]
+	      = copy_command_lines (cmds->body_list[i]).release ();
         }
       else
         result->body_list = NULL;
     }
 
-  return result;
+  return command_line_up (result);
 }
 \f
 /* Validate that *COMNAME is a valid name for a command.  Return the
@@ -1460,7 +1431,6 @@ define_command (char *comname, int from_tty)
       CMD_PRE_HOOK,
       CMD_POST_HOOK
     };
-  struct command_line *cmds;
   struct cmd_list_element *c, *newc, *hookc = 0, **list;
   char *tem, *comfull;
   const char *tem_c;
@@ -1536,7 +1506,7 @@ define_command (char *comname, int from_tty)
 
   xsnprintf (tmpbuf, sizeof (tmpbuf),
 	     "Type commands for definition of \"%s\".", comfull);
-  cmds = read_command_lines (tmpbuf, from_tty, 1, 0, 0);
+  command_line_up cmds = read_command_lines (tmpbuf, from_tty, 1, 0, 0);
 
   if (c && c->theclass == class_user)
     free_command_lines (&c->user_commands);
@@ -1544,7 +1514,7 @@ define_command (char *comname, int from_tty)
   newc = add_cmd (comname, class_user, user_defined_command,
 		  (c && c->theclass == class_user)
 		  ? c->doc : xstrdup ("User-defined."), list);
-  newc->user_commands = cmds;
+  newc->user_commands = cmds.release ();
 
   /* If this new command is a hook, then mark both commands as being
      tied.  */
@@ -1571,7 +1541,6 @@ define_command (char *comname, int from_tty)
 static void
 document_command (char *comname, int from_tty)
 {
-  struct command_line *doclines;
   struct cmd_list_element *c, **list;
   const char *tem;
   char *comfull;
@@ -1588,7 +1557,7 @@ document_command (char *comname, int from_tty)
 
   xsnprintf (tmpbuf, sizeof (tmpbuf), "Type documentation for \"%s\".",
 	     comfull);
-  doclines = read_command_lines (tmpbuf, from_tty, 0, 0, 0);
+  command_line_up doclines = read_command_lines (tmpbuf, from_tty, 0, 0, 0);
 
   if (c->doc)
     xfree ((char *) c->doc);
@@ -1598,13 +1567,13 @@ document_command (char *comname, int from_tty)
     int len = 0;
     char *doc;
 
-    for (cl1 = doclines; cl1; cl1 = cl1->next)
+    for (cl1 = doclines.get (); cl1; cl1 = cl1->next)
       len += strlen (cl1->line) + 1;
 
     doc = (char *) xmalloc (len + 1);
     *doc = 0;
 
-    for (cl1 = doclines; cl1; cl1 = cl1->next)
+    for (cl1 = doclines.get (); cl1; cl1 = cl1->next)
       {
 	strcat (doc, cl1->line);
 	if (cl1->next)
@@ -1613,8 +1582,6 @@ document_command (char *comname, int from_tty)
 
     c->doc = doc;
   }
-
-  free_command_lines (&doclines);
 }
 \f
 struct source_cleanup_lines_args
diff --git a/gdb/cli/cli-script.h b/gdb/cli/cli-script.h
index c885976..9514938 100644
--- a/gdb/cli/cli-script.h
+++ b/gdb/cli/cli-script.h
@@ -38,16 +38,13 @@ extern enum command_control_type
 extern enum command_control_type
 	execute_control_command_untraced (struct command_line *cmd);
 
-extern struct command_line *get_command_line (enum command_control_type,
-					      const char *);
+extern command_line_up get_command_line (enum command_control_type,
+					 const char *);
 
 extern void print_command_lines (struct ui_out *,
 				 struct command_line *, unsigned int);
 
-extern struct command_line * copy_command_lines (struct command_line *cmds);
-
-extern struct cleanup *
-  make_cleanup_free_command_lines (struct command_line **arg);
+extern command_line_up copy_command_lines (struct command_line *cmds);
 
 /* Exported to gdb/infrun.c */
 
diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c
index b525f61..1771692 100644
--- a/gdb/compile/compile.c
+++ b/gdb/compile/compile.c
@@ -151,12 +151,10 @@ compile_code_command (char *arg, int from_tty)
     eval_compile_command (NULL, arg, scope, NULL);
   else
     {
-      struct command_line *l = get_command_line (compile_control, "");
-      struct cleanup *cleanup = make_cleanup_free_command_lines (&l);
+      command_line_up l = get_command_line (compile_control, "");
 
       l->control_u.compile.scope = scope;
-      execute_control_command_untraced (l);
-      do_cleanups (cleanup);
+      execute_control_command_untraced (l.get ());
     }
 }
 
@@ -192,13 +190,11 @@ compile_print_command (char *arg_param, int from_tty)
     eval_compile_command (NULL, arg, scope, &fmt);
   else
     {
-      struct command_line *l = get_command_line (compile_control, "");
-      struct cleanup *cleanup = make_cleanup_free_command_lines (&l);
+      command_line_up l = get_command_line (compile_control, "");
 
       l->control_u.compile.scope = scope;
       l->control_u.compile.scope_data = &fmt;
-      execute_control_command_untraced (l);
-      do_cleanups (cleanup);
+      execute_control_command_untraced (l.get ());
     }
 }
 
diff --git a/gdb/defs.h b/gdb/defs.h
index f689ec5..a0b586f 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -445,15 +445,29 @@ struct command_line
     struct command_line **body_list;
   };
 
-extern struct command_line *read_command_lines (char *, int, int,
-						void (*)(char *, void *),
-						void *);
-extern struct command_line *read_command_lines_1 (char * (*) (void), int,
-						  void (*)(char *, void *),
-						  void *);
-
 extern void free_command_lines (struct command_line **);
 
+/* A deleter for command_line that calls free_command_lines.  */
+
+struct command_lines_deleter
+{
+  void operator() (command_line *lines) const
+  {
+    free_command_lines (&lines);
+  }
+};
+
+/* A unique pointer to a command_line.  */
+
+typedef std::unique_ptr<command_line, command_lines_deleter> command_line_up;
+
+extern command_line_up read_command_lines (char *, int, int,
+					   void (*)(char *, void *),
+					   void *);
+extern command_line_up read_command_lines_1 (char * (*) (void), int,
+					     void (*)(char *, void *),
+					     void *);
+
 /* * Parameters of the "info proc" command.  */
 
 enum info_proc_what
diff --git a/gdb/guile/guile.c b/gdb/guile/guile.c
index 27c0a58..9bb2487 100644
--- a/gdb/guile/guile.c
+++ b/gdb/guile/guile.c
@@ -215,10 +215,9 @@ guile_command (char *arg, int from_tty)
     }
   else
     {
-      struct command_line *l = get_command_line (guile_control, "");
+      command_line_up l = get_command_line (guile_control, "");
 
-      make_cleanup_free_command_lines (&l);
-      execute_control_command_untraced (l);
+      execute_control_command_untraced (l.get ());
     }
 
   do_cleanups (cleanup);
@@ -421,11 +420,9 @@ guile_command (char *arg, int from_tty)
     {
       /* Even if Guile isn't enabled, we still have to slurp the
 	 command list to the corresponding "end".  */
-      struct command_line *l = get_command_line (guile_control, "");
-      struct cleanup *cleanups = make_cleanup_free_command_lines (&l);
+      command_line_up l = get_command_line (guile_control, "");
 
-      execute_control_command_untraced (l);
-      do_cleanups (cleanups);
+      execute_control_command_untraced (l.get ());
     }
 }
 
diff --git a/gdb/mi/mi-cmd-break.c b/gdb/mi/mi-cmd-break.c
index 0780f7b..cfe2d34 100644
--- a/gdb/mi/mi-cmd-break.c
+++ b/gdb/mi/mi-cmd-break.c
@@ -493,7 +493,7 @@ mi_read_next_line (void)
 void
 mi_cmd_break_commands (const char *command, char **argv, int argc)
 {
-  struct command_line *break_command;
+  command_line_up break_command;
   char *endptr;
   int bnum;
   struct breakpoint *b;
@@ -523,6 +523,6 @@ mi_cmd_break_commands (const char *command, char **argv, int argc)
   else
     break_command = read_command_lines_1 (mi_read_next_line, 1, 0, 0);
 
-  breakpoint_set_commands (b, break_command);
+  breakpoint_set_commands (b, std::move (break_command));
 }
 
diff --git a/gdb/python/python.c b/gdb/python/python.c
index ec202de..7e0c507 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -433,11 +433,9 @@ python_command (char *arg, int from_tty)
     }
   else
     {
-      struct command_line *l = get_command_line (python_control, "");
-      struct cleanup *cleanup = make_cleanup_free_command_lines (&l);
+      command_line_up l = get_command_line (python_control, "");
 
-      execute_control_command_untraced (l);
-      do_cleanups (cleanup);
+      execute_control_command_untraced (l.get ());
     }
 }
 
@@ -1452,11 +1450,9 @@ python_interactive_command (char *arg, int from_tty)
     error (_("Python scripting is not supported in this copy of GDB."));
   else
     {
-      struct command_line *l = get_command_line (python_control, "");
-      struct cleanup *cleanups = make_cleanup_free_command_lines (&l);
+      command_line_up l = get_command_line (python_control, "");
 
-      execute_control_command_untraced (l);
-      do_cleanups (cleanups);
+      execute_control_command_untraced (l.get ());
     }
 }
 
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index dac1657..c947c95 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -647,20 +647,17 @@ static void
 actions_command (char *args, int from_tty)
 {
   struct tracepoint *t;
-  struct command_line *l;
 
   t = get_tracepoint_by_number (&args, NULL);
   if (t)
     {
-      char *tmpbuf =
-	xstrprintf ("Enter actions for tracepoint %d, one per line.",
-		    t->base.number);
-      struct cleanup *cleanups = make_cleanup (xfree, tmpbuf);
-
-      l = read_command_lines (tmpbuf, from_tty, 1,
-			      check_tracepoint_command, t);
-      do_cleanups (cleanups);
-      breakpoint_set_commands (&t->base, l);
+      std::string tmpbuf =
+	string_printf ("Enter actions for tracepoint %d, one per line.",
+		       t->base.number);
+
+      command_line_up l = read_command_lines (&tmpbuf[0], from_tty, 1,
+					      check_tracepoint_command, t);
+      breakpoint_set_commands (&t->base, std::move (l));
     }
   /* else just return */
 }
-- 
2.9.3

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

* [RFA v2 07/17] Fix up wchar_iterator comment
  2017-04-11 15:02 [RFA v2 00/17] miscellaneous C++-ification Tom Tromey
                   ` (10 preceding siblings ...)
  2017-04-11 15:02 ` [RFA v2 08/17] Remove some cleanups from gnu-v3-abi.c Tom Tromey
@ 2017-04-11 15:02 ` Tom Tromey
  2017-04-11 15:02 ` [RFA v2 13/17] Use std::vector in find_instruction_backward Tom Tromey
                   ` (5 subsequent siblings)
  17 siblings, 0 replies; 36+ messages in thread
From: Tom Tromey @ 2017-04-11 15:02 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This fixes up a comment in charset.h that has been obsolete for a
while.

gdb/ChangeLog
2017-04-11  Tom Tromey  <tom@tromey.com>

	* charset.h (wchar_iterator): Fix comment.
---
 gdb/ChangeLog | 4 ++++
 gdb/charset.h | 6 ++----
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index c50eaa8..6f16318 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,9 @@
 2017-04-11  Tom Tromey  <tom@tromey.com>
 
+	* charset.h (wchar_iterator): Fix comment.
+
+2017-04-11  Tom Tromey  <tom@tromey.com>
+
 	* charset.c (iconv_wrapper): New class.
 	(cleanup_iconv): Remove.
 	(convert_between_encodings): Use it.
diff --git a/gdb/charset.h b/gdb/charset.h
index 9e8ca4a..51180e3 100644
--- a/gdb/charset.h
+++ b/gdb/charset.h
@@ -93,10 +93,8 @@ class wchar_iterator
      buffer.  CHARSET is the name of the character set in which INPUT is
      encoded.  WIDTH is the number of bytes in a base character of
      CHARSET.
-   
-     This function either returns a new character set iterator, or calls
-     error.  The result can be freed using a cleanup; see
-     make_cleanup_wchar_iterator.  */
+
+     This constructor can throw on error.  */
   wchar_iterator (const gdb_byte *input, size_t bytes, const char *charset,
 		  size_t width);
 
-- 
2.9.3

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

* [RFA v2 08/17] Remove some cleanups from gnu-v3-abi.c
  2017-04-11 15:02 [RFA v2 00/17] miscellaneous C++-ification Tom Tromey
                   ` (9 preceding siblings ...)
  2017-04-11 15:01 ` [RFA v2 02/17] Introduce command_line_up Tom Tromey
@ 2017-04-11 15:02 ` Tom Tromey
  2017-04-11 15:02 ` [RFA v2 07/17] Fix up wchar_iterator comment Tom Tromey
                   ` (6 subsequent siblings)
  17 siblings, 0 replies; 36+ messages in thread
From: Tom Tromey @ 2017-04-11 15:02 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This removes some cleanups from gnu-v3-abi.c, by using std::vector
rather than VEC.

gdb/ChangeLog
2017-04-11  Tom Tromey  <tom@tromey.com>

	* gnu-v3-abi.c (value_and_voffset_p): Remove typedef.
	(compare_value_and_voffset): Change type.  Update.
	(compute_vtable_size): Change type of "offset_vec".
	(gnuv3_print_vtable): Use std::vector.  Remove cleanups.
	(gnuv3_get_typeid): Remove extraneous declaration.
---
 gdb/ChangeLog    |  8 ++++++++
 gdb/gnu-v3-abi.c | 54 ++++++++++++++++++------------------------------------
 2 files changed, 26 insertions(+), 36 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 6f16318..d47f796 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,13 @@
 2017-04-11  Tom Tromey  <tom@tromey.com>
 
+	* gnu-v3-abi.c (value_and_voffset_p): Remove typedef.
+	(compare_value_and_voffset): Change type.  Update.
+	(compute_vtable_size): Change type of "offset_vec".
+	(gnuv3_print_vtable): Use std::vector.  Remove cleanups.
+	(gnuv3_get_typeid): Remove extraneous declaration.
+
+2017-04-11  Tom Tromey  <tom@tromey.com>
+
 	* charset.h (wchar_iterator): Fix comment.
 
 2017-04-11  Tom Tromey  <tom@tromey.com>
diff --git a/gdb/gnu-v3-abi.c b/gdb/gnu-v3-abi.c
index 6c7f35b..0090990 100644
--- a/gdb/gnu-v3-abi.c
+++ b/gdb/gnu-v3-abi.c
@@ -27,6 +27,7 @@
 #include "valprint.h"
 #include "c-lang.h"
 #include "typeprint.h"
+#include <algorithm>
 
 static struct cp_abi_ops gnu_v3_abi_ops;
 
@@ -767,9 +768,6 @@ struct value_and_voffset
   int max_voffset;
 };
 
-typedef struct value_and_voffset *value_and_voffset_p;
-DEF_VEC_P (value_and_voffset_p);
-
 /* Hash function for value_and_voffset.  */
 
 static hashval_t
@@ -792,25 +790,18 @@ eq_value_and_voffset (const void *a, const void *b)
 	  == value_address (ovb->value) + value_embedded_offset (ovb->value));
 }
 
-/* qsort comparison function for value_and_voffset.  */
+/* Comparison function for value_and_voffset.  */
 
-static int
-compare_value_and_voffset (const void *a, const void *b)
+static bool
+compare_value_and_voffset (const struct value_and_voffset *va,
+			   const struct value_and_voffset *vb)
 {
-  const struct value_and_voffset * const *ova
-    = (const struct value_and_voffset * const *) a;
-  CORE_ADDR addra = (value_address ((*ova)->value)
-		     + value_embedded_offset ((*ova)->value));
-  const struct value_and_voffset * const *ovb
-    = (const struct value_and_voffset * const *) b;
-  CORE_ADDR addrb = (value_address ((*ovb)->value)
-		     + value_embedded_offset ((*ovb)->value));
-
-  if (addra < addrb)
-    return -1;
-  if (addra > addrb)
-    return 1;
-  return 0;
+  CORE_ADDR addra = (value_address (va->value)
+		     + value_embedded_offset (va->value));
+  CORE_ADDR addrb = (value_address (vb->value)
+		     + value_embedded_offset (vb->value));
+
+  return addra < addrb;
 }
 
 /* A helper function used when printing vtables.  This determines the
@@ -821,7 +812,7 @@ compare_value_and_voffset (const void *a, const void *b)
 
 static void
 compute_vtable_size (htab_t offset_hash,
-		     VEC (value_and_voffset_p) **offset_vec,
+		     std::vector<value_and_voffset *> *offset_vec,
 		     struct value *value)
 {
   int i;
@@ -847,7 +838,7 @@ compute_vtable_size (htab_t offset_hash,
       current_vo->value = value;
       current_vo->max_voffset = -1;
       *slot = current_vo;
-      VEC_safe_push (value_and_voffset_p, *offset_vec, current_vo);
+      offset_vec->push_back (current_vo);
     }
 
   /* Update the value_and_voffset object with the highest vtable
@@ -940,10 +931,7 @@ gnuv3_print_vtable (struct value *value)
   struct type *type;
   struct value *vtable;
   struct value_print_options opts;
-  struct cleanup *cleanup;
-  VEC (value_and_voffset_p) *result_vec = NULL;
-  struct value_and_voffset *iter;
-  int i, count;
+  int count;
 
   value = coerce_ref (value);
   type = check_typedef (value_type (value));
@@ -978,17 +966,14 @@ gnuv3_print_vtable (struct value *value)
   htab_up offset_hash (htab_create_alloc (1, hash_value_and_voffset,
 					  eq_value_and_voffset,
 					  xfree, xcalloc, xfree));
-  cleanup = make_cleanup (VEC_cleanup (value_and_voffset_p), &result_vec);
+  std::vector<value_and_voffset *> result_vec;
 
   compute_vtable_size (offset_hash.get (), &result_vec, value);
-
-  qsort (VEC_address (value_and_voffset_p, result_vec),
-	 VEC_length (value_and_voffset_p, result_vec),
-	 sizeof (value_and_voffset_p),
-	 compare_value_and_voffset);
+  std::sort (result_vec.begin (), result_vec.end (),
+	     compare_value_and_voffset);
 
   count = 0;
-  for (i = 0; VEC_iterate (value_and_voffset_p, result_vec, i, iter); ++i)
+  for (value_and_voffset *iter : result_vec)
     {
       if (iter->max_voffset >= 0)
 	{
@@ -998,8 +983,6 @@ gnuv3_print_vtable (struct value *value)
 	  ++count;
 	}
     }
-
-  do_cleanups (cleanup);
 }
 
 /* Return a GDB type representing `struct std::type_info', laid out
@@ -1077,7 +1060,6 @@ gnuv3_get_typeid (struct value *value)
   struct type *typeinfo_type;
   struct type *type;
   struct gdbarch *gdbarch;
-  struct cleanup *cleanup;
   struct value *result;
   std::string type_name, canonical;
 
-- 
2.9.3

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

* [RFA v2 13/17] Use std::vector in find_instruction_backward
  2017-04-11 15:02 [RFA v2 00/17] miscellaneous C++-ification Tom Tromey
                   ` (11 preceding siblings ...)
  2017-04-11 15:02 ` [RFA v2 07/17] Fix up wchar_iterator comment Tom Tromey
@ 2017-04-11 15:02 ` Tom Tromey
  2017-04-11 15:21 ` [RFA v2 03/17] Change find_pcs_for_symtab_line to return a std::vector Tom Tromey
                   ` (4 subsequent siblings)
  17 siblings, 0 replies; 36+ messages in thread
From: Tom Tromey @ 2017-04-11 15:02 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes find_instruction_backward to use std::vector, removing a
cleanup.

gdb/ChangeLog
2017-04-11  Tom Tromey  <tom@tromey.com>

	* printcmd.c (find_instruction_backward): Use std::vector.
---
 gdb/ChangeLog  |  4 ++++
 gdb/printcmd.c | 16 ++++++----------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ca92e86..f9e4a83 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,9 @@
 2017-04-11  Tom Tromey  <tom@tromey.com>
 
+	* printcmd.c (find_instruction_backward): Use std::vector.
+
+2017-04-11  Tom Tromey  <tom@tromey.com>
+
 	* symfile.c (objfilep): Remove typedef.
 	(reread_symbols): Use a std::vector.
 
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index f09f18a..02d6e1c 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -806,9 +806,8 @@ find_instruction_backward (struct gdbarch *gdbarch, CORE_ADDR addr,
   /* The vector PCS is used to store instruction addresses within
      a pc range.  */
   CORE_ADDR loop_start, loop_end, p;
-  VEC (CORE_ADDR) *pcs = NULL;
+  std::vector<CORE_ADDR> pcs;
   struct symtab_and_line sal;
-  struct cleanup *cleanup = make_cleanup (VEC_cleanup (CORE_ADDR), &pcs);
 
   *inst_read = 0;
   loop_start = loop_end = addr;
@@ -822,7 +821,7 @@ find_instruction_backward (struct gdbarch *gdbarch, CORE_ADDR addr,
      instructions from INST_COUNT, and go to the next iteration.  */
   do
     {
-      VEC_truncate (CORE_ADDR, pcs, 0);
+      pcs.clear ();
       sal = find_pc_sect_line (loop_start, NULL, 1);
       if (sal.line <= 0)
         {
@@ -844,12 +843,12 @@ find_instruction_backward (struct gdbarch *gdbarch, CORE_ADDR addr,
          LOOP_START to LOOP_END.  */
       for (p = loop_start; p < loop_end;)
         {
-          VEC_safe_push (CORE_ADDR, pcs, p);
+	  pcs.push_back (p);
           p += gdb_insn_length (gdbarch, p);
         }
 
-      inst_count -= VEC_length (CORE_ADDR, pcs);
-      *inst_read += VEC_length (CORE_ADDR, pcs);
+      inst_count -= pcs.size ();
+      *inst_read += pcs.size ();
     }
   while (inst_count > 0);
 
@@ -875,9 +874,7 @@ find_instruction_backward (struct gdbarch *gdbarch, CORE_ADDR addr,
      The case when the length of PCS is 0 means that we reached an area for
      which line info is not available.  In such case, we return LOOP_START,
      which was the lowest instruction address that had line info.  */
-  p = VEC_length (CORE_ADDR, pcs) > 0
-    ? VEC_index (CORE_ADDR, pcs, -inst_count)
-    : loop_start;
+  p = pcs.size () > 0 ? pcs[-inst_count] : loop_start;
 
   /* INST_READ includes all instruction addresses in a pc range.  Need to
      exclude the beginning part up to the address we're returning.  That
@@ -885,7 +882,6 @@ find_instruction_backward (struct gdbarch *gdbarch, CORE_ADDR addr,
   if (inst_count < 0)
     *inst_read += inst_count;
 
-  do_cleanups (cleanup);
   return p;
 }
 
-- 
2.9.3

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

* [RFA v2 00/17] miscellaneous C++-ification
@ 2017-04-11 15:02 Tom Tromey
  2017-04-11 15:01 ` [RFA v2 09/17] Remove some cleanups from location.c Tom Tromey
                   ` (17 more replies)
  0 siblings, 18 replies; 36+ messages in thread
From: Tom Tromey @ 2017-04-11 15:02 UTC (permalink / raw)
  To: gdb-patches

This is version 2 of my miscellaneous C++-ification patch series.

I believe this version addresses all the earlier review comments.  It
also adds a few patches to fix some areas pointed out by Simon.

Built and regtested by the buildbot.

Tom

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

* [RFA v2 06/17] Remove cleanup_iconv
  2017-04-11 15:02 [RFA v2 00/17] miscellaneous C++-ification Tom Tromey
                   ` (13 preceding siblings ...)
  2017-04-11 15:21 ` [RFA v2 03/17] Change find_pcs_for_symtab_line to return a std::vector Tom Tromey
@ 2017-04-11 15:21 ` Tom Tromey
  2017-04-12 11:19   ` Pedro Alves
  2017-04-11 15:22 ` [RFA v2 14/17] Use std::vector in compile-loc2c.c Tom Tromey
                   ` (2 subsequent siblings)
  17 siblings, 1 reply; 36+ messages in thread
From: Tom Tromey @ 2017-04-11 15:21 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This introduces a new "iconv_wrapper" class, to be used in
convert_between_encodings.  This allows the removal of cleanup_iconv.

gdb/ChangeLog
2017-04-11  Tom Tromey  <tom@tromey.com>

	* charset.c (iconv_wrapper): New class.
	(cleanup_iconv): Remove.
	(convert_between_encodings): Use it.
---
 gdb/ChangeLog |  6 ++++++
 gdb/charset.c | 43 +++++++++++++++++++++++++++----------------
 2 files changed, 33 insertions(+), 16 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 1c76edb..c50eaa8 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
 2017-04-11  Tom Tromey  <tom@tromey.com>
 
+	* charset.c (iconv_wrapper): New class.
+	(cleanup_iconv): Remove.
+	(convert_between_encodings): Use it.
+
+2017-04-11  Tom Tromey  <tom@tromey.com>
+
 	* symfile.h (increment_reading_symtab): Update type.
 	* symfile.c (decrement_reading_symtab): Remove.
 	(increment_reading_symtab): Return a scoped_restore_tmpl<int>.
diff --git a/gdb/charset.c b/gdb/charset.c
index 8302c59..a6da512 100644
--- a/gdb/charset.c
+++ b/gdb/charset.c
@@ -481,14 +481,32 @@ host_hex_value (char c)
 \f
 /* Public character management functions.  */
 
-/* A cleanup function which is run to close an iconv descriptor.  */
-
-static void
-cleanup_iconv (void *p)
+class iconv_wrapper
 {
-  iconv_t *descp = (iconv_t *) p;
-  iconv_close (*descp);
-}
+public:
+
+  iconv_wrapper (const char *from, const char *to)
+  {
+    m_desc = iconv_open (to, from);
+    if (m_desc == (iconv_t) -1)
+      perror_with_name (_("Converting character sets"));
+  }
+
+  ~iconv_wrapper ()
+  {
+    iconv_close (m_desc);
+  }
+
+  size_t convert (ICONV_CONST char **inp, size_t *inleft, char **outp,
+		  size_t *outleft)
+  {
+    return iconv (m_desc, inp, inleft, outp, outleft);
+  }
+
+private:
+
+  iconv_t m_desc;
+};
 
 void
 convert_between_encodings (const char *from, const char *to,
@@ -496,8 +514,6 @@ convert_between_encodings (const char *from, const char *to,
 			   int width, struct obstack *output,
 			   enum transliterations translit)
 {
-  iconv_t desc;
-  struct cleanup *cleanups;
   size_t inleft;
   ICONV_CONST char *inp;
   unsigned int space_request;
@@ -509,10 +525,7 @@ convert_between_encodings (const char *from, const char *to,
       return;
     }
 
-  desc = iconv_open (to, from);
-  if (desc == (iconv_t) -1)
-    perror_with_name (_("Converting character sets"));
-  cleanups = make_cleanup (cleanup_iconv, &desc);
+  iconv_wrapper desc (from, to);
 
   inleft = num_bytes;
   inp = (ICONV_CONST char *) bytes;
@@ -531,7 +544,7 @@ convert_between_encodings (const char *from, const char *to,
       outp = (char *) obstack_base (output) + old_size;
       outleft = space_request;
 
-      r = iconv (desc, &inp, &inleft, &outp, &outleft);
+      r = desc.convert (&inp, &inleft, &outp, &outleft);
 
       /* Now make sure that the object on the obstack only includes
 	 bytes we have converted.  */
@@ -583,8 +596,6 @@ convert_between_encodings (const char *from, const char *to,
 	    }
 	}
     }
-
-  do_cleanups (cleanups);
 }
 
 \f
-- 
2.9.3

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

* [RFA v2 03/17] Change find_pcs_for_symtab_line to return a std::vector
  2017-04-11 15:02 [RFA v2 00/17] miscellaneous C++-ification Tom Tromey
                   ` (12 preceding siblings ...)
  2017-04-11 15:02 ` [RFA v2 13/17] Use std::vector in find_instruction_backward Tom Tromey
@ 2017-04-11 15:21 ` Tom Tromey
  2017-04-11 15:21 ` [RFA v2 06/17] Remove cleanup_iconv Tom Tromey
                   ` (3 subsequent siblings)
  17 siblings, 0 replies; 36+ messages in thread
From: Tom Tromey @ 2017-04-11 15:21 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes find_pcs_for_symtab_line to return a std::vector.  This
allows the removal of some cleanups.

gdb/ChangeLog
2017-04-11  Tom Tromey  <tom@tromey.com>

	* symtab.h (find_pcs_for_symtab_line): Change return type.
	* symtab.c (find_pcs_for_symtab_line): Change return type.
	* python/py-linetable.c (build_line_table_tuple_from_pcs): Change
	type of "vec".  Update.
	(ltpy_get_pcs_for_line): Update.
	* linespec.c (decode_digits_ordinary): Update.
---
 gdb/ChangeLog             |  9 +++++++++
 gdb/linespec.c            |  8 ++------
 gdb/python/py-linetable.c | 24 +++++++++---------------
 gdb/symtab.c              | 11 +++++------
 gdb/symtab.h              |  6 +++---
 5 files changed, 28 insertions(+), 30 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 7fa5099..a71de4a 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,14 @@
 2017-04-11  Tom Tromey  <tom@tromey.com>
 
+	* symtab.h (find_pcs_for_symtab_line): Change return type.
+	* symtab.c (find_pcs_for_symtab_line): Change return type.
+	* python/py-linetable.c (build_line_table_tuple_from_pcs): Change
+	type of "vec".  Update.
+	(ltpy_get_pcs_for_line): Update.
+	* linespec.c (decode_digits_ordinary): Update.
+
+2017-04-11  Tom Tromey  <tom@tromey.com>
+
 	* tracepoint.c (actions_command): Update.
 	* python/python.c (python_command, python_interactive_command):
 	Update.
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 41b82d7..bccabaf 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -3469,9 +3469,7 @@ decode_digits_ordinary (struct linespec_state *self,
 
   for (ix = 0; VEC_iterate (symtab_ptr, ls->file_symtabs, ix, elt); ++ix)
     {
-      int i;
-      VEC (CORE_ADDR) *pcs;
-      CORE_ADDR pc;
+      std::vector<CORE_ADDR> pcs;
 
       /* The logic above should ensure this.  */
       gdb_assert (elt != NULL);
@@ -3479,7 +3477,7 @@ decode_digits_ordinary (struct linespec_state *self,
       set_current_program_space (SYMTAB_PSPACE (elt));
 
       pcs = find_pcs_for_symtab_line (elt, line, best_entry);
-      for (i = 0; VEC_iterate (CORE_ADDR, pcs, i, pc); ++i)
+      for (CORE_ADDR pc : pcs)
 	{
 	  struct symtab_and_line sal;
 
@@ -3490,8 +3488,6 @@ decode_digits_ordinary (struct linespec_state *self,
 	  sal.pc = pc;
 	  add_sal_to_sals_basic (sals, &sal);
 	}
-
-      VEC_free (CORE_ADDR, pcs);
     }
 }
 
diff --git a/gdb/python/py-linetable.c b/gdb/python/py-linetable.c
index 8d17aab..13daa3d 100644
--- a/gdb/python/py-linetable.c
+++ b/gdb/python/py-linetable.c
@@ -115,30 +115,28 @@ build_linetable_entry (int line, CORE_ADDR address)
   return (PyObject *) obj;
 }
 
-/* Internal helper function to build a Python Tuple from a GDB Vector.
+/* Internal helper function to build a Python Tuple from a vector.
    A line table entry can have multiple PCs for a given source line.
    Construct a Tuple of all entries for the given source line, LINE
-   from the line table VEC.  Construct one line table entry object per
+   from the line table PCS.  Construct one line table entry object per
    address.  */
 
 static PyObject *
-build_line_table_tuple_from_pcs (int line, VEC (CORE_ADDR) *vec)
+build_line_table_tuple_from_pcs (int line, const std::vector<CORE_ADDR> &pcs)
 {
-  int vec_len = 0;
-  CORE_ADDR pc;
   int i;
 
-  vec_len = VEC_length (CORE_ADDR, vec);
-  if (vec_len < 1)
+  if (pcs.size () < 1)
     Py_RETURN_NONE;
 
-  gdbpy_ref<> tuple (PyTuple_New (vec_len));
+  gdbpy_ref<> tuple (PyTuple_New (pcs.size ()));
 
   if (tuple == NULL)
     return NULL;
 
-  for (i = 0; VEC_iterate (CORE_ADDR, vec, i, pc); ++i)
+  for (i = 0; i < pcs.size (); ++i)
     {
+      CORE_ADDR pc = pcs[i];
       gdbpy_ref<> obj (build_linetable_entry (line, pc));
 
       if (obj == NULL)
@@ -160,8 +158,7 @@ ltpy_get_pcs_for_line (PyObject *self, PyObject *args)
   struct symtab *symtab;
   gdb_py_longest py_line;
   struct linetable_entry *best_entry = NULL;
-  VEC (CORE_ADDR) *pcs = NULL;
-  PyObject *tuple;
+  std::vector<CORE_ADDR> pcs;
 
   LTPY_REQUIRE_VALID (self, symtab);
 
@@ -178,10 +175,7 @@ ltpy_get_pcs_for_line (PyObject *self, PyObject *args)
     }
   END_CATCH
 
-  tuple = build_line_table_tuple_from_pcs (py_line, pcs);
-  VEC_free (CORE_ADDR, pcs);
-
-  return tuple;
+  return build_line_table_tuple_from_pcs (py_line, pcs);
 }
 
 /* Implementation of gdb.LineTable.has_line (self, line) -> Boolean.
diff --git a/gdb/symtab.c b/gdb/symtab.c
index cc2f400..20ef76d 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -3289,15 +3289,15 @@ done:
 }
 
 /* Given SYMTAB, returns all the PCs function in the symtab that
-   exactly match LINE.  Returns NULL if there are no exact matches,
-   but updates BEST_ITEM in this case.  */
+   exactly match LINE.  Returns an empty vector if there are no exact
+   matches, but updates BEST_ITEM in this case.  */
 
-VEC (CORE_ADDR) *
+std::vector<CORE_ADDR>
 find_pcs_for_symtab_line (struct symtab *symtab, int line,
 			  struct linetable_entry **best_item)
 {
   int start = 0;
-  VEC (CORE_ADDR) *result = NULL;
+  std::vector<CORE_ADDR> result;
 
   /* First, collect all the PCs that are at this line.  */
   while (1)
@@ -3320,8 +3320,7 @@ find_pcs_for_symtab_line (struct symtab *symtab, int line,
 	  break;
 	}
 
-      VEC_safe_push (CORE_ADDR, result,
-		     SYMTAB_LINETABLE (symtab)->item[idx].pc);
+      result.push_back (SYMTAB_LINETABLE (symtab)->item[idx].pc);
       start = idx + 1;
     }
 
diff --git a/gdb/symtab.h b/gdb/symtab.h
index d8c665c..341deca 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -20,7 +20,7 @@
 #if !defined (SYMTAB_H)
 #define SYMTAB_H 1
 
-#include "vec.h"
+#include <vector>
 #include "gdb_vecs.h"
 #include "gdbtypes.h"
 #include "common/enum-flags.h"
@@ -1618,8 +1618,8 @@ void iterate_over_symtabs (const char *name,
 			   gdb::function_view<bool (symtab *)> callback);
 
 
-VEC (CORE_ADDR) *find_pcs_for_symtab_line (struct symtab *symtab, int line,
-					   struct linetable_entry **best_entry);
+std::vector<CORE_ADDR> find_pcs_for_symtab_line
+    (struct symtab *symtab, int line, struct linetable_entry **best_entry);
 
 /* Prototype for callbacks for LA_ITERATE_OVER_SYMBOLS.  The callback
    is called once per matching symbol SYM.  The callback should return
-- 
2.9.3

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

* [RFA v2 14/17] Use std::vector in compile-loc2c.c
  2017-04-11 15:02 [RFA v2 00/17] miscellaneous C++-ification Tom Tromey
                   ` (14 preceding siblings ...)
  2017-04-11 15:21 ` [RFA v2 06/17] Remove cleanup_iconv Tom Tromey
@ 2017-04-11 15:22 ` Tom Tromey
  2017-04-11 15:23 ` [RFA v2 15/17] Change breakpoint event locations to event_location_up Tom Tromey
  2017-04-12  2:49 ` [RFA v2 00/17] miscellaneous C++-ification Simon Marchi
  17 siblings, 0 replies; 36+ messages in thread
From: Tom Tromey @ 2017-04-11 15:22 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes compile-loc2c.c to use std::vector in place of a VEC,
allowing the removal of a cleanup.

gdb/ChangeLog
2017-04-11  Tom Tromey  <tom@tromey.com>

	* compile/compile-loc2c.c (compute_stack_depth_worker): Change
	type of "to_do".  Update.
	(compute_stack_depth): Use std::vector.
---
 gdb/ChangeLog               |  6 ++++++
 gdb/compile/compile-loc2c.c | 20 +++++++++-----------
 2 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index f9e4a83..346ee66 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
 2017-04-11  Tom Tromey  <tom@tromey.com>
 
+	* compile/compile-loc2c.c (compute_stack_depth_worker): Change
+	type of "to_do".  Update.
+	(compute_stack_depth): Use std::vector.
+
+2017-04-11  Tom Tromey  <tom@tromey.com>
+
 	* printcmd.c (find_instruction_backward): Use std::vector.
 
 2017-04-11  Tom Tromey  <tom@tromey.com>
diff --git a/gdb/compile/compile-loc2c.c b/gdb/compile/compile-loc2c.c
index f1296e8..a53214f 100644
--- a/gdb/compile/compile-loc2c.c
+++ b/gdb/compile/compile-loc2c.c
@@ -72,7 +72,7 @@ struct insn_info
 static void
 compute_stack_depth_worker (int start, int *need_tempvar,
 			    struct insn_info *info,
-			    VEC (int) **to_do,
+			    std::vector<int> *to_do,
 			    enum bfd_endian byte_order, unsigned int addr_size,
 			    const gdb_byte *op_ptr, const gdb_byte *op_end)
 {
@@ -334,7 +334,7 @@ compute_stack_depth_worker (int start, int *need_tempvar,
 	  /* If the destination has not been seen yet, add it to the
 	     to-do list.  */
 	  if (!info[offset].visited)
-	    VEC_safe_push (int, *to_do, offset);
+	    to_do->push_back (offset);
 	  SET_CHECK_DEPTH (offset);
 	  info[offset].label = 1;
 	  /* We're done with this line of code.  */
@@ -348,7 +348,7 @@ compute_stack_depth_worker (int start, int *need_tempvar,
 	  /* If the destination has not been seen yet, add it to the
 	     to-do list.  */
 	  if (!info[offset].visited)
-	    VEC_safe_push (int, *to_do, offset);
+	    to_do->push_back (offset);
 	  SET_CHECK_DEPTH (offset);
 	  info[offset].label = 1;
 	  break;
@@ -390,22 +390,21 @@ compute_stack_depth (enum bfd_endian byte_order, unsigned int addr_size,
 		     struct insn_info **info)
 {
   unsigned char *set;
-  struct cleanup *outer_cleanup, *cleanup;
-  VEC (int) *to_do = NULL;
+  struct cleanup *outer_cleanup;
+  std::vector<int> to_do;
   int stack_depth, i;
 
   *info = XCNEWVEC (struct insn_info, op_end - op_ptr);
   outer_cleanup = make_cleanup (xfree, *info);
 
-  cleanup = make_cleanup (VEC_cleanup (int), &to_do);
-
-  VEC_safe_push (int, to_do, 0);
+  to_do.push_back (0);
   (*info)[0].depth = initial_depth;
   (*info)[0].visited = 1;
 
-  while (!VEC_empty (int, to_do))
+  while (!to_do.empty ())
     {
-      int ndx = VEC_pop (int, to_do);
+      int ndx = to_do.back ();
+      to_do.pop_back ();
 
       compute_stack_depth_worker (ndx, need_tempvar, *info, &to_do,
 				  byte_order, addr_size,
@@ -422,7 +421,6 @@ compute_stack_depth (enum bfd_endian byte_order, unsigned int addr_size,
 	*is_tls = 1;
     }
 
-  do_cleanups (cleanup);
   discard_cleanups (outer_cleanup);
   return stack_depth + 1;
 }
-- 
2.9.3

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

* [RFA v2 15/17] Change breakpoint event locations to event_location_up
  2017-04-11 15:02 [RFA v2 00/17] miscellaneous C++-ification Tom Tromey
                   ` (15 preceding siblings ...)
  2017-04-11 15:22 ` [RFA v2 14/17] Use std::vector in compile-loc2c.c Tom Tromey
@ 2017-04-11 15:23 ` Tom Tromey
  2017-04-12  2:25   ` Simon Marchi
  2017-04-12  2:49 ` [RFA v2 00/17] miscellaneous C++-ification Simon Marchi
  17 siblings, 1 reply; 36+ messages in thread
From: Tom Tromey @ 2017-04-11 15:23 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This is a follow-up to an earlier patch.  It changes breakpoint's
location and location_range_end members to be of type
event_location_up, then fixes up the users.

gdb/ChangeLog
2017-04-11  Tom Tromey  <tom@tromey.com>

	* remote.c (remote_download_tracepoint): Update.
	* python/py-breakpoint.c (bppy_get_location): Update.
	* guile/scm-breakpoint.c (bpscm_print_breakpoint_smob)
	(gdbscm_breakpoint_location): Update.
	* elfread.c (elf_gnu_ifunc_resolver_return_stop): Update.
	* breakpoint.h (struct breakpoint) <location, location_range_end>:
	Change type to event_location_up.
	* breakpoint.c (create_overlay_event_breakpoint)
	(create_longjmp_master_breakpoint)
	(create_std_terminate_master_breakpoint)
	(create_exception_master_breakpoint)
	(breakpoint_event_location_empty_p, print_breakpoint_location)
	(print_one_breakpoint_location, create_thread_event_breakpoint)
	(init_breakpoint_sal, create_breakpoint)
	(print_recreate_ranged_breakpoint, break_range_command)
	(init_ada_exception_breakpoint, say_where): Update.
	(base_breakpoint_dtor): Don't call delete_event_location.
	(bkpt_print_recreate, tracepoint_print_recreate)
	(dprintf_print_recreate, update_static_tracepoint)
	(breakpoint_re_set_default): Update.
---
 gdb/ChangeLog              | 23 ++++++++++++++
 gdb/breakpoint.c           | 74 ++++++++++++++++++++++------------------------
 gdb/breakpoint.h           | 10 +++----
 gdb/elfread.c              |  2 +-
 gdb/guile/scm-breakpoint.c |  4 +--
 gdb/python/py-breakpoint.c |  2 +-
 gdb/remote.c               |  2 +-
 7 files changed, 68 insertions(+), 49 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 346ee66..3faadc2 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,28 @@
 2017-04-11  Tom Tromey  <tom@tromey.com>
 
+	* remote.c (remote_download_tracepoint): Update.
+	* python/py-breakpoint.c (bppy_get_location): Update.
+	* guile/scm-breakpoint.c (bpscm_print_breakpoint_smob)
+	(gdbscm_breakpoint_location): Update.
+	* elfread.c (elf_gnu_ifunc_resolver_return_stop): Update.
+	* breakpoint.h (struct breakpoint) <location, location_range_end>:
+	Change type to event_location_up.
+	* breakpoint.c (create_overlay_event_breakpoint)
+	(create_longjmp_master_breakpoint)
+	(create_std_terminate_master_breakpoint)
+	(create_exception_master_breakpoint)
+	(breakpoint_event_location_empty_p, print_breakpoint_location)
+	(print_one_breakpoint_location, create_thread_event_breakpoint)
+	(init_breakpoint_sal, create_breakpoint)
+	(print_recreate_ranged_breakpoint, break_range_command)
+	(init_ada_exception_breakpoint, say_where): Update.
+	(base_breakpoint_dtor): Don't call delete_event_location.
+	(bkpt_print_recreate, tracepoint_print_recreate)
+	(dprintf_print_recreate, update_static_tracepoint)
+	(breakpoint_re_set_default): Update.
+
+2017-04-11  Tom Tromey  <tom@tromey.com>
+
 	* compile/compile-loc2c.c (compute_stack_depth_worker): Change
 	type of "to_do".  Update.
 	(compute_stack_depth): Use std::vector.
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 3a3cd80..afc8309 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -67,7 +67,6 @@
 #include "dummy-frame.h"
 #include "interps.h"
 #include "format.h"
-#include "location.h"
 #include "thread-fsm.h"
 #include "tid-parse.h"
 
@@ -3472,7 +3471,7 @@ create_overlay_event_breakpoint (void)
 				      &internal_breakpoint_ops);
       initialize_explicit_location (&explicit_loc);
       explicit_loc.function_name = ASTRDUP (func_name);
-      b->location = new_explicit_location (&explicit_loc).release ();
+      b->location = new_explicit_location (&explicit_loc);
 
       if (overlay_debugging == ovly_auto)
         {
@@ -3552,8 +3551,7 @@ create_longjmp_master_breakpoint (void)
 								 objfile),
 					      bp_longjmp_master,
 					      &internal_breakpoint_ops);
-	      b->location
-		= new_probe_location ("-probe-stap libc:longjmp").release ();
+	      b->location = new_probe_location ("-probe-stap libc:longjmp");
 	      b->enable_state = bp_disabled;
 	    }
 
@@ -3593,7 +3591,7 @@ create_longjmp_master_breakpoint (void)
 					  &internal_breakpoint_ops);
 	  initialize_explicit_location (&explicit_loc);
 	  explicit_loc.function_name = ASTRDUP (func_name);
-	  b->location = new_explicit_location (&explicit_loc).release ();
+	  b->location = new_explicit_location (&explicit_loc);
 	  b->enable_state = bp_disabled;
 	}
     }
@@ -3651,7 +3649,7 @@ create_std_terminate_master_breakpoint (void)
 				      &internal_breakpoint_ops);
       initialize_explicit_location (&explicit_loc);
       explicit_loc.function_name = ASTRDUP (func_name);
-      b->location = new_explicit_location (&explicit_loc).release ();
+      b->location = new_explicit_location (&explicit_loc);
       b->enable_state = bp_disabled;
     }
   }
@@ -3720,8 +3718,7 @@ create_exception_master_breakpoint (void)
 								 objfile),
 					      bp_exception_master,
 					      &internal_breakpoint_ops);
-	      b->location
-		= new_probe_location ("-probe-stap libgcc:unwind").release ();
+	      b->location = new_probe_location ("-probe-stap libgcc:unwind");
 	      b->enable_state = bp_disabled;
 	    }
 
@@ -3756,7 +3753,7 @@ create_exception_master_breakpoint (void)
 				      &internal_breakpoint_ops);
       initialize_explicit_location (&explicit_loc);
       explicit_loc.function_name = ASTRDUP (func_name);
-      b->location = new_explicit_location (&explicit_loc).release ();
+      b->location = new_explicit_location (&explicit_loc);
       b->enable_state = bp_disabled;
     }
 }
@@ -3766,7 +3763,7 @@ create_exception_master_breakpoint (void)
 static int
 breakpoint_event_location_empty_p (const struct breakpoint *b)
 {
-  return b->location != NULL && event_location_empty_p (b->location);
+  return b->location != NULL && event_location_empty_p (b->location.get ());
 }
 
 void
@@ -6150,7 +6147,7 @@ print_breakpoint_location (struct breakpoint *b,
     set_current_program_space (loc->pspace);
 
   if (b->display_canonical)
-    uiout->field_string ("what", event_location_to_string (b->location));
+    uiout->field_string ("what", event_location_to_string (b->location.get ()));
   else if (loc && loc->symtab)
     {
       struct symbol *sym 
@@ -6182,7 +6179,8 @@ print_breakpoint_location (struct breakpoint *b,
     }
   else
     {
-      uiout->field_string ("pending", event_location_to_string (b->location));
+      uiout->field_string ("pending",
+			   event_location_to_string (b->location.get ()));
       /* If extra_string is available, it could be holding a condition
 	 or dprintf arguments.  In either case, make sure it is printed,
 	 too, but only for non-MI streams.  */
@@ -6670,9 +6668,9 @@ print_one_breakpoint_location (struct breakpoint *b,
 	  uiout->field_string ("original-location", w->exp_string);
 	}
       else if (b->location != NULL
-	       && event_location_to_string (b->location) != NULL)
+	       && event_location_to_string (b->location.get ()) != NULL)
 	uiout->field_string ("original-location",
-			     event_location_to_string (b->location));
+			     event_location_to_string (b->location.get ()));
     }
 }
 
@@ -7796,7 +7794,7 @@ create_thread_event_breakpoint (struct gdbarch *gdbarch, CORE_ADDR address)
 
   b->enable_state = bp_enabled;
   /* location has to be used or breakpoint_re_set will delete me.  */
-  b->location = new_address_location (b->loc->address, NULL, 0).release ();
+  b->location = new_address_location (b->loc->address, NULL, 0);
 
   update_global_location_list_nothrow (UGLL_MAY_INSERT);
 
@@ -9285,7 +9283,8 @@ init_breakpoint_sal (struct breakpoint *b, struct gdbarch *gdbarch,
 		{
 		  /* We already know the marker exists, otherwise, we
 		     wouldn't see a sal for it.  */
-		  const char *p = &event_location_to_string (b->location)[3];
+		  const char *p
+		    = &event_location_to_string (b->location.get ())[3];
 		  const char *endp;
 		  char *marker_str;
 
@@ -9348,9 +9347,9 @@ init_breakpoint_sal (struct breakpoint *b, struct gdbarch *gdbarch,
 
   b->display_canonical = display_canonical;
   if (location != NULL)
-    b->location = location.release ();
+    b->location = std::move (location);
   else
-    b->location = new_address_location (b->loc->address, NULL, 0).release ();
+    b->location = new_address_location (b->loc->address, NULL, 0);
   b->filter = filter;
 }
 
@@ -9879,7 +9878,7 @@ create_breakpoint (struct gdbarch *gdbarch,
 	b = new breakpoint ();
 
       init_raw_breakpoint_without_location (b, gdbarch, type_wanted, ops);
-      b->location = copy_event_location (location).release ();
+      b->location = copy_event_location (location);
 
       if (parse_extra)
 	b->cond_string = NULL;
@@ -10305,8 +10304,8 @@ static void
 print_recreate_ranged_breakpoint (struct breakpoint *b, struct ui_file *fp)
 {
   fprintf_unfiltered (fp, "break-range %s, %s",
-		      event_location_to_string (b->location),
-		      event_location_to_string (b->location_range_end));
+		      event_location_to_string (b->location.get ()),
+		      event_location_to_string (b->location_range_end.get ()));
   print_recreate_thread (b, fp);
 }
 
@@ -10452,8 +10451,8 @@ break_range_command (char *arg, int from_tty)
   set_breakpoint_count (breakpoint_count + 1);
   b->number = breakpoint_count;
   b->disposition = disp_donttouch;
-  b->location = start_location.release ();
-  b->location_range_end = end_location.release ();
+  b->location = std::move (start_location);
+  b->location_range_end = std::move (end_location);
   b->loc->length = length;
 
   do_cleanups (cleanup_bkpt);
@@ -11899,9 +11898,8 @@ init_ada_exception_breakpoint (struct breakpoint *b,
 
   b->enable_state = enabled ? bp_enabled : bp_disabled;
   b->disposition = tempflag ? disp_del : disp_donttouch;
-  b->location
-    = string_to_event_location (&addr_string,
-				language_def (language_ada)).release ();
+  b->location = string_to_event_location (&addr_string,
+					  language_def (language_ada));
   b->language = language_ada;
 }
 
@@ -12769,18 +12767,18 @@ say_where (struct breakpoint *b)
       if (b->extra_string == NULL)
 	{
 	  printf_filtered (_(" (%s) pending."),
-			   event_location_to_string (b->location));
+			   event_location_to_string (b->location.get ()));
 	}
       else if (b->type == bp_dprintf)
 	{
 	  printf_filtered (_(" (%s,%s) pending."),
-			   event_location_to_string (b->location),
+			   event_location_to_string (b->location.get ()),
 			   b->extra_string);
 	}
       else
 	{
 	  printf_filtered (_(" (%s %s) pending."),
-			   event_location_to_string (b->location),
+			   event_location_to_string (b->location.get ()),
 			   b->extra_string);
 	}
     }
@@ -12805,7 +12803,7 @@ say_where (struct breakpoint *b)
 	       different file name, and this at least reflects the
 	       real situation somewhat.  */
 	    printf_filtered (": %s.",
-			     event_location_to_string (b->location));
+			     event_location_to_string (b->location.get ()));
 	}
 
       if (b->loc->next)
@@ -12842,8 +12840,6 @@ base_breakpoint_dtor (struct breakpoint *self)
   xfree (self->cond_string);
   xfree (self->extra_string);
   xfree (self->filter);
-  delete_event_location (self->location);
-  delete_event_location (self->location_range_end);
 }
 
 static struct bp_location *
@@ -13177,7 +13173,7 @@ bkpt_print_recreate (struct breakpoint *tp, struct ui_file *fp)
 		    _("unhandled breakpoint type %d"), (int) tp->type);
 
   fprintf_unfiltered (fp, " %s",
-		      event_location_to_string (tp->location));
+		      event_location_to_string (tp->location.get ()));
 
   /* Print out extra_string if this breakpoint is pending.  It might
      contain, for example, conditions that were set by the user.  */
@@ -13501,7 +13497,7 @@ tracepoint_print_recreate (struct breakpoint *self, struct ui_file *fp)
 		    _("unhandled tracepoint type %d"), (int) self->type);
 
   fprintf_unfiltered (fp, " %s",
-		      event_location_to_string (self->location));
+		      event_location_to_string (self->location.get ()));
   print_recreate_thread (self, fp);
 
   if (tp->pass_count)
@@ -13603,7 +13599,7 @@ static void
 dprintf_print_recreate (struct breakpoint *tp, struct ui_file *fp)
 {
   fprintf_unfiltered (fp, "dprintf %s,%s",
-		      event_location_to_string (tp->location),
+		      event_location_to_string (tp->location.get ()),
 		      tp->extra_string);
   print_recreate_thread (tp, fp);
 }
@@ -14119,13 +14115,13 @@ update_static_tracepoint (struct breakpoint *b, struct symtab_and_line sal)
 	  b->loc->line_number = sal2.line;
 	  b->loc->symtab = sym != NULL ? sal2.symtab : NULL;
 
-	  delete_event_location (b->location);
+	  b->location.reset (NULL);
 	  initialize_explicit_location (&explicit_loc);
 	  explicit_loc.source_filename
 	    = ASTRDUP (symtab_to_filename_for_display (sal2.symtab));
 	  explicit_loc.line_offset.offset = b->loc->line_number;
 	  explicit_loc.line_offset.sign = LINE_OFFSET_NONE;
-	  b->location = new_explicit_location (&explicit_loc).release ();
+	  b->location = new_explicit_location (&explicit_loc);
 
 	  /* Might be nice to check if function changed, and warn if
 	     so.  */
@@ -14424,7 +14420,7 @@ breakpoint_re_set_default (struct breakpoint *b)
   struct symtabs_and_lines expanded_end = {0};
   struct program_space *filter_pspace = current_program_space;
 
-  sals = location_to_sals (b, b->location, filter_pspace, &found);
+  sals = location_to_sals (b, b->location.get (), filter_pspace, &found);
   if (found)
     {
       make_cleanup (xfree, sals.sals);
@@ -14433,7 +14429,7 @@ breakpoint_re_set_default (struct breakpoint *b)
 
   if (b->location_range_end != NULL)
     {
-      sals_end = location_to_sals (b, b->location_range_end,
+      sals_end = location_to_sals (b, b->location_range_end.get (),
 				   filter_pspace, &found);
       if (found)
 	{
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 1301fb4..f5c2751 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -26,6 +26,7 @@
 #include "command.h"
 #include "break-common.h"
 #include "probe.h"
+#include "location.h"
 #include <vector>
 
 struct value;
@@ -38,7 +39,6 @@ struct bpstats;
 struct bp_location;
 struct linespec_result;
 struct linespec_sals;
-struct event_location;
 
 /* Why are we removing the breakpoint from the target?  */
 
@@ -716,8 +716,8 @@ struct breakpoint
        non-thread-specific ordinary breakpoints this is NULL.  */
     struct program_space *pspace;
 
-    /* Location we used to set the breakpoint (malloc'd).  */
-    struct event_location *location;
+    /* Location we used to set the breakpoint.  */
+    event_location_up location;
 
     /* The filter that should be passed to decode_line_full when
        re-setting this breakpoint.  This may be NULL, but otherwise is
@@ -725,8 +725,8 @@ struct breakpoint
     char *filter;
 
     /* For a ranged breakpoint, the location we used to find
-       the end of the range (malloc'd).  */
-    struct event_location *location_range_end;
+       the end of the range.  */
+    event_location_up location_range_end;
 
     /* Architecture we used to set the breakpoint.  */
     struct gdbarch *gdbarch;
diff --git a/gdb/elfread.c b/gdb/elfread.c
index 2ca10f8..fba2026 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -1006,7 +1006,7 @@ elf_gnu_ifunc_resolver_return_stop (struct breakpoint *b)
   resolved_pc = gdbarch_addr_bits_remove (gdbarch, resolved_pc);
 
   gdb_assert (current_program_space == b->pspace || b->pspace == NULL);
-  elf_gnu_ifunc_record_cache (event_location_to_string (b->location),
+  elf_gnu_ifunc_record_cache (event_location_to_string (b->location.get ()),
 			      resolved_pc);
 
   sal = find_pc_line (resolved_pc, 0);
diff --git a/gdb/guile/scm-breakpoint.c b/gdb/guile/scm-breakpoint.c
index 3a1e4d3..9165684 100644
--- a/gdb/guile/scm-breakpoint.c
+++ b/gdb/guile/scm-breakpoint.c
@@ -184,7 +184,7 @@ bpscm_print_breakpoint_smob (SCM self, SCM port, scm_print_state *pstate)
       gdbscm_printf (port, " hit:%d", b->hit_count);
       gdbscm_printf (port, " ignore:%d", b->ignore_count);
 
-      str = event_location_to_string (b->location);
+      str = event_location_to_string (b->location.get ());
       if (str != NULL)
 	gdbscm_printf (port, " @%s", str);
     }
@@ -832,7 +832,7 @@ gdbscm_breakpoint_location (SCM self)
   if (bp_smob->bp->type != bp_breakpoint)
     return SCM_BOOL_F;
 
-  str = event_location_to_string (bp_smob->bp->location);
+  str = event_location_to_string (bp_smob->bp->location.get ());
   if (! str)
     str = "";
 
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index d63f458..64de803 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -390,7 +390,7 @@ bppy_get_location (PyObject *self, void *closure)
   if (obj->bp->type != bp_breakpoint)
     Py_RETURN_NONE;
 
-  str = event_location_to_string (obj->bp->location);
+  str = event_location_to_string (obj->bp->location.get ());
   if (! str)
     str = "";
   return host_string_to_python_string (str);
diff --git a/gdb/remote.c b/gdb/remote.c
index a61469c..cdc3cdd 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -12416,7 +12416,7 @@ remote_download_tracepoint (struct target_ops *self, struct bp_location *loc)
 	{
 	  strcpy (buf, "QTDPsrc:");
 	  encode_source_string (b->number, loc->address, "at",
-				event_location_to_string (b->location),
+				event_location_to_string (b->location.get ()),
 				buf + strlen (buf), 2048 - strlen (buf));
 	  putpkt (buf);
 	  remote_get_noisy_reply (&target_buf, &target_buf_size);
-- 
2.9.3

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

* Re: [RFA v2 16/17] Add a constructor and destructor to  linespec_result
  2017-04-11 15:01 ` [RFA v2 16/17] Add a constructor and destructor to linespec_result Tom Tromey
@ 2017-04-12  2:25   ` Simon Marchi
  2017-04-12 14:30     ` Tom Tromey
  0 siblings, 1 reply; 36+ messages in thread
From: Simon Marchi @ 2017-04-12  2:25 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 2017-04-11 11:01, Tom Tromey wrote:
> linespec_result is only ever allocated on the stack, so it's
> relatively easy to convert to having a constructor and a destructor.
> This patch makes this change.  This removes some cleanups.

LGTM, though there are still some references in comments to 
destroy_linespec_result that could be cleaned up.

Simon

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

* Re: [RFA v2 15/17] Change breakpoint event locations to  event_location_up
  2017-04-11 15:23 ` [RFA v2 15/17] Change breakpoint event locations to event_location_up Tom Tromey
@ 2017-04-12  2:25   ` Simon Marchi
  0 siblings, 0 replies; 36+ messages in thread
From: Simon Marchi @ 2017-04-12  2:25 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 2017-04-11 11:01, Tom Tromey wrote:
> This is a follow-up to an earlier patch.  It changes breakpoint's
> location and location_range_end members to be of type
> event_location_up, then fixes up the users.

LGTM.

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

* Re: [RFA v2 17/17] Change linespec_result::location to be an  event_location_up
  2017-04-11 15:01 ` [RFA v2 17/17] Change linespec_result::location to be an event_location_up Tom Tromey
@ 2017-04-12  2:39   ` Simon Marchi
  0 siblings, 0 replies; 36+ messages in thread
From: Simon Marchi @ 2017-04-12  2:39 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 2017-04-11 11:01, Tom Tromey wrote:
> This is a follow-up to another patch.  It changes
> linespec_result::location to be an event_location_up.

LGTM.

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

* Re: [RFA v2 00/17] miscellaneous C++-ification
  2017-04-11 15:02 [RFA v2 00/17] miscellaneous C++-ification Tom Tromey
                   ` (16 preceding siblings ...)
  2017-04-11 15:23 ` [RFA v2 15/17] Change breakpoint event locations to event_location_up Tom Tromey
@ 2017-04-12  2:49 ` Simon Marchi
  2017-04-12 11:38   ` Pedro Alves
  17 siblings, 1 reply; 36+ messages in thread
From: Simon Marchi @ 2017-04-12  2:49 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 2017-04-11 11:00, Tom Tromey wrote:
> This is version 2 of my miscellaneous C++-ification patch series.
> 
> I believe this version addresses all the earlier review comments.  It
> also adds a few patches to fix some areas pointed out by Simon.
> 
> Built and regtested by the buildbot.
> 
> Tom

I just glanced at patches 1-14, everything is fine with me there.

Simon

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

* Re: [RFA v2 05/17] Change increment_reading_symtab to return a scoped_restore
  2017-04-11 15:01 ` [RFA v2 05/17] Change increment_reading_symtab to return a scoped_restore Tom Tromey
@ 2017-04-12 11:16   ` Pedro Alves
  2017-04-12 14:31     ` Tom Tromey
  0 siblings, 1 reply; 36+ messages in thread
From: Pedro Alves @ 2017-04-12 11:16 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches

On 04/11/2017 04:01 PM, Tom Tromey wrote:
> -struct cleanup *
> +scoped_restore_tmpl<int>
>  increment_reading_symtab (void)
>  {
> -  ++currently_reading_symtab;
> +  return make_scoped_restore (&currently_reading_symtab,
> +			      currently_reading_symtab + 1);
>    gdb_assert (currently_reading_symtab > 0);
> -  return make_cleanup (decrement_reading_symtab, NULL);
>  }

I noticed that this makes the gdb_assert unreachable.

Thanks,
Pedro Alves

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

* Re: [RFA v2 06/17] Remove cleanup_iconv
  2017-04-11 15:21 ` [RFA v2 06/17] Remove cleanup_iconv Tom Tromey
@ 2017-04-12 11:19   ` Pedro Alves
  2017-04-12 15:05     ` Tom Tromey
  0 siblings, 1 reply; 36+ messages in thread
From: Pedro Alves @ 2017-04-12 11:19 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches

On 04/11/2017 04:01 PM, Tom Tromey wrote:
> +  iconv_wrapper (const char *from, const char *to)
> +  {
> +    m_desc = iconv_open (to, from);

...

> -  desc = iconv_open (to, from);
> -  if (desc == (iconv_t) -1)
> -    perror_with_name (_("Converting character sets"));
> -  cleanups = make_cleanup (cleanup_iconv, &desc);
> +  iconv_wrapper desc (from, to);
>  

I find the inversion of the order of "from" and "to" potentially
confusing/misleading, given this is a "wrapper" class.  Is there a
rationale behind the swap?

Thanks,
Pedro Alves

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

* Re: [RFA v2 10/17] C++ify mi_parse
  2017-04-11 15:01 ` [RFA v2 10/17] C++ify mi_parse Tom Tromey
@ 2017-04-12 11:26   ` Pedro Alves
  2017-04-12 16:15     ` Tom Tromey
  0 siblings, 1 reply; 36+ messages in thread
From: Pedro Alves @ 2017-04-12 11:26 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches

On 04/11/2017 04:01 PM, Tom Tromey wrote:
> -  memset (parse, 0, sizeof (*parse));
> +  std::unique_ptr<struct mi_parse> parse (new struct mi_parse);
> +  memset (parse.get (), 0, sizeof (struct mi_parse));
>    parse->all = 0;

The patch made mi_parse be a non-POD (non-trivial dtor), so I
think we should get rid of that memset at the same time.

Thanks,
Pedro Alves

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

* Re: [RFA v2 00/17] miscellaneous C++-ification
  2017-04-12  2:49 ` [RFA v2 00/17] miscellaneous C++-ification Simon Marchi
@ 2017-04-12 11:38   ` Pedro Alves
  0 siblings, 0 replies; 36+ messages in thread
From: Pedro Alves @ 2017-04-12 11:38 UTC (permalink / raw)
  To: Simon Marchi, Tom Tromey; +Cc: gdb-patches

On 04/12/2017 03:49 AM, Simon Marchi wrote:
> On 2017-04-11 11:00, Tom Tromey wrote:
>> This is version 2 of my miscellaneous C++-ification patch series.
>>
>> I believe this version addresses all the earlier review comments.  It
>> also adds a few patches to fix some areas pointed out by Simon.
>>
>> Built and regtested by the buildbot.
>>
>> Tom
> 
> I just glanced at patches 1-14, everything is fine with me there.

I read the patches too and send a few comments pointing at some
easy-to-fix details.  Fix those and you're good to go, to me too.
No need to repost a whole v3 for those.

Thanks,
Pedro Alves

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

* Re: [RFA v2 16/17] Add a constructor and destructor to  linespec_result
  2017-04-12  2:25   ` Simon Marchi
@ 2017-04-12 14:30     ` Tom Tromey
  0 siblings, 0 replies; 36+ messages in thread
From: Tom Tromey @ 2017-04-12 14:30 UTC (permalink / raw)
  To: Simon Marchi; +Cc: Tom Tromey, gdb-patches

>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon> On 2017-04-11 11:01, Tom Tromey wrote:
>> linespec_result is only ever allocated on the stack, so it's
>> relatively easy to convert to having a constructor and a destructor.
>> This patch makes this change.  This removes some cleanups.

Simon> LGTM, though there are still some references in comments to
Simon> destroy_linespec_result that could be cleaned up.

I've changed these to refer to the linespec_result destructor instead.

Tom

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

* Re: [RFA v2 05/17] Change increment_reading_symtab to return a scoped_restore
  2017-04-12 11:16   ` Pedro Alves
@ 2017-04-12 14:31     ` Tom Tromey
  0 siblings, 0 replies; 36+ messages in thread
From: Tom Tromey @ 2017-04-12 14:31 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Tom Tromey, gdb-patches

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> I noticed that this makes the gdb_assert unreachable.

I changed the assert to ">= 0" and moved it before the return.

Tom

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

* Re: [RFA v2 06/17] Remove cleanup_iconv
  2017-04-12 11:19   ` Pedro Alves
@ 2017-04-12 15:05     ` Tom Tromey
  0 siblings, 0 replies; 36+ messages in thread
From: Tom Tromey @ 2017-04-12 15:05 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Tom Tromey, gdb-patches

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> I find the inversion of the order of "from" and "to" potentially
Pedro> confusing/misleading, given this is a "wrapper" class.  Is there a
Pedro> rationale behind the swap?

It mirrors the call to convert_between_encodings, but I agree with your
critique, so I've changed it.

Tom

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

* Re: [RFA v2 10/17] C++ify mi_parse
  2017-04-12 11:26   ` Pedro Alves
@ 2017-04-12 16:15     ` Tom Tromey
  2017-04-12 16:19       ` Pedro Alves
  0 siblings, 1 reply; 36+ messages in thread
From: Tom Tromey @ 2017-04-12 16:15 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Tom Tromey, gdb-patches

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> The patch made mi_parse be a non-POD (non-trivial dtor), so I
Pedro> think we should get rid of that memset at the same time.

This change was a little bigger than the others.  I've added this
constructor:

    mi_parse::mi_parse ()
      : op (MI_COMMAND),
        command (NULL),
        token (NULL),
        cmd (NULL),
        cmd_start (NULL),
        args (NULL),
        argv (NULL),
        argc (0),
        all (0),
        thread_group (-1),
        thread (-1),
        frame (-1),
        language (language_unknown)
    {
    }

... and changed the construction of the object like:

@@ -241,12 +241,5 @@
 
-  memset (parse, 0, sizeof (*parse));
-  parse->all = 0;
-  parse->thread_group = -1;
-  parse->thread = -1;
-  parse->frame = -1;
-  parse->language = language_unknown;
-
-  cleanup = make_cleanup (mi_parse_cleanup, parse);
+  std::unique_ptr<struct mi_parse> parse (new struct mi_parse);
 
   /* Before starting, skip leading white space.  */
   cmd = skip_spaces_const (cmd);

Tom

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

* Re: [RFA v2 10/17] C++ify mi_parse
  2017-04-12 16:15     ` Tom Tromey
@ 2017-04-12 16:19       ` Pedro Alves
  2017-04-12 18:05         ` Tom Tromey
  0 siblings, 1 reply; 36+ messages in thread
From: Pedro Alves @ 2017-04-12 16:19 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 04/12/2017 05:15 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> The patch made mi_parse be a non-POD (non-trivial dtor), so I
> Pedro> think we should get rid of that memset at the same time.
> 
> This change was a little bigger than the others.  I've added this
> constructor:
> 
>     mi_parse::mi_parse ()
>       : op (MI_COMMAND),
>         command (NULL),
>         token (NULL),
>         cmd (NULL),
>         cmd_start (NULL),
>         args (NULL),
>         argv (NULL),
>         argc (0),
>         all (0),
>         thread_group (-1),
>         thread (-1),
>         frame (-1),
>         language (language_unknown)
>     {
>     }
> 

Thanks.  Totally fine with me.  If you prefer, using in-class
initialization is also fine.  (We've been using it more in recent patches.)

> ... and changed the construction of the object like:
> 
> @@ -241,12 +241,5 @@
>  
> -  memset (parse, 0, sizeof (*parse));
> -  parse->all = 0;
> -  parse->thread_group = -1;
> -  parse->thread = -1;
> -  parse->frame = -1;
> -  parse->language = language_unknown;
> -
> -  cleanup = make_cleanup (mi_parse_cleanup, parse);
> +  std::unique_ptr<struct mi_parse> parse (new struct mi_parse);
>  

Perfect.

Thanks,
Pedro Alves

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

* Re: [RFA v2 10/17] C++ify mi_parse
  2017-04-12 16:19       ` Pedro Alves
@ 2017-04-12 18:05         ` Tom Tromey
  2017-04-12 18:37           ` Pedro Alves
  0 siblings, 1 reply; 36+ messages in thread
From: Tom Tromey @ 2017-04-12 18:05 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Tom Tromey, gdb-patches

Pedro> Thanks.  Totally fine with me.  If you prefer, using in-class
Pedro> initialization is also fine.  (We've been using it more in recent
Pedro> patches.)

I just left it as-is, but not for any good reason.

Tom

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

* Re: [RFA v2 10/17] C++ify mi_parse
  2017-04-12 18:05         ` Tom Tromey
@ 2017-04-12 18:37           ` Pedro Alves
  2017-04-12 19:25             ` Tom Tromey
  0 siblings, 1 reply; 36+ messages in thread
From: Pedro Alves @ 2017-04-12 18:37 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 04/12/2017 07:05 PM, Tom Tromey wrote:
> Pedro> Thanks.  Totally fine with me.  If you prefer, using in-class
> Pedro> initialization is also fine.  (We've been using it more in recent
> Pedro> patches.)
> 
> I just left it as-is, but not for any good reason.

That's totally fine with me.

FYI, I just now tried the hack below against master, and
that caught a few other cases that shouldn't have been
using memset for initialization.

struct bp_location, struct inferior, struct btrace_insn
$ make -k 2>&1 | grep "no matching.*pod_only_memset"
src/gdb/inferior.c:132:32: error: no matching function for call to ‘pod_only_memset(inferior*&, int, long unsigned int)’
src/gdb/btrace.c:1153:42: error: no matching function for call to ‘pod_only_memset(btrace_insn*, int, long unsigned int)’
src/gdb/breakpoint.c:951:53: error: no matching function for call to ‘pod_only_memset(bp_location*, int, long unsigned int)’
src/gdb/breakpoint.c:7325:32: error: no matching function for call to ‘pod_only_memset(bp_location*&, int, long unsigned int)’

I've already posted a patch to fix struct inferior:
  https://sourceware.org/ml/gdb-patches/2017-04/msg00298.html

I hadn't realized it already wasn't a POD.  Looks like that
happened because inferior has an enum_flags member, and that
one was recently made non-POD (gained a user-defined ctor to
default-zero-initialize).

I'll take a look at the others...

I wonder how bad would it be to put this hack in master.  Guess
we could always add it behind an #if 0 at least, to make it easy
to enable for quick checking?

From f8b5c40ff07891eb607dba4233419ca4e4295d22 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Wed, 12 Apr 2017 19:28:40 +0100
Subject: [PATCH] Make the compiler error out with memset on non-POD types

---
 gdb/common/common-defs.h | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/gdb/common/common-defs.h b/gdb/common/common-defs.h
index af37111..fe94000 100644
--- a/gdb/common/common-defs.h
+++ b/gdb/common/common-defs.h
@@ -90,4 +90,20 @@
 /* Pull in gdb::unique_xmalloc_ptr.  */
 #include "common/gdb_unique_ptr.h"
 
+#include <type_traits>
+
+/* Redefine memset to only work with POD types.  This catches
+   initialization of structs that should have been converted to
+   ctors.  */
+template <typename T,
+	  typename = typename std::enable_if<std::is_pod<T>::value
+					     || std::is_void<T>::value>::type>
+static inline void *
+pod_only_memset (T *s, int c, size_t n)
+{
+  return memset (s, c, n);
+}
+
+#define memset pod_only_memset
+
 #endif /* COMMON_DEFS_H */
-- 
2.5.5


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

* Re: [RFA v2 10/17] C++ify mi_parse
  2017-04-12 18:37           ` Pedro Alves
@ 2017-04-12 19:25             ` Tom Tromey
  2017-04-13  2:36               ` Pedro Alves
  0 siblings, 1 reply; 36+ messages in thread
From: Tom Tromey @ 2017-04-12 19:25 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Tom Tromey, gdb-patches

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> FYI, I just now tried the hack below against master, and
Pedro> that caught a few other cases that shouldn't have been
Pedro> using memset for initialization.
[...]
Pedro> I wonder how bad would it be to put this hack in master.  Guess
Pedro> we could always add it behind an #if 0 at least, to make it easy
Pedro> to enable for quick checking?

I think it would be helpful if the compiler could warn about missing
member initializations in a constructor.  That would make something like
the mi_parse change more robust, which I think is the reason for the
memsets in the first place.

Tom

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

* Re: [RFA v2 10/17] C++ify mi_parse
  2017-04-12 19:25             ` Tom Tromey
@ 2017-04-13  2:36               ` Pedro Alves
  2017-04-13 13:44                 ` Tom Tromey
  0 siblings, 1 reply; 36+ messages in thread
From: Pedro Alves @ 2017-04-13  2:36 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 04/12/2017 08:25 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> FYI, I just now tried the hack below against master, and
> Pedro> that caught a few other cases that shouldn't have been
> Pedro> using memset for initialization.
> [...]
> Pedro> I wonder how bad would it be to put this hack in master.  Guess
> Pedro> we could always add it behind an #if 0 at least, to make it easy
> Pedro> to enable for quick checking?
> 
> I think it would be helpful if the compiler could warn about missing
> member initializations in a constructor.

Yeah, that would be nice.  In principle, -Wuninitialized would catch
those too, and be quiet if you have some field that you do want to
be left uninitialized in the ctor (e.g., some big array lazily
filled in).  (And for OOL ctors, there's -flto.)
There are unfortunately a number of bugs with it, though.
The last I ran into was:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79658

> That would make something like
> the mi_parse change more robust, which I think is the reason for the
> memsets in the first place.

Yeah.  The memsets become invalid as soon as your ctor
does something non-trivial though, we really need to zap them
sooner than later.  My current thinking is that in-class initialization
helps by making it easier to check whether all fields have default
initialization.

I've posted a series fixing the issues the hack caught, and some more,
along with a better version of the hack, here:

 https://sourceware.org/ml/gdb-patches/2017-04/msg00378.html

Thanks,
Pedro Alves

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

* Re: [RFA v2 10/17] C++ify mi_parse
  2017-04-13  2:36               ` Pedro Alves
@ 2017-04-13 13:44                 ` Tom Tromey
  0 siblings, 0 replies; 36+ messages in thread
From: Tom Tromey @ 2017-04-13 13:44 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Tom Tromey, gdb-patches

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> Yeah.  The memsets become invalid as soon as your ctor
Pedro> does something non-trivial though, we really need to zap them
Pedro> sooner than later.

Yes, I agree.

Tom

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

end of thread, other threads:[~2017-04-13 13:44 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-11 15:02 [RFA v2 00/17] miscellaneous C++-ification Tom Tromey
2017-04-11 15:01 ` [RFA v2 09/17] Remove some cleanups from location.c Tom Tromey
2017-04-11 15:01 ` [RFA v2 10/17] C++ify mi_parse Tom Tromey
2017-04-12 11:26   ` Pedro Alves
2017-04-12 16:15     ` Tom Tromey
2017-04-12 16:19       ` Pedro Alves
2017-04-12 18:05         ` Tom Tromey
2017-04-12 18:37           ` Pedro Alves
2017-04-12 19:25             ` Tom Tromey
2017-04-13  2:36               ` Pedro Alves
2017-04-13 13:44                 ` Tom Tromey
2017-04-11 15:01 ` [RFA v2 17/17] Change linespec_result::location to be an event_location_up Tom Tromey
2017-04-12  2:39   ` Simon Marchi
2017-04-11 15:01 ` [RFA v2 16/17] Add a constructor and destructor to linespec_result Tom Tromey
2017-04-12  2:25   ` Simon Marchi
2017-04-12 14:30     ` Tom Tromey
2017-04-11 15:01 ` [RFA v2 11/17] Use scoped_restore in more places Tom Tromey
2017-04-11 15:01 ` [RFA v2 05/17] Change increment_reading_symtab to return a scoped_restore Tom Tromey
2017-04-12 11:16   ` Pedro Alves
2017-04-12 14:31     ` Tom Tromey
2017-04-11 15:01 ` [RFA v2 12/17] Use std::vector in reread_symbols Tom Tromey
2017-04-11 15:01 ` [RFA v2 01/17] Introduce event_location_up Tom Tromey
2017-04-11 15:01 ` [RFA v2 04/17] Introduce gdb_dlhandle_up Tom Tromey
2017-04-11 15:01 ` [RFA v2 02/17] Introduce command_line_up Tom Tromey
2017-04-11 15:02 ` [RFA v2 08/17] Remove some cleanups from gnu-v3-abi.c Tom Tromey
2017-04-11 15:02 ` [RFA v2 07/17] Fix up wchar_iterator comment Tom Tromey
2017-04-11 15:02 ` [RFA v2 13/17] Use std::vector in find_instruction_backward Tom Tromey
2017-04-11 15:21 ` [RFA v2 03/17] Change find_pcs_for_symtab_line to return a std::vector Tom Tromey
2017-04-11 15:21 ` [RFA v2 06/17] Remove cleanup_iconv Tom Tromey
2017-04-12 11:19   ` Pedro Alves
2017-04-12 15:05     ` Tom Tromey
2017-04-11 15:22 ` [RFA v2 14/17] Use std::vector in compile-loc2c.c Tom Tromey
2017-04-11 15:23 ` [RFA v2 15/17] Change breakpoint event locations to event_location_up Tom Tromey
2017-04-12  2:25   ` Simon Marchi
2017-04-12  2:49 ` [RFA v2 00/17] miscellaneous C++-ification Simon Marchi
2017-04-12 11:38   ` Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).