Change serial "open" functions to throw exception

remote.c assumes that a failure to open the serial connection will set
errno.  This is somewhat true, because the Windows code tries to set
errno appropriately -- but only somewhat, because it isn't clear that
the "pex" code sets it, and the tcp code seems to do the wrong thing.
It seems better to simply have the serial open functions throw on
error.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30770

10 files changed, 90 insertions, 123 deletions

diff --git a/gdb/remote.c b/gdb/remote.c
index 00c84f1..03ebcc6 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -6077,8 +6077,6 @@ remote_target::open_1 (const char *name, int from_tty, int extended_p)
     rs->wait_forever_enabled_p = true;
 
   rs->remote_desc = remote_serial_open (name);
-  if (!rs->remote_desc)
-    perror_with_name (name);
 
   if (baud_rate != -1)
     {
diff --git a/gdb/ser-event.c b/gdb/ser-event.c
index 5865e84..fb02327 100644
--- a/gdb/ser-event.c
+++ b/gdb/ser-event.c
@@ -48,7 +48,7 @@ struct serial_event_state
 
 /* Open a new serial event.  */
 
-static int
+static void
 serial_event_open (struct serial *scb, const char *name)
 {
   struct serial_event_state *state;
@@ -85,8 +85,6 @@ serial_event_open (struct serial *scb, const char *name)
     scb->fd = _open_osfhandle ((intptr_t) dummy_file, 0);
   }
 #endif
-
-  return 0;
 }
 
 static void
diff --git a/gdb/ser-mingw.c b/gdb/ser-mingw.c
index 65ee169..30d9085 100644
--- a/gdb/ser-mingw.c
+++ b/gdb/ser-mingw.c
@@ -48,7 +48,7 @@ static CancelIo_ftype *CancelIo;
 
 /* Open up a real live device for serial I/O.  */
 
-static int
+static void
 ser_windows_open (struct serial *scb, const char *name)
 {
   HANDLE h;
@@ -59,22 +59,18 @@ ser_windows_open (struct serial *scb, const char *name)
 		  OPEN_EXISTING, FILE_FLAG_OVERLAPPED, NULL);
   if (h == INVALID_HANDLE_VALUE)
     {
-      errno = ENOENT;
-      return -1;
+      std::string msg = string_printf(_("could not open file: %s"),
+				      name);
+      throw_winerror_with_name (msg.c_str (), GetLastError ());
     }
 
   scb->fd = _open_osfhandle ((intptr_t) h, O_RDWR);
   if (scb->fd < 0)
-    {
-      errno = ENOENT;
-      return -1;
-    }
+    error (_("could not get underlying file descriptor"));
 
   if (!SetCommMask (h, EV_RXCHAR))
-    {
-      errno = EINVAL;
-      return -1;
-    }
+    throw_winerror_with_name (_("error calling SetCommMask"),
+			      GetLastError ());
 
   timeouts.ReadIntervalTimeout = MAXDWORD;
   timeouts.ReadTotalTimeoutConstant = 0;
@@ -82,10 +78,8 @@ ser_windows_open (struct serial *scb, const char *name)
   timeouts.WriteTotalTimeoutConstant = 0;
   timeouts.WriteTotalTimeoutMultiplier = 0;
   if (!SetCommTimeouts (h, &timeouts))
-    {
-      errno = EINVAL;
-      return -1;
-    }
+    throw_winerror_with_name (_("error calling SetCommTimeouts"),
+			      GetLastError ());
 
   state = XCNEW (struct ser_windows_state);
   scb->state = state;
@@ -95,8 +89,6 @@ ser_windows_open (struct serial *scb, const char *name)
 
   /* Create a (currently unused) handle to record exceptions.  */
   state->except_event = CreateEvent (0, TRUE, FALSE, 0);
