posix/uart: Fixes for failure to terminate (#568)

These changes address various issues found in the POSIX platform UART
driver which lead to the process not terminating properly when the
parent dies or closes `stdin`/`stdout`. Some of the errors could even
lead to unexpected program termination.

Among the various errors corrected are:

*   Using `assert()` to terminate under normal operating conditions.
*   `posixUartProcess()` only checked `s_in_fd` for readability and
    did not check `s_out_fd` for writability. It also caused data
    corruption if `write()` ever returned less than all of the bytes
    sent.
*   `ISIG` was being set on the termios flags. This can cause problems
    when the serial API is used for binary data (Like when used with
    the NCP app). (This requires the CLI be explicitly able to be able
    to handle CTRL-C when built for POSIX)
*   Socket errors weren't being tracked on `select()`.
*   The platform main loop would (possibly) terminate if `select()`
    failed with `EINTR`, which makes it difficult to attach to the
    process with a debugger.

examples/platforms/posix/platform-posix.h

@@ -107,7 +107,7 @@ void posixRandomInit(void);
  * @param[inout]  aMaxFd       A pointer to the max file descriptor.
  *
  */
-void posixUartUpdateFdSet(fd_set *aReadFdSet, fd_set *aWriteFdSet, int *aMaxFd);
+void posixUartUpdateFdSet(fd_set *aReadFdSet, fd_set *aWriteFdSet, fd_set *aErrorFdSet, int *aMaxFd);
 
 /**
  * This function performs radio driver processing.

examples/platforms/posix/platform.c

@@ -54,7 +54,7 @@ void PlatformInit(int argc, char *argv[])
 
     if (argc != 2)
     {
-        exit(1);
+        exit(EXIT_FAILURE);
     }
 
     NODE_ID = (uint32_t)strtol(argv[1], &endptr, 0);
@@ -62,7 +62,7 @@ void PlatformInit(int argc, char *argv[])
     if (*endptr != '\0')
     {
         fprintf(stderr, "Invalid NODE_ID: %s\n", argv[1]);
-        exit(1);
+        exit(EXIT_FAILURE);
     }
 
     posixAlarmInit();
@@ -75,21 +75,28 @@ void PlatformProcessDrivers(void)
 {
     fd_set read_fds;
     fd_set write_fds;
+    fd_set error_fds;
     int max_fd = -1;
     struct timeval timeout;
     int rval;
 
     FD_ZERO(&read_fds);
     FD_ZERO(&write_fds);
+    FD_ZERO(&error_fds);
 
-    posixUartUpdateFdSet(&read_fds, &write_fds, &max_fd);
+    posixUartUpdateFdSet(&read_fds, &write_fds, &error_fds, &max_fd);
     posixRadioUpdateFdSet(&read_fds, &write_fds, &max_fd);
     posixAlarmUpdateTimeout(&timeout);
 
     if (!otAreTaskletsPending())
     {
-        rval = select(max_fd + 1, &read_fds, &write_fds, NULL, &timeout);
-        assert(rval >= 0 && errno != ETIME);
+        rval = select(max_fd + 1, &read_fds, &write_fds, &error_fds, &timeout);
+
+        if ((rval < 0) && (errno != EINTR))
+        {
+            perror("select");
+            exit(EXIT_FAILURE);
+        }
     }
 
     posixUartProcess();

examples/platforms/posix/radio.c

@@ -446,7 +446,12 @@ bool otPlatRadioGetPromiscuous(void)
 void radioReceive(void)
 {
     ssize_t rval = recvfrom(sSockFd, &sReceiveMessage, sizeof(sReceiveMessage), 0, NULL, NULL);
-    assert(rval >= 0);
+
+    if (rval < 0)
+    {
+        perror("recvfrom");
+        exit(EXIT_FAILURE);
+    }
 
     sReceiveFrame.mLength = (uint8_t)(rval - 1);
 
@@ -563,7 +568,12 @@ void radioTransmit(const struct RadioMessage *msg, const struct RadioPacket *pkt
         sockaddr.sin_port = htons(9000 + sPortOffset + i);
         rval = sendto(sSockFd, msg, 1 + pkt->mLength,
                       0, (struct sockaddr *)&sockaddr, sizeof(sockaddr));
-        assert(rval >= 0);
+
+        if (rval < 0)
+        {
+            perror("recvfrom");
+            exit(EXIT_FAILURE);
+        }
     }
 }
 

examples/platforms/posix/uart.c

@@ -33,12 +33,15 @@
 #include <stdio.h>
 #include <termios.h>
 #include <unistd.h>
+#include <errno.h>
+#include <signal.h>
 
 #include <common/code_utils.hpp>
 #include <platform/uart.h>
 #include "platform-posix.h"
 
 #ifdef OPENTHREAD_TARGET_LINUX
+#include <sys/prctl.h>
 int posix_openpt(int oflag);
 int grantpt(int fildes);
 int unlockpt(int fd);
@@ -69,10 +72,20 @@ ThreadError otPlatUartEnable(void)
     ThreadError error = kThreadError_None;
     struct termios termios;
 
+#ifdef OPENTHREAD_TARGET_LINUX
+    // Ensure we terminate this process if the
+    // parent process dies.
+    prctl(PR_SET_PDEATHSIG, SIGHUP);
+#endif
+
     s_in_fd = dup(STDIN_FILENO);
     s_out_fd = dup(STDOUT_FILENO);
     dup2(STDERR_FILENO, STDOUT_FILENO);
 
+    // We need this signal to make sure that this
+    // process terminates properly.
+    signal(SIGPIPE, SIG_DFL);
+
     if (isatty(s_in_fd))
     {
         tcgetattr(s_in_fd, &original_stdin_termios);
@@ -90,20 +103,17 @@ ThreadError otPlatUartEnable(void)
         // get current configuration
         VerifyOrExit(tcgetattr(s_in_fd, &termios) == 0, perror("tcgetattr"); error = kThreadError_Error);
 
-        // turn off input processing
-        termios.c_iflag &= ~(unsigned long)(IGNBRK | BRKINT | ICRNL | INLCR | PARMRK | INPCK | ISTRIP | IXON);
+        // Set up the termios settings for raw mode. This turns
+        // off input/output processing, line processing, and character processing.
+        cfmakeraw(&termios);
 
-        // turn off line processing
-        termios.c_lflag &= ~(unsigned long)(ECHO | ECHONL | ICANON | IEXTEN);
+        // Set up our cflags for local use. Turn on hangup-on-close.
+        termios.c_cflag |= HUPCL | CREAD | CLOCAL;
 
-        // turn off character processing
-        termios.c_cflag &= ~(unsigned long)(CSIZE | PARENB);
-        termios.c_cflag |= CS8 | HUPCL | CREAD | CLOCAL;
-
-        // return 1 byte at a time
+        // "Minimum number of characters for noncanonical read"
         termios.c_cc[VMIN]  = 1;
 
-        // turn off inter-character timer
+        // "Timeout in deciseconds for noncanonical read"
         termios.c_cc[VTIME] = 0;
 
         // configure baud rate
@@ -118,12 +128,15 @@ ThreadError otPlatUartEnable(void)
         // get current configuration
         VerifyOrExit(tcgetattr(s_out_fd, &termios) == 0, perror("tcgetattr"); error = kThreadError_Error);
 
-        // turn off output processing
+        // Set up the termios settings for raw mode. This turns
+        // off input/output processing, line processing, and character processing.
+        cfmakeraw(&termios);
+
+        // Absolutely obliterate all output processing.
         termios.c_oflag = 0;
 
-        // turn off character processing
-        termios.c_cflag &= ~(unsigned long)(CSIZE | PARENB);
-        termios.c_cflag |= CS8 | HUPCL | CREAD | CLOCAL;
+        // Set up our cflags for local use. Turn on hangup-on-close.
+        termios.c_cflag |= HUPCL | CREAD | CLOCAL;
 
         // configure baud rate
         VerifyOrExit(cfsetospeed(&termios, B115200) == 0, perror("cfsetospeed"); error = kThreadError_Error);
@@ -163,22 +176,32 @@ exit:
     return error;
 }
 
-void posixUartUpdateFdSet(fd_set *aReadFdSet, fd_set *aWriteFdSet, int *aMaxFd)
+void posixUartUpdateFdSet(fd_set *aReadFdSet, fd_set *aWriteFdSet, fd_set *aErrorFdSet, int *aMaxFd)
 {
     if (aReadFdSet != NULL)
     {
         FD_SET(s_in_fd, aReadFdSet);
 
+        if (aErrorFdSet != NULL)
+        {
+            FD_SET(s_in_fd, aErrorFdSet);
+        }
+
         if (aMaxFd != NULL && *aMaxFd < s_in_fd)
         {
             *aMaxFd = s_in_fd;
         }
     }
 
-    if (aWriteFdSet != NULL && s_write_length > 0)
+    if ((aWriteFdSet != NULL) && (s_write_length > 0))
     {
         FD_SET(s_out_fd, aWriteFdSet);
 
+        if (aErrorFdSet != NULL)
+        {
+            FD_SET(s_out_fd, aErrorFdSet);
+        }
+
         if (aMaxFd != NULL && *aMaxFd < s_out_fd)
         {
             *aMaxFd = s_out_fd;
@@ -188,22 +211,68 @@ void posixUartUpdateFdSet(fd_set *aReadFdSet, fd_set *aWriteFdSet, int *aMaxFd)
 
 void posixUartProcess(void)
 {
-    const int flags = POLLIN | POLLRDNORM | POLLERR | POLLNVAL | POLLHUP;
-    struct pollfd pollfd = { s_in_fd, flags, 0 };
     ssize_t rval;
-
-    if (poll(&pollfd, 1, 0) > 0 && (pollfd.revents & flags) != 0)
+    const int error_flags = POLLERR | POLLNVAL | POLLHUP;
+    struct pollfd pollfd[] =
     {
-        rval = read(s_in_fd, s_receive_buffer, sizeof(s_receive_buffer));
-        assert(rval >= 0);
-        otPlatUartReceived(s_receive_buffer, (uint16_t)rval);
+        { s_in_fd,  POLLIN  | error_flags, 0 },
+        { s_out_fd, POLLOUT | error_flags, 0 },
+    };
+
+    errno = 0;
+
+    rval = poll(pollfd, sizeof(pollfd) / sizeof(*pollfd), 0);
+
+    if (rval < 0)
+    {
+        perror("poll");
+        exit(EXIT_FAILURE);
     }
 
-    if (s_write_length > 0)
+    if (rval > 0)
     {
-        rval = write(s_out_fd, s_write_buffer, s_write_length);
-        assert(rval >= 0);
-        s_write_length = 0;
-        otPlatUartSendDone();
+        if ((pollfd[0].revents & error_flags) != 0)
+        {
+            perror("s_in_fd");
+            exit(EXIT_FAILURE);
+        }
+
+        if ((pollfd[1].revents & error_flags) != 0)
+        {
+            perror("s_out_fd");
+            exit(EXIT_FAILURE);
+        }
+
+        if (pollfd[0].revents & POLLIN)
+        {
+            rval = read(s_in_fd, s_receive_buffer, sizeof(s_receive_buffer));
+
+            if (rval <= 0)
+            {
+                perror("read");
+                exit(EXIT_FAILURE);
+            }
+
+            otPlatUartReceived(s_receive_buffer, (uint16_t)rval);
+        }
+
+        if ((s_write_length > 0) && (pollfd[1].revents & POLLOUT))
+        {
+            rval = write(s_out_fd, s_write_buffer, s_write_length);
+
+            if (rval <= 0)
+            {
+                perror("write");
+                exit(EXIT_FAILURE);
+            }
+
+            s_write_buffer += (uint16_t)rval;
+            s_write_length -= (uint16_t)rval;
+
+            if (s_write_length == 0)
+            {
+                otPlatUartSendDone();
+            }
+        }
     }
 }

src/cli/cli_uart.cpp

@@ -98,6 +98,13 @@ void Uart::ReceiveTask(const uint8_t *aBuf, uint16_t aBufLength)
 
             break;
 
+#ifdef OPENTHREAD_EXAMPLES_POSIX
+
+        case 0x03: // ASCII for Ctrl-C
+            exit(EXIT_SUCCESS);
+            break;
+#endif
+
         case '\b':
         case 127:
             if (mRxLength > 0)