Does not handle EINTR appropriately in a few places.

[mstewartgallus]

At least in a few places EINTR is not handled appropriately:

• s2n_init in ./utils/s2n_random.c for the open system call
• s2n_stuffer_alloc_ro_from_file in ./stuffer/s2n_stuffer_file.c for the open system call
• ./tls/s2n_recv.c:106: GUARD(usleep(delay % 1000000));
• ./bin/echo.c:124: bytes_read = read(STDIN_FILENO, buffer, bytes_available);
• ./bin/echo.c:91: while (poll(readers, 2, -1) > 0) {

echo is a sample application and not too important right?

Also note that the atomicity of reads and writes is weird. Sometimes, reads and writes CANNOT be interrupted and give only partial results but sometimes they can be interrupted in the middle and return less bytes than expected (but not a value of -1.) But I think that you handle that part well enough in most places.

Also, system calls such as read and write return a value of type ssize_t and not int.

Also, some tests may fail due to this stuff but they're tests so they don't matter too much.

There are a few possibilities to handle this cases.

• Override the system's signal handlers to restart system calls (not recommended)
• Temporally block all system signals so that system calls can not be interrupted
• Make system calls nonblocking so that system calls can not be interrupted
• Simply handle the EINTR error as appropriate (also remember to handle partial reads and writes)

Also note that close is an absolute pain to handle errors for and is completely fucked from a standardization point of view. I use the following hacky workaround for close but you may simply want to ignore errors from it.

my_error fd_close(int fd)
{
    my_error errnum;
    /*
     * The state of a file descriptor after close gives an EINTR
     * error
     * is unspecified by POSIX so this function avoids the problem
     * by
     * simply blocking all signals.
     */

    sigset_t sigset;

    /* First use the signal set for the full set */
    sigfillset(&sigset);

    errnum = pthread_sigmask(SIG_BLOCK, &sigset, &sigset);
    if (errnum != 0)
        return errnum;

    /* Then reuse the signal set for the old set */

    if (-1 == close(fd)) {
        errnum = errno;
        assert(errnum != 0);
    } else {
        errnum = 0;
    }

    my_error mask_errnum =
        pthread_sigmask(SIG_SETMASK, &sigset, 0);
    if (0 == errnum)
        errnum = mask_errnum;

    return errnum;
}

[colmmacc]

Can you take a look at the referenced pull-request? it should fix all but the close() case, which I think it's fair to pass on for now.

[mstewartgallus]

That looks good. Thanks.