diff mbox

[RFA] Remove cleanups from ser-mingw.c

Message ID 20180614212047.14265-1-tom@tromey.com
State New, archived
Headers

Commit Message

Tom Tromey June 14, 2018, 9:20 p.m. UTC 
  This removes the only cleanup from ser-mingw.c, replacing it with a
specialization of std::unique_ptr.

Tested by the buildbot.

gdb/ChangeLog
2018-06-14  Tom Tromey  <tom@tromey.com>

	* ser-mingw.c (struct pipe_state_destroyer): New.
	(pipe_state_up): New typedef.
	(cleanup_pipe_state): Remove.
	(pipe_windows_open): Use pipe_state_up.
---
 gdb/ChangeLog   |  7 +++++++
 gdb/ser-mingw.c | 21 +++++++++------------
 2 files changed, 16 insertions(+), 12 deletions(-)

Comments

Simon Marchi June 18, 2018, 3:16 a.m. UTC | #1 
Hi Tom,

LGTM, with some nits.

On 2018-06-14 05:20 PM, Tom Tromey wrote:
> @@ -914,14 +913,12 @@ pipe_windows_open (struct serial *scb, const char *name)
>      goto fail;
>    scb->error_fd = fileno (pex_stderr);
>  
> -  scb->state = (void *) ps;
> +  scb->state = (void *) ps.release ();

- I think we could remove the cast, unless you think it helps readability.
- I think we could remove the goto fail mechanism now.

>  
>    argv.release ();

Side note: doesn't this argv.release() look fishy to you?  Who ends up owning it?

Simon
Tom Tromey June 18, 2018, 2:17 p.m. UTC | #2 
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

>> -  scb->state = (void *) ps;
>> +  scb->state = (void *) ps.release ();

Simon> - I think we could remove the cast, unless you think it helps readability.
Simon> - I think we could remove the goto fail mechanism now.

Agreed on both counts, I made this change.

>> argv.release ();

Simon> Side note: doesn't this argv.release() look fishy to you?  Who ends up owning it?

It is fishy.  I looked at the history and it seems that when I added
gdb_argv here, I kept things working the same; I suppose without looking
too hard.  I looked and PEX does not seem to take ownership of the
arguments, so removing this line seems correct to me, and I've done that
in the version that I will check in.

Tom
diff mbox

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 8150685e5c..d045385d26 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@ 
+2018-06-14  Tom Tromey  <tom@tromey.com>
+
+	* ser-mingw.c (struct pipe_state_destroyer): New.
+	(pipe_state_up): New typedef.
+	(cleanup_pipe_state): Remove.
+	(pipe_windows_open): Use pipe_state_up.
+
 2018-06-14  Tom de Vries  <tdevries@suse.de>
 
 	PR cli/22573
diff --git a/gdb/ser-mingw.c b/gdb/ser-mingw.c
index d5e5aabb18..c9c0fff2ef 100644
--- a/gdb/ser-mingw.c
+++ b/gdb/ser-mingw.c
@@ -848,20 +848,20 @@  free_pipe_state (struct pipe_state *ps)
   errno = saved_errno;
 }
 
-static void
-cleanup_pipe_state (void *untyped)
+struct pipe_state_destroyer
 {
-  struct pipe_state *ps = (struct pipe_state *) untyped;
+  void operator() (pipe_state *ps) const
+  {
+    free_pipe_state (ps);
+  }
+};
 
-  free_pipe_state (ps);
-}
+typedef std::unique_ptr<pipe_state, pipe_state_destroyer> pipe_state_up;
 
 static int
 pipe_windows_open (struct serial *scb, const char *name)
 {
-  struct pipe_state *ps;
   FILE *pex_stderr;
-  struct cleanup *back_to;
 
   if (name == NULL)
     error_no_arg (_("child command"));
@@ -871,8 +871,7 @@  pipe_windows_open (struct serial *scb, const char *name)
   if (! argv[0] || argv[0][0] == '\0')
     error (_("missing child command"));
 
-  ps = make_pipe_state ();
-  back_to = make_cleanup (cleanup_pipe_state, ps);
+  pipe_state_up ps (make_pipe_state ());
 
   ps->pex = pex_init (PEX_USE_PIPES, "target remote pipe", NULL);
   if (! ps->pex)
@@ -914,14 +913,12 @@  pipe_windows_open (struct serial *scb, const char *name)
     goto fail;
   scb->error_fd = fileno (pex_stderr);
 
-  scb->state = (void *) ps;
+  scb->state = (void *) ps.release ();
 
   argv.release ();
-  discard_cleanups (back_to);
   return 0;
 
  fail:
-  do_cleanups (back_to);
   return -1;
 }