-
-  return 0;
 }
 
 /* Wait for the output to drain away, as opposed to flushing (discarding)
@@ -860,7 +852,7 @@ struct pipe_state_destroyer
 
 typedef std::unique_ptr<pipe_state, pipe_state_destroyer> pipe_state_up;
 
-static int
+static void
 pipe_windows_open (struct serial *scb, const char *name)
 {
   FILE *pex_stderr;
@@ -883,10 +875,10 @@ pipe_windows_open (struct serial *scb, const char *name)
 
   ps->pex = pex_init (PEX_USE_PIPES, "target remote pipe", NULL);
   if (! ps->pex)
-    return -1;
+    error (_("could not start pipeline"));
   ps->input = pex_input_pipe (ps->pex, 1);
   if (! ps->input)
-    return -1;
+    error (_("could not find input pipe"));
 
   {
     int err;
@@ -913,17 +905,15 @@ pipe_windows_open (struct serial *scb, const char *name)
 
   ps->output = pex_read_output (ps->pex, 1);
   if (! ps->output)
-    return -1;
+    error (_("could not find output pipe"));
   scb->fd = fileno (ps->output);
 
   pex_stderr = pex_read_err (ps->pex, 1);
   if (! pex_stderr)
-    return -1;
+    error (_("could not find error pipe"));
   scb->error_fd = fileno (pex_stderr);
 
   scb->state = ps.release ();
-
-  return 0;
 }
 
 static int
@@ -1191,15 +1181,12 @@ net_windows_done_wait_handle (struct serial *scb)
   stop_select_thread (&state->base);
 }
 
-static int
+static void
 net_windows_open (struct serial *scb, const char *name)
 {
   struct net_windows_state *state;
-  int ret;
 
-  ret = net_open (scb, name);
-  if (ret != 0)
-    return ret;
+  net_open (scb, name);
 
   state = XCNEW (struct net_windows_state);
   scb->state = state;
@@ -1210,8 +1197,6 @@ net_windows_open (struct serial *scb, const char *name)
 
   /* Start the thread.  */
   create_select_thread (net_windows_select_thread, scb, &state->base);
-
-  return 0;
 }
 
 
diff --git a/gdb/ser-pipe.c b/gdb/ser-pipe.c
index 47ccd33..7bdaaa0 100644
--- a/gdb/ser-pipe.c
+++ b/gdb/ser-pipe.c
@@ -34,7 +34,6 @@
 
 #include <signal.h>
 
-static int pipe_open (struct serial *scb, const char *name);
 static void pipe_close (struct serial *scb);
 
 struct pipe_state
@@ -44,7 +43,7 @@ struct pipe_state
 
 /* Open up a raw pipe.  */
 
-static int
+static void
 pipe_open (struct serial *scb, const char *name)
 {
 #if !HAVE_SOCKETPAIR
@@ -69,12 +68,13 @@ pipe_open (struct serial *scb, const char *name)
     }
 
   if (gdb_socketpair_cloexec (AF_UNIX, SOCK_STREAM, 0, pdes) < 0)
-    return -1;
+    perror_with_name (_("could not open socket pair"));
   if (gdb_socketpair_cloexec (AF_UNIX, SOCK_STREAM, 0, err_pdes) < 0)
     {
+      int save = errno;
       close (pdes[0]);
       close (pdes[1]);
-      return -1;
+      perror_with_name (_("could not open socket pair"), save);
     }
 
   /* Create the child process to run the command in.  Note that the
@@ -86,11 +86,12 @@ pipe_open (struct serial *scb, const char *name)
   /* Error.  */
   if (pid == -1)
     {
+      int save = errno;
       close (pdes[0]);
       close (pdes[1]);
       close (err_pdes[0]);
       close (err_pdes[1]);
-      return -1;
+      perror_with_name (_("could not vfork"), save);
     }
 
   if (fcntl (err_pdes[0], F_SETFL, O_NONBLOCK) == -1)
@@ -148,7 +149,6 @@ pipe_open (struct serial *scb, const char *name)
 
   /* If we don't do this, GDB simply exits when the remote side dies.  */
   signal (SIGPIPE, SIG_IGN);
-  return 0;
 #endif
 }
 
diff --git a/gdb/ser-tcp.c b/gdb/ser-tcp.c
index 0367a5a..5e36840 100644
--- a/gdb/ser-tcp.c
+++ b/gdb/ser-tcp.c
@@ -84,11 +84,11 @@ static unsigned int tcp_retry_limit = 15;
 
 /* Helper function to wait a while.  If SOCK is not -1, wait on its
    file descriptor.  Otherwise just wait on a timeout, updating
-   *POLLS.  Returns -1 on timeout or interrupt, otherwise the value of
-   select.  */
+   *POLLS.  Returns -1 on timeout or interrupt and set OUT_ERROR,
+   otherwise the value of select.  */
 
 static int
