* [RFA] Remove a cleanup from scm-frame.c
@ 2018-04-21 22:39 Tom Tromey
2018-04-22 14:21 ` Simon Marchi
0 siblings, 1 reply; 4+ messages in thread
From: Tom Tromey @ 2018-04-21 22:39 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This removes a cleanup from scm-frame.c, replacing it with
unique_xmalloc_ptr and a new scope. I believe this also fixes a
latent bug involving calling do_cleanups twice for a single cleanup.
Regression tested using the gdb.guile test suite on x86-64 Fedora 26.
ChangeLog
2018-04-21 Tom Tromey <tom@tromey.com>
* guile/scm-frame.c (gdbscm_frame_read_var): Use
gdb::unique_xmalloc_ptr.
---
gdb/ChangeLog | 5 +++++
gdb/guile/scm-frame.c | 54 ++++++++++++++++++++++++---------------------------
2 files changed, 30 insertions(+), 29 deletions(-)
diff --git a/gdb/guile/scm-frame.c b/gdb/guile/scm-frame.c
index 4f4766aceb..7b539677ff 100644
--- a/gdb/guile/scm-frame.c
+++ b/gdb/guile/scm-frame.c
@@ -877,7 +877,6 @@ gdbscm_frame_read_var (SCM self, SCM symbol_scm, SCM rest)
}
else if (scm_is_string (symbol_scm))
{
- char *var_name;
const struct block *block = NULL;
struct cleanup *cleanup;
struct gdb_exception except = exception_none;
@@ -893,38 +892,35 @@ gdbscm_frame_read_var (SCM self, SCM symbol_scm, SCM rest)
gdbscm_throw (except_scm);
}
- var_name = gdbscm_scm_to_c_string (symbol_scm);
- cleanup = make_cleanup (xfree, var_name);
- /* N.B. Between here and the call to do_cleanups, don't do anything
- to cause a Scheme exception without performing the cleanup. */
+ {
+ gdb::unique_xmalloc_ptr<char> var_name
+ (gdbscm_scm_to_c_string (symbol_scm));
+ /* N.B. Between here and the end of the scope, don't do anything
+ to cause a Scheme exception. */
+
+ TRY
+ {
+ struct block_symbol lookup_sym;
+
+ if (block == NULL)
+ block = get_frame_block (frame, NULL);
+ lookup_sym = lookup_symbol (var_name.get (), block, VAR_DOMAIN,
+ NULL);
+ var = lookup_sym.symbol;
+ block = lookup_sym.block;
+ }
+ CATCH (ex, RETURN_MASK_ALL)
+ {
+ except = ex;
+ }
+ END_CATCH
+ }
- TRY
- {
- struct block_symbol lookup_sym;
-
- if (block == NULL)
- block = get_frame_block (frame, NULL);
- lookup_sym = lookup_symbol (var_name, block, VAR_DOMAIN, NULL);
- var = lookup_sym.symbol;
- block = lookup_sym.block;
- }
- CATCH (ex, RETURN_MASK_ALL)
- {
- except = ex;
- }
- END_CATCH
-
- do_cleanups (cleanup);
GDBSCM_HANDLE_GDB_EXCEPTION (except);
if (var == NULL)
- {
- do_cleanups (cleanup);
- gdbscm_out_of_range_error (FUNC_NAME, 0, symbol_scm,
- _("variable not found"));
- }
-
- do_cleanups (cleanup);
+ gdbscm_out_of_range_error (FUNC_NAME, 0, symbol_scm,
+ _("variable not found"));
}
else
{
--
2.13.6
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFA] Remove a cleanup from scm-frame.c
2018-04-21 22:39 [RFA] Remove a cleanup from scm-frame.c Tom Tromey
@ 2018-04-22 14:21 ` Simon Marchi
2018-04-22 23:47 ` Tom Tromey
0 siblings, 1 reply; 4+ messages in thread
From: Simon Marchi @ 2018-04-22 14:21 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On 2018-04-21 18:39, Tom Tromey wrote:
> This removes a cleanup from scm-frame.c, replacing it with
> unique_xmalloc_ptr and a new scope. I believe this also fixes a
> latent bug involving calling do_cleanups twice for a single cleanup.
>
> Regression tested using the gdb.guile test suite on x86-64 Fedora 26.
>
> ChangeLog
> 2018-04-21 Tom Tromey <tom@tromey.com>
>
> * guile/scm-frame.c (gdbscm_frame_read_var): Use
> gdb::unique_xmalloc_ptr.
> ---
> gdb/ChangeLog | 5 +++++
> gdb/guile/scm-frame.c | 54
> ++++++++++++++++++++++++---------------------------
> 2 files changed, 30 insertions(+), 29 deletions(-)
>
> diff --git a/gdb/guile/scm-frame.c b/gdb/guile/scm-frame.c
> index 4f4766aceb..7b539677ff 100644
> --- a/gdb/guile/scm-frame.c
> +++ b/gdb/guile/scm-frame.c
> @@ -877,7 +877,6 @@ gdbscm_frame_read_var (SCM self, SCM symbol_scm,
> SCM rest)
> }
> else if (scm_is_string (symbol_scm))
> {
> - char *var_name;
> const struct block *block = NULL;
> struct cleanup *cleanup;
> struct gdb_exception except = exception_none;
> @@ -893,38 +892,35 @@ gdbscm_frame_read_var (SCM self, SCM symbol_scm,
> SCM rest)
> gdbscm_throw (except_scm);
> }
>
> - var_name = gdbscm_scm_to_c_string (symbol_scm);
> - cleanup = make_cleanup (xfree, var_name);
> - /* N.B. Between here and the call to do_cleanups, don't do
> anything
> - to cause a Scheme exception without performing the cleanup. */
> + {
> + gdb::unique_xmalloc_ptr<char> var_name
> + (gdbscm_scm_to_c_string (symbol_scm));
> + /* N.B. Between here and the end of the scope, don't do anything
> + to cause a Scheme exception. */
> +
> + TRY
> + {
> + struct block_symbol lookup_sym;
> +
> + if (block == NULL)
> + block = get_frame_block (frame, NULL);
> + lookup_sym = lookup_symbol (var_name.get (), block, VAR_DOMAIN,
> + NULL);
> + var = lookup_sym.symbol;
> + block = lookup_sym.block;
> + }
> + CATCH (ex, RETURN_MASK_ALL)
> + {
> + except = ex;
> + }
> + END_CATCH
> + }
>
> - TRY
> - {
> - struct block_symbol lookup_sym;
> -
> - if (block == NULL)
> - block = get_frame_block (frame, NULL);
> - lookup_sym = lookup_symbol (var_name, block, VAR_DOMAIN, NULL);
> - var = lookup_sym.symbol;
> - block = lookup_sym.block;
> - }
> - CATCH (ex, RETURN_MASK_ALL)
> - {
> - except = ex;
> - }
> - END_CATCH
> -
> - do_cleanups (cleanup);
> GDBSCM_HANDLE_GDB_EXCEPTION (except);
>
> if (var == NULL)
> - {
> - do_cleanups (cleanup);
> - gdbscm_out_of_range_error (FUNC_NAME, 0, symbol_scm,
> - _("variable not found"));
> - }
> -
> - do_cleanups (cleanup);
> + gdbscm_out_of_range_error (FUNC_NAME, 0, symbol_scm,
> + _("variable not found"));
> }
> else
> {
This looks good to me at first glance. Do you know if scm exceptions
(scm_throw) play well with C++, the destructors of the objects in the
exited scopes will correctly be called?
Simon
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFA] Remove a cleanup from scm-frame.c
2018-04-22 14:21 ` Simon Marchi
@ 2018-04-22 23:47 ` Tom Tromey
0 siblings, 0 replies; 4+ messages in thread
From: Tom Tromey @ 2018-04-22 23:47 UTC (permalink / raw)
To: Simon Marchi; +Cc: Tom Tromey, gdb-patches
>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
Simon> This looks good to me at first glance. Do you know if scm exceptions
Simon> (scm_throw) play well with C++, the destructors of the objects in the
Simon> exited scopes will correctly be called?
I don't believe so, which is why this patch introduces a new scope.
Tom
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFA] Remove a cleanup from scm-frame.c
@ 2018-04-23 1:40 Simon Marchi
0 siblings, 0 replies; 4+ messages in thread
From: Simon Marchi @ 2018-04-23 1:40 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On 2018-04-22 19:47, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
>
> Simon> This looks good to me at first glance. Do you know if scm
> exceptions
> Simon> (scm_throw) play well with C++, the destructors of the objects
> in the
> Simon> exited scopes will correctly be called?
>
> I don't believe so, which is why this patch introduces a new scope.
>
> Tom
Ahh, I didn't catch that initially, and it makes sense with the comment.
LGTM then!
Simon
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-04-23 1:40 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-21 22:39 [RFA] Remove a cleanup from scm-frame.c Tom Tromey
2018-04-22 14:21 ` Simon Marchi
2018-04-22 23:47 ` Tom Tromey
2018-04-23 1:40 Simon Marchi
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).