From 21492da89e3f9d387c8992d85df6d9807fe12027 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sat, 18 Apr 2020 13:30:42 +0100 Subject: [PATCH] Improve serial-port setup error messages. Now you can see exactly what pathname the backend tried to open for the serial port, and what error code it got back from the OS when it tried. That should help users distinguish between (for example) a permissions problem and a typo in the filename. --- unix/uxser.c | 5 ++-- windows/winser.c | 68 +++++++++++++++++++++++++----------------------- 2 files changed, 39 insertions(+), 34 deletions(-) diff --git a/unix/uxser.c b/unix/uxser.c index d9d28bce..991d6fd8 100644 --- a/unix/uxser.c +++ b/unix/uxser.c @@ -268,7 +268,7 @@ static char *serial_configure(Serial *serial, Conf *conf) options.c_cc[VTIME] = 0; if (tcsetattr(serial->fd, TCSANOW, &options) < 0) - return dupstr("Unable to configure serial port"); + return dupprintf("Configuring serial port: %s", strerror(errno)); return NULL; } @@ -308,7 +308,8 @@ static char *serial_init(const BackendVtable *vt, Seat *seat, serial->fd = open(line, O_RDWR | O_NOCTTY | O_NDELAY | O_NONBLOCK); if (serial->fd < 0) - return dupstr("Unable to open serial port"); + return dupprintf("Opening serial port '%s': %s", + line, strerror(errno)); cloexec(serial->fd); diff --git a/windows/winser.c b/windows/winser.c index 2a132f83..e387b4b0 100644 --- a/windows/winser.c +++ b/windows/winser.c @@ -170,7 +170,8 @@ static char *serial_configure(Serial *serial, HANDLE serport, Conf *conf) logeventf(serial->logctx, "Configuring %s flow control", str); if (!SetCommState(serport, &dcb)) - return dupstr("Unable to configure serial port"); + return dupprintf("Configuring serial port: %s", + win_strerror(GetLastError())); timeouts.ReadIntervalTimeout = 1; timeouts.ReadTotalTimeoutMultiplier = 0; @@ -178,7 +179,8 @@ static char *serial_configure(Serial *serial, HANDLE serport, Conf *conf) timeouts.WriteTotalTimeoutMultiplier = 0; timeouts.WriteTotalTimeoutConstant = 0; if (!SetCommTimeouts(serport, &timeouts)) - return dupstr("Unable to configure serial timeouts"); + return dupprintf("Configuring serial timeouts: %s", + win_strerror(GetLastError())); } return NULL; @@ -219,39 +221,41 @@ static char *serial_init(const BackendVtable *vt, Seat *seat, serline = conf_get_str(conf, CONF_serline); logeventf(serial->logctx, "Opening serial device %s", serline); - { - /* - * Munge the string supplied by the user into a Windows filename. - * - * Windows supports opening a few "legacy" devices (including - * COM1-9) by specifying their names verbatim as a filename to - * open. (Thus, no files can ever have these names. See - * - * ("Naming a File") for the complete list of reserved names.) - * - * However, this doesn't let you get at devices COM10 and above. - * For that, you need to specify a filename like "\\.\COM10". - * This is also necessary for special serial and serial-like - * devices such as \\.\WCEUSBSH001. It also works for the "legacy" - * names, so you can do \\.\COM1 (verified as far back as Win95). - * See - * (CreateFile() docs). - * - * So, we believe that prepending "\\.\" should always be the - * Right Thing. However, just in case someone finds something to - * talk to that doesn't exist under there, if the serial line - * contains a backslash, we use it verbatim. (This also lets - * existing configurations using \\.\ continue working.) - */ - char *serfilename = - dupprintf("%s%s", strchr(serline, '\\') ? "" : "\\\\.\\", serline); - serport = CreateFile(serfilename, GENERIC_READ | GENERIC_WRITE, 0, NULL, - OPEN_EXISTING, FILE_FLAG_OVERLAPPED, NULL); + /* + * Munge the string supplied by the user into a Windows filename. + * + * Windows supports opening a few "legacy" devices (including + * COM1-9) by specifying their names verbatim as a filename to + * open. (Thus, no files can ever have these names. See + * + * ("Naming a File") for the complete list of reserved names.) + * + * However, this doesn't let you get at devices COM10 and above. + * For that, you need to specify a filename like "\\.\COM10". + * This is also necessary for special serial and serial-like + * devices such as \\.\WCEUSBSH001. It also works for the "legacy" + * names, so you can do \\.\COM1 (verified as far back as Win95). + * See + * (CreateFile() docs). + * + * So, we believe that prepending "\\.\" should always be the + * Right Thing. However, just in case someone finds something to + * talk to that doesn't exist under there, if the serial line + * contains a backslash, we use it verbatim. (This also lets + * existing configurations using \\.\ continue working.) + */ + char *serfilename = + dupprintf("%s%s", strchr(serline, '\\') ? "" : "\\\\.\\", serline); + serport = CreateFile(serfilename, GENERIC_READ | GENERIC_WRITE, 0, NULL, + OPEN_EXISTING, FILE_FLAG_OVERLAPPED, NULL); + if (serport == INVALID_HANDLE_VALUE) { + err = dupprintf("Opening '%s': %s", + serfilename, win_strerror(GetLastError())); sfree(serfilename); + return err; } - if (serport == INVALID_HANDLE_VALUE) - return dupstr("Unable to open serial port"); + sfree(serfilename); err = serial_configure(serial, serport, conf); if (err)