This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v2] RAII-fy make_cleanup_restore_current_thread & friends
- From: Sergio Durigan Junior <sergiodj at redhat dot com>
- To: Pedro Alves <palves at redhat dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Tue, 25 Apr 2017 15:50:24 -0400
- Subject: Re: [PATCH v2] RAII-fy make_cleanup_restore_current_thread & friends
- Authentication-results: sourceware.org; auth=none
- Authentication-results: ext-mx05.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com
- Authentication-results: ext-mx05.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=sergiodj at redhat dot com
- Dkim-filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 27CF623E6E4
- Dmarc-filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 27CF623E6E4
- References: <1492618687-11704-1-git-send-email-palves@redhat.com>
On Wednesday, April 19 2017, Pedro Alves wrote:
> New in v2:
> - v1 had missed deleting save_current_program_space / restore_program_space.
>
> After all the make_cleanup_restore_current_thread fixing, I thought
> I'd convert that and its relatives (which are all cleanups) to RAII
> classes.
>
> scoped_restore_current_pspace_and_thread was put in a separate file to
> avoid a circular dependency.
>
> Tested on x86-64 Fedora 23, native and gdbserver.
Nice cleanup, thanks for that. I just have one comment.
>[...]
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index 4940ec2..09d5d5c 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -80,6 +80,7 @@
> #include "mi/mi-common.h"
> #include "extension.h"
> #include <algorithm>
> +#include "progspace-and-thread.h"
>
> /* Enums for exception-handling support. */
> enum exception_event_kind
> @@ -3066,7 +3067,7 @@ update_inserted_breakpoint_locations (void)
> there was an error. */
> tmp_error_stream.puts ("Warning:\n");
>
> - struct cleanup *cleanups = save_current_space_and_thread ();
> + scoped_restore_current_pspace_and_thread restore;
"restore" doesn't seem like a very good name; I think it's too generic.
How about naming the variable "restore_pspace_thread" or something like
that, like you did with other variables?
> [...]
Otherwise, LGTM.
Thanks,
--
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/