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

diff --git a/gdb/remote.c b/gdb/remote.c
index 00c84f167d09f2742ea2e4381b5df1e0a13f8b8d..03ebcc6cdfc40b0fc0ce9bd863aa1b1e811d57d8 100644 (file)
--- 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 5865e847bb5d007c64858c9b7a301951e65da985..fb02327492a4096fed73fdd2a84123433a6662d0 100644 (file)
--- 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 65ee1697331bc8a1f1f448e64e91219ce6c72dd9..30d908576dafd20d40d7866be92bc8d559799231 100644 (file)
--- 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 47ccd33cece3b157e44e9e19ba8a7e9eb822f84c..7bdaaa071dc85542c91c46ded9cc6e05cc6ec574 100644 (file)
--- 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 0367a5ac65b53a9bf905f9915209250c5873f324..5e36840082e100556cacab2e3eb852f992e93247 100644 (file)
--- 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 aeb4de04a3b50f74c9ffedd55618ecd1b1dbd9d6..444d77c0b74244f36900386dd7736afb078e773b 100644 (file)
--- 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 665aab912da442900fbbdc2037c86ee36b9399ba..e51058b534c5a7b49afcd670a4d40d9d23cd1c38 100644 (file)
--- 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 5bd15985d8c056b809c6564fecd565471a8d3bdb..020b41e563db9cabaf7db9fb6a394821ebae34bb 100644 (file)
--- 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 122ab0bd10e6fc8022c10c8999bc91a975c7f39f..720af1a356d4db63848dcb02e21393b21f9f3ad2 100644 (file)
--- 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 9a51fdf78166aa4921680353dc69d5401ffed608..9ae7f4f13f1ceb530b3f0faf009df531f8437a93 100644 (file)
--- 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);