-wait_for_connect (int sock, unsigned int *polls)
+wait_for_connect (int sock, unsigned int *polls, ULONGEST *out_error)
 {
   struct timeval t;
   int n;
@@ -98,14 +98,14 @@ wait_for_connect (int sock, unsigned int *polls)
      interrupt.  */
   if (deprecated_ui_loop_hook && deprecated_ui_loop_hook (0))
     {
-      errno = EINTR;
+      *out_error = EINTR;
       return -1;
     }
 
   /* Check for timeout.  */
   if (*polls > tcp_retry_limit * POLL_INTERVAL)
     {
-      errno = ETIMEDOUT;
+      *out_error = ETIMEDOUT;
       return -1;
     }
 
@@ -155,19 +155,34 @@ wait_for_connect (int sock, unsigned int *polls)
   return n;
 }
 
+/* A helper to get the error number for either Windows or POSIX.  */
+static ULONGEST
+get_error ()
+{
+#ifdef USE_WIN32API
+  return WSAGetLastError ();
+#else
+  return errno;
+#endif
+}
+
 /* Try to connect to the host represented by AINFO.  If the connection
-   succeeds, return its socket.  Otherwise, return -1 and set ERRNO
+   succeeds, return its socket.  Otherwise, return -1 and set OUT_ERROR
    accordingly.  POLLS is used when 'connect' returns EINPROGRESS, and
    we need to invoke 'wait_for_connect' to obtain the status.  */
 
 static int
-try_connect (const struct addrinfo *ainfo, unsigned int *polls)
+try_connect (const struct addrinfo *ainfo, unsigned int *polls,
+	     ULONGEST *out_error)
 {
   int sock = gdb_socket_cloexec (ainfo->ai_family, ainfo->ai_socktype,
 				 ainfo->ai_protocol);
 
   if (sock < 0)
-    return -1;
+    {
+      *out_error = get_error ();
+      return -1;
+    }
 
   /* Set socket nonblocking.  */
 #ifdef USE_WIN32API
@@ -182,11 +197,7 @@ try_connect (const struct addrinfo *ainfo, unsigned int *polls)
      already.  */
   if (connect (sock, ainfo->ai_addr, ainfo->ai_addrlen) < 0)
     {
-#ifdef USE_WIN32API
-      int err = WSAGetLastError();
-#else
-      int err = errno;
-#endif
+      ULONGEST err = get_error ();
 
       /* If we've got a "connection refused" error, just return
 	 -1.  The caller will know what to do.  */
@@ -199,7 +210,7 @@ try_connect (const struct addrinfo *ainfo, unsigned int *polls)
 	  )
 	{
 	  close (sock);
-	  errno = err;
+	  *out_error = err;
 	  return -1;
 	}
 
@@ -218,7 +229,7 @@ try_connect (const struct addrinfo *ainfo, unsigned int *polls)
 	  )
 	{
 	  close (sock);
-	  errno = err;
+	  *out_error = err;
 	  return -1;
 	}
 
@@ -226,17 +237,15 @@ try_connect (const struct addrinfo *ainfo, unsigned int *polls)
       int n;
 
       do
-	n = wait_for_connect (sock, polls);
+	n = wait_for_connect (sock, polls, out_error);
       while (n == 0);
 
       if (n < 0)
 	{
-	  int saved_errno = errno;
-
 	  /* A negative value here means that we either timed out or
 	     got interrupted by the user.  Just return.  */
 	  close (sock);
-	  errno = saved_errno;
+	  /* OUT_ERROR was set by wait_for_connect, above.  */
 	  return -1;
 	}
     }
@@ -253,16 +262,14 @@ try_connect (const struct addrinfo *ainfo, unsigned int *polls)
 
   if (ret < 0)
     {
-      int saved_errno = errno;
-
+      *out_error = get_error ();
       close (sock);
-      errno = saved_errno;
       return -1;
     }
   else if (ret == 0 && err != 0)
     {
+      *out_error = err;
       close (sock);
-      errno = err;
       return -1;
     }
 
@@ -272,7 +279,7 @@ try_connect (const struct addrinfo *ainfo, unsigned int *polls)
 
 /* Open a tcp socket.  */
 
-int
+void
 net_open (struct serial *scb, const char *name)
 {
   struct addrinfo hint;
@@ -295,12 +302,7 @@ net_open (struct serial *scb, const char *name)
 		       &hint, &ainfo);
 
   if (r != 0)
-    {
-      gdb_printf (gdb_stderr, _("%s: cannot resolve name: %s\n"),
-		  name, gai_strerror (r));
-      errno = ENOENT;
-      return -1;
-    }
+    error (_("%s: cannot resolve name: %s\n"), name, gai_strerror (r));
 
   scoped_free_addrinfo free_ainfo (ainfo);
 
