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
This commit is contained in:
Tom Tromey 2023-09-01 12:11:37 -06:00
parent ad3cf8c64e
commit a2e0acea42
10 changed files with 90 additions and 123 deletions

View File

@ -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)
{

View File

@ -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

View File

@ -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;
}

View File

@ -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
}

View File

@ -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

View File

@ -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);

View File

@ -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

View File

@ -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

View File

@ -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 *

View File

@ -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);