[RFA] Remove cleanups from check_fast_tracepoint_sals
Commit Message
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(-)
Comments
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
@@ -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;
}
@@ -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,
@@ -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 ());
}
}
@@ -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);
@@ -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
@@ -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,
@@ -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;
}
}