@@ -311,6 +313,7 @@ net_open (struct serial *scb, const char *name)
      'struct addrinfo' that succeed.  */
   struct addrinfo *success_ainfo = NULL;
   unsigned int polls = 0;
+  ULONGEST last_error = 0;
 
   /* Assume the worst.  */
   scb->fd = -1;
@@ -324,7 +327,7 @@ net_open (struct serial *scb, const char *name)
 	  /* Iterate over the list of possible addresses to connect
 	     to.  For each, we'll try to connect and see if it
 	     succeeds.  */
-	  int sock = try_connect (iter, &polls);
+	  int sock = try_connect (iter, &polls, &last_error);
 
 	  if (sock >= 0)
 	    {
@@ -336,9 +339,9 @@ net_open (struct serial *scb, const char *name)
 	    }
 	  else if (
 #ifdef USE_WIN32API
-	  errno == WSAECONNREFUSED
+		   last_error == WSAECONNREFUSED
 #else
-	  errno == ECONNREFUSED
+		   last_error == ECONNREFUSED
 #endif
 		   )
 	    got_connrefused = true;
@@ -353,12 +356,16 @@ net_open (struct serial *scb, const char *name)
   while (tcp_auto_retry
 	 && success_ainfo == NULL
 	 && got_connrefused
-	 && wait_for_connect (-1, &polls) >= 0);
+	 && wait_for_connect (-1, &polls, &last_error) >= 0);
 
   if (success_ainfo == NULL)
     {
       net_close (scb);
-      return -1;
+#ifdef USE_WIN32API
+      throw_winerror_with_name (_("could not connect"), last_error);
+#else
+      perror_with_name (_("could not connect"), last_error);
+#endif
     }
 
   /* Turn off nonblocking.  */
@@ -384,8 +391,6 @@ net_open (struct serial *scb, const char *name)
      when the remote side dies.  */
   signal (SIGPIPE, SIG_IGN);
 #endif
-
-  return 0;
 }
 
 void
diff --git a/gdb/ser-tcp.h b/gdb/ser-tcp.h
index aeb4de0..444d77c 100644
--- a/gdb/ser-tcp.h
+++ b/gdb/ser-tcp.h
@@ -22,7 +22,7 @@
 
 struct serial;
 
-extern int net_open (struct serial *scb, const char *name);
+extern void net_open (struct serial *scb, const char *name);
 extern void net_close (struct serial *scb);
 extern int net_read_prim (struct serial *scb, size_t count);
 extern int net_write_prim (struct serial *scb, const void *buf, size_t count);
diff --git a/gdb/ser-uds.c b/gdb/ser-uds.c
index 665aab9..e51058b 100644
--- a/gdb/ser-uds.c
+++ b/gdb/ser-uds.c
@@ -30,36 +30,30 @@
 
 /* Open an AF_UNIX socket.  */
 
