[RFA] Remove cleanups from check_fast_tracepoint_sals

Message ID 20180222170220.27190-1-tom@tromey.com
State New
Headers show
Series
  • [RFA] Remove cleanups from check_fast_tracepoint_sals
Related show

Commit Message

Tom Tromey Feb. 22, 2018, 5:02 p.m.
This changes the gdbarch fast_tracepoint_valid_at method to use a
std::string as its out parameter, and then updates all the uses.  This
allows removing a cleanup from breakpoint.c.

Regression tested by the buildbot.

ChangeLog
2018-02-21  Tom Tromey  <tom@tromey.com>

	* i386-tdep.c (i386_fast_tracepoint_valid_at): "msg" now a
	std::string.
	* gdbarch.sh (fast_tracepoint_valid_at): Change "msg" to a
	std::string*.
	* gdbarch.c: Rebuild.
	* gdbarch.h: Rebuild.
	* breakpoint.c (check_fast_tracepoint_sals): Use std::string.
	* arch-utils.h (default_fast_tracepoint_valid_at): Update.
	* arch-utils.c (default_fast_tracepoint_valid_at): "msg" now a
	std::string*.
---
 gdb/ChangeLog    | 13 +++++++++++++
 gdb/arch-utils.c |  4 ++--
 gdb/arch-utils.h |  2 +-
 gdb/breakpoint.c | 12 +++---------
 gdb/gdbarch.c    |  2 +-
 gdb/gdbarch.h    |  4 ++--
 gdb/gdbarch.sh   |  2 +-
 gdb/i386-tdep.c  | 10 +++++-----
 8 files changed, 28 insertions(+), 21 deletions(-)

-- 
2.13.6

Comments

Simon Marchi Feb. 24, 2018, 1:37 a.m. | #1
On 2018-02-22 12:02 PM, Tom Tromey wrote:
> This changes the gdbarch fast_tracepoint_valid_at method to use a

> std::string as its out parameter, and then updates all the uses.  This

> allows removing a cleanup from breakpoint.c.

> 

> Regression tested by the buildbot.

> 

> ChangeLog

> 2018-02-21  Tom Tromey  <tom@tromey.com>

> 

> 	* i386-tdep.c (i386_fast_tracepoint_valid_at): "msg" now a

> 	std::string.

> 	* gdbarch.sh (fast_tracepoint_valid_at): Change "msg" to a

> 	std::string*.

> 	* gdbarch.c: Rebuild.

> 	* gdbarch.h: Rebuild.

> 	* breakpoint.c (check_fast_tracepoint_sals): Use std::string.

> 	* arch-utils.h (default_fast_tracepoint_valid_at): Update.

> 	* arch-utils.c (default_fast_tracepoint_valid_at): "msg" now a

> 	std::string*.

> ---

>  gdb/ChangeLog    | 13 +++++++++++++

>  gdb/arch-utils.c |  4 ++--

>  gdb/arch-utils.h |  2 +-

>  gdb/breakpoint.c | 12 +++---------

>  gdb/gdbarch.c    |  2 +-

>  gdb/gdbarch.h    |  4 ++--

>  gdb/gdbarch.sh   |  2 +-

>  gdb/i386-tdep.c  | 10 +++++-----

>  8 files changed, 28 insertions(+), 21 deletions(-)

> 

> diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c

> index 693d7e3dc8..2ff0f4d623 100644

> --- a/gdb/arch-utils.c

> +++ b/gdb/arch-utils.c

> @@ -813,12 +813,12 @@ default_has_shared_address_space (struct gdbarch *gdbarch)

>  

>  int

>  default_fast_tracepoint_valid_at (struct gdbarch *gdbarch, CORE_ADDR addr,

> -				  char **msg)

> +				  std::string *msg)

>  {

>    /* We don't know if maybe the target has some way to do fast

>       tracepoints that doesn't need gdbarch, so always say yes.  */

>    if (msg)

> -    *msg = NULL;

> +    msg->clear ();

>    return 1;

>  }

>  

> diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h

> index b51a4ec1ee..3407a165e0 100644

> --- a/gdb/arch-utils.h

> +++ b/gdb/arch-utils.h

> @@ -202,7 +202,7 @@ extern struct gdbarch *get_current_arch (void);

>  extern int default_has_shared_address_space (struct gdbarch *);

>  

>  extern int default_fast_tracepoint_valid_at (struct gdbarch *gdbarch,

> -					     CORE_ADDR addr, char **msg);

> +					     CORE_ADDR addr, std::string *msg);

>  

>  extern const gdb_byte *default_breakpoint_from_pc (struct gdbarch *gdbarch,

>  						   CORE_ADDR *pcptr,

> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c

> index 91ecca62fc..61c86fe5fc 100644

> --- a/gdb/breakpoint.c

> +++ b/gdb/breakpoint.c

> @@ -9178,8 +9178,6 @@ check_fast_tracepoint_sals (struct gdbarch *gdbarch,

>  			    gdb::array_view<const symtab_and_line> sals)

>  {

>    int rslt;


The rslt variable can be removed.

Otherwise, LGTM.

Simon

Patch

diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
index 693d7e3dc8..2ff0f4d623 100644
--- a/gdb/arch-utils.c
+++ b/gdb/arch-utils.c
@@ -813,12 +813,12 @@  default_has_shared_address_space (struct gdbarch *gdbarch)
 
 int
 default_fast_tracepoint_valid_at (struct gdbarch *gdbarch, CORE_ADDR addr,
-				  char **msg)
+				  std::string *msg)
 {
   /* We don't know if maybe the target has some way to do fast
      tracepoints that doesn't need gdbarch, so always say yes.  */
   if (msg)
-    *msg = NULL;
+    msg->clear ();
   return 1;
 }
 
diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h
index b51a4ec1ee..3407a165e0 100644
--- a/gdb/arch-utils.h
+++ b/gdb/arch-utils.h
@@ -202,7 +202,7 @@  extern struct gdbarch *get_current_arch (void);
 extern int default_has_shared_address_space (struct gdbarch *);
 
 extern int default_fast_tracepoint_valid_at (struct gdbarch *gdbarch,
-					     CORE_ADDR addr, char **msg);
+					     CORE_ADDR addr, std::string *msg);
 
 extern const gdb_byte *default_breakpoint_from_pc (struct gdbarch *gdbarch,
 						   CORE_ADDR *pcptr,
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 91ecca62fc..61c86fe5fc 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -9178,8 +9178,6 @@  check_fast_tracepoint_sals (struct gdbarch *gdbarch,
 			    gdb::array_view<const symtab_and_line> sals)
 {
   int rslt;
-  char *msg;
-  struct cleanup *old_chain;
 
   for (const auto &sal : sals)
     {
@@ -9190,14 +9188,10 @@  check_fast_tracepoint_sals (struct gdbarch *gdbarch,
 	 associated with SAL.  */
       if (sarch == NULL)
 	sarch = gdbarch;
-      rslt = gdbarch_fast_tracepoint_valid_at (sarch, sal.pc, &msg);
-      old_chain = make_cleanup (xfree, msg);
-
-      if (!rslt)
+      std::string msg;
+      if (!gdbarch_fast_tracepoint_valid_at (sarch, sal.pc, &msg))
 	error (_("May not have a fast tracepoint at %s%s"),
-	       paddress (sarch, sal.pc), (msg ? msg : ""));
-
-      do_cleanups (old_chain);
+	       paddress (sarch, sal.pc), msg.c_str ());
     }
 }
 
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index 62cb9a5d88..b8703e5a55 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -4650,7 +4650,7 @@  set_gdbarch_has_shared_address_space (struct gdbarch *gdbarch,
 }
 
 int
-gdbarch_fast_tracepoint_valid_at (struct gdbarch *gdbarch, CORE_ADDR addr, char **msg)
+gdbarch_fast_tracepoint_valid_at (struct gdbarch *gdbarch, CORE_ADDR addr, std::string *msg)
 {
   gdb_assert (gdbarch != NULL);
   gdb_assert (gdbarch->fast_tracepoint_valid_at != NULL);
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 30c2bf3af3..5cb131de1d 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -1366,8 +1366,8 @@  extern void set_gdbarch_has_shared_address_space (struct gdbarch *gdbarch, gdbar
 
 /* True if a fast tracepoint can be set at an address. */
 
-typedef int (gdbarch_fast_tracepoint_valid_at_ftype) (struct gdbarch *gdbarch, CORE_ADDR addr, char **msg);
-extern int gdbarch_fast_tracepoint_valid_at (struct gdbarch *gdbarch, CORE_ADDR addr, char **msg);
+typedef int (gdbarch_fast_tracepoint_valid_at_ftype) (struct gdbarch *gdbarch, CORE_ADDR addr, std::string *msg);
+extern int gdbarch_fast_tracepoint_valid_at (struct gdbarch *gdbarch, CORE_ADDR addr, std::string *msg);
 extern void set_gdbarch_fast_tracepoint_valid_at (struct gdbarch *gdbarch, gdbarch_fast_tracepoint_valid_at_ftype *fast_tracepoint_valid_at);
 
 /* Guess register state based on tracepoint location.  Used for tracepoints
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index 10a2aa9f6f..33dfa6b349 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -1045,7 +1045,7 @@  v;int;has_global_breakpoints;;;0;0;;0
 m;int;has_shared_address_space;void;;;default_has_shared_address_space;;0
 
 # True if a fast tracepoint can be set at an address.
-m;int;fast_tracepoint_valid_at;CORE_ADDR addr, char **msg;addr, msg;;default_fast_tracepoint_valid_at;;0
+m;int;fast_tracepoint_valid_at;CORE_ADDR addr, std::string *msg;addr, msg;;default_fast_tracepoint_valid_at;;0
 
 # Guess register state based on tracepoint location.  Used for tracepoints
 # where no registers have been collected, but there's only one location,
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 6b59278e22..60dc8013a2 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -8111,7 +8111,7 @@  static const int i386_record_regmap[] =
 
 static int
 i386_fast_tracepoint_valid_at (struct gdbarch *gdbarch, CORE_ADDR addr,
-			       char **msg)
+			       std::string *msg)
 {
   int len, jumplen;
 
@@ -8144,15 +8144,15 @@  i386_fast_tracepoint_valid_at (struct gdbarch *gdbarch, CORE_ADDR addr,
       /* Return a bit of target-specific detail to add to the caller's
 	 generic failure message.  */
       if (msg)
-	*msg = xstrprintf (_("; instruction is only %d bytes long, "
-			     "need at least %d bytes for the jump"),
-			   len, jumplen);
+	*msg = string_printf (_("; instruction is only %d bytes long, "
+				"need at least %d bytes for the jump"),
+			      len, jumplen);
       return 0;
     }
   else
     {
       if (msg)
-	*msg = NULL;
+	msg->clear ();
       return 1;
     }
 }