[6/6] Change serial_readchar to throw

Message ID 20230912-serial-exceptions-v1-6-af5097485390@adacore.com
State New
Headers
Series Fix error messages from serial code |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed

Commit Message

Tom Tromey Sept. 12, 2023, 4:27 p.m. UTC
  This changes serial_readchar to throw an exception rather than trying
to set and preserve errno.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30770
---
 gdb/remote.c    | 56 +++++++++++++++++++++-----------------------------------
 gdb/ser-mingw.c | 12 ++++++++----
 gdb/ser-tcp.c   |  5 ++++-
 gdb/ser-uds.c   |  5 ++++-
 gdb/ser-unix.c  |  5 ++++-
 5 files changed, 41 insertions(+), 42 deletions(-)
  

Patch

diff --git a/gdb/remote.c b/gdb/remote.c
index 23046beeb2e..fe5bd74b9fd 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -9592,22 +9592,6 @@  remote_target::flash_done ()
 /* Stuff for dealing with the packets which are part of this protocol.
    See comment at top of file for details.  */
 
-/* Close/unpush the remote target, and throw a TARGET_CLOSE_ERROR
-   error to higher layers.  Called when a serial error is detected.
-   The exception message is STRING, followed by a colon and a blank,
-   the system error message for errno at function entry and final dot
-   for output compatibility with throw_perror_with_name.  */
-
-static void
-unpush_and_perror (remote_target *target, const char *string)
-{
-  int saved_errno = errno;
-
-  remote_unpush_target (target);
-  throw_error (TARGET_CLOSE_ERROR, "%s: %s.", string,
-	       safe_strerror (saved_errno));
-}
-
 /* Read a single character from the remote end.  The current quit
    handler is overridden to avoid quitting in the middle of packet
    sequence, as that would break communication with the remote server.
@@ -9619,36 +9603,38 @@  remote_target::readchar (int timeout)
   int ch;
   struct remote_state *rs = get_remote_state ();
 
-  {
-    scoped_restore restore_quit_target
-      = make_scoped_restore (&curr_quit_handler_target, this);
-    scoped_restore restore_quit
-      = make_scoped_restore (&quit_handler, ::remote_serial_quit_handler);
+  try
+    {
+      scoped_restore restore_quit_target
+	= make_scoped_restore (&curr_quit_handler_target, this);
+      scoped_restore restore_quit
+	= make_scoped_restore (&quit_handler, ::remote_serial_quit_handler);
 
-    rs->got_ctrlc_during_io = 0;
+      rs->got_ctrlc_during_io = 0;
 
-    ch = serial_readchar (rs->remote_desc, timeout);
+      ch = serial_readchar (rs->remote_desc, timeout);
 
-    if (rs->got_ctrlc_during_io)
-      set_quit_flag ();
-  }
+      if (rs->got_ctrlc_during_io)
+	set_quit_flag ();
+    }
+  catch (const gdb_exception_error &ex)
+    {
+      remote_unpush_target (this);
+      throw_error (TARGET_CLOSE_ERROR,
+		   _("Remote communication error.  "
+		     "Target disconnected: %s"),
+		   ex.what ());
+    }
 
   if (ch >= 0)
     return ch;
 
-  switch ((enum serial_rc) ch)
+  if (ch == SERIAL_EOF)
     {
-    case SERIAL_EOF:
       remote_unpush_target (this);
       throw_error (TARGET_CLOSE_ERROR, _("Remote connection closed"));
-      /* no return */
-    case SERIAL_ERROR:
-      unpush_and_perror (this, _("Remote communication error.  "
-				 "Target disconnected"));
-      /* no return */
-    case SERIAL_TIMEOUT:
-      break;
     }
+
   return ch;
 }
 
diff --git a/gdb/ser-mingw.c b/gdb/ser-mingw.c
index 4607a10fde8..25016aebee5 100644
--- a/gdb/ser-mingw.c
+++ b/gdb/ser-mingw.c
@@ -333,7 +333,11 @@  ser_windows_read_prim (struct serial *scb, size_t count)
     {
       if (GetLastError () != ERROR_IO_PENDING
 	  || !GetOverlappedResult (h, &ov, &bytes_read, TRUE))
-	bytes_read = -1;
+	{
+	  ULONGEST err = GetLastError ();
+	  CloseHandle (ov.hEvent);
+	  throw_winerror_with_name (_("error while reading"), err);
+	}
     }
 
   CloseHandle (ov.hEvent);
@@ -962,16 +966,16 @@  pipe_windows_read (struct serial *scb, size_t count)
   DWORD bytes_read;
 
   if (pipeline_out == INVALID_HANDLE_VALUE)
-    return -1;
+    error (_("could not find file number for pipe"));
 
   if (! PeekNamedPipe (pipeline_out, NULL, 0, NULL, &available, NULL))
-    return -1;
+    throw_winerror_with_name (_("could not peek into pipe"), GetLastError ());
 
   if (count > available)
     count = available;
 
   if (! ReadFile (pipeline_out, scb->buf, count, &bytes_read, NULL))
-    return -1;
+    throw_winerror_with_name (_("could not read from pipe"), GetLastError ());
 
   return bytes_read;
 }
diff --git a/gdb/ser-tcp.c b/gdb/ser-tcp.c
index 62f8a519782..b31b030461b 100644
--- a/gdb/ser-tcp.c
+++ b/gdb/ser-tcp.c
@@ -409,7 +409,10 @@  net_read_prim (struct serial *scb, size_t count)
   /* Need to cast to silence -Wpointer-sign on MinGW, as Winsock's
      'recv' takes 'char *' as second argument, while 'scb->buf' is
      'unsigned char *'.  */
-  return recv (scb->fd, (char *) scb->buf, count, 0);
+  int result = recv (scb->fd, (char *) scb->buf, count, 0);
+  if (result == -1 && errno != EINTR)
+    perror_with_name ("error while reading");
+  return result;
 }
 
 int
diff --git a/gdb/ser-uds.c b/gdb/ser-uds.c
index baa660be21d..ad1f79e0800 100644
--- a/gdb/ser-uds.c
+++ b/gdb/ser-uds.c
@@ -69,7 +69,10 @@  uds_close (struct serial *scb)
 static int
 uds_read_prim (struct serial *scb, size_t count)
 {
-  return recv (scb->fd, scb->buf, count, 0);
+  int result = recv (scb->fd, scb->buf, count, 0);
+  if (result == -1 && errno != EINTR)
+    perror_with_name ("error while reading");
+  return result;
 }
 
 static int
diff --git a/gdb/ser-unix.c b/gdb/ser-unix.c
index a2a897d4442..07cd8b7b5b4 100644
--- a/gdb/ser-unix.c
+++ b/gdb/ser-unix.c
@@ -574,7 +574,10 @@  when debugging using remote targets."),
 int
 ser_unix_read_prim (struct serial *scb, size_t count)
 {
-  return read (scb->fd, scb->buf, count);
+  int result = recv (scb->fd, scb->buf, count, 0);
+  if (result == -1 && errno != EINTR)
+    perror_with_name ("error while reading");
+  return result;
 }
 
 int