-static int
+static void
 uds_open (struct serial *scb, const char *name)
 {
   struct sockaddr_un addr;
 
   if (strlen (name) > UNIX_PATH_MAX - 1)
-    {
-      warning
-	(_("The socket name is too long.  It may be no longer than %s bytes."),
-	 pulongest (UNIX_PATH_MAX - 1L));
-      return -1;
-    }
+    error (_("The socket name is too long.  It may be no longer than %s bytes."),
+	   pulongest (UNIX_PATH_MAX - 1L));
 
   memset (&addr, 0, sizeof addr);
   addr.sun_family = AF_UNIX;
   strncpy (addr.sun_path, name, UNIX_PATH_MAX - 1);
 
-  int sock = socket (AF_UNIX, SOCK_STREAM, 0);
+  scb->fd = socket (AF_UNIX, SOCK_STREAM, 0);
+  if (scb->fd < 0)
+    perror_with_name (_("could not open socket"));
 
-  if (connect (sock, (struct sockaddr *) &addr,
+  if (connect (scb->fd, (struct sockaddr *) &addr,
 	       sizeof (struct sockaddr_un)) < 0)
     {
-      close (sock);
-      scb->fd = -1;
-      return -1;
+      int saved = errno;
+      close (scb->fd);
+      perror_with_name (_("could not connect to remote"), saved);
     }
-
-  scb->fd = sock;
-
-  return 0;
 }
 
 static void
diff --git a/gdb/ser-unix.c b/gdb/ser-unix.c
index 5bd1598..020b41e 100644
--- a/gdb/ser-unix.c
+++ b/gdb/ser-unix.c
@@ -50,7 +50,6 @@ show_serial_hwflow (struct ui_file *file, int from_tty,
 }
 #endif
 
-static int hardwire_open (struct serial *scb, const char *name);
 static void hardwire_raw (struct serial *scb);
 static int rate_to_code (int rate);
 static void hardwire_setbaudrate (struct serial *scb, int rate);
@@ -72,14 +71,12 @@ static int hardwire_setstopbits (struct serial *, int);
 
 /* Open up a real live device for serial I/O.  */
 
-static int
+static void
 hardwire_open (struct serial *scb, const char *name)
 {
   scb->fd = gdb_open_cloexec (name, O_RDWR, 0).release ();
   if (scb->fd < 0)
-    return -1;
-
-  return 0;
+    perror_with_name ("could not open device");
 }
 
 static int
diff --git a/gdb/serial.c b/gdb/serial.c
index 122ab0b..720af1a 100644
--- a/gdb/serial.c
+++ b/gdb/serial.c
@@ -173,12 +173,10 @@ serial_for_fd (int fd)
 
 /* Create a new serial for OPS.  */
 
-static struct serial *
+static gdb::unique_xmalloc_ptr<struct serial>
 new_serial (const struct serial_ops *ops)
 {
-  struct serial *scb;
-
-  scb = XCNEW (struct serial);
+  gdb::unique_xmalloc_ptr<struct serial> scb (XCNEW (struct serial));
 
   scb->ops = ops;
 
@@ -221,7 +219,7 @@ serial_open (const char *name)
     }
 
   if (!ops)
-    return NULL;
+    error (_("could not find serial handler for '%s'"), name);
 
   return serial_open_ops_1 (ops, open_name);
 }
@@ -231,20 +229,14 @@ serial_open (const char *name)
 static struct serial *
 serial_open_ops_1 (const struct serial_ops *ops, const char *open_name)
 {
-  struct serial *scb;
-
-  scb = new_serial (ops);
+  gdb::unique_xmalloc_ptr<struct serial> scb = new_serial (ops);
 
   /* `...->open (...)' would get expanded by the open(2) syscall macro.  */
-  if ((*scb->ops->open) (scb, open_name))
-    {
-      xfree (scb);
-      return NULL;
-    }
+  (*scb->ops->open) (scb.get (), open_name);
 
   scb->name = open_name != NULL ? xstrdup (open_name) : NULL;
   scb->next = scb_base;
-  scb_base = scb;
+  scb_base = scb.get ();
 
   if (!serial_logfile.empty ())
     {
@@ -256,7 +248,7 @@ serial_open_ops_1 (const struct serial_ops *ops, const char *open_name)
       serial_logfp = file.release ();
     }
 
-  return scb;
+  return scb.release ();
 }
 
 /* See serial.h.  */
@@ -273,8 +265,6 @@ serial_open_ops (const struct serial_ops *ops)
 static struct serial *
 serial_fdopen_ops (const int fd, const struct serial_ops *ops)
 {
-  struct serial *scb;
-
   if (!ops)
     {
       ops = serial_interface_lookup ("terminal");
@@ -285,18 +275,18 @@ serial_fdopen_ops (const int fd, const struct serial_ops *ops)
   if (!ops)
     return NULL;
 
-  scb = new_serial (ops);
+  gdb::unique_xmalloc_ptr<struct serial> scb = new_serial (ops);
 
   scb->name = NULL;
   scb->next = scb_base;
-  scb_base = scb;
+  scb_base = scb.get ();
 
   if ((ops->fdopen) != NULL)
-    (*ops->fdopen) (scb, fd);
+    (*ops->fdopen) (scb.get (), fd);
   else
     scb->fd = fd;
 
-  return scb;
+  return scb.release ();
 }
 
 struct serial *
diff --git a/gdb/serial.h b/gdb/serial.h
index 9a51fdf..9ae7f4f 100644
--- a/gdb/serial.h
+++ b/gdb/serial.h
@@ -259,7 +259,7 @@ struct serial
 struct serial_ops
   {
     const char *name;
-    int (*open) (struct serial *, const char *name);
+    void (*open) (struct serial *, const char *name);
     void (*close) (struct serial *);
     int (*fdopen) (struct serial *, int fd);
     int (*readchar) (struct serial *, int timeout);