Repository: valkey-io/valkey
File: src/tls.c (C)

Code before:
    /* Accumulate the amount of bytes of each buffer and check if it exceeds NET_MAX_WRITES_PER_EVENT. */
    size_t iov_bytes_len = 0;
    for (int i = 0; i < iovcnt; i++) {
        iov_bytes_len += iov[i].iov_len;
        if (iov_bytes_len > NET_MAX_WRITES_PER_EVENT) break;
    }

    /* The amount of all buffers is greater than NET_MAX_WRITES_PER_EVENT,
     * which is not worth doing so much memory copying to reduce system calls,
     * therefore, invoke connTLSWrite() multiple times to avoid memory copies. */
    if (iov_bytes_len > NET_MAX_WRITES_PER_EVENT && iovcnt > 0 && iov[0].iov_len >= conn->last_failed_write_data_len) {
        ssize_t tot_sent = 0;
        for (int i = 0; i < iovcnt; i++) {
            ssize_t sent = connTLSWrite(conn_, iov[i].iov_base, iov[i].iov_len);
            if (sent <= 0) return tot_sent > 0 ? tot_sent : sent;
            tot_sent += sent;
            if ((size_t)sent != iov[i].iov_len) break;
        }
        return tot_sent;
    }

    /* The amount of all buffers is less than NET_MAX_WRITES_PER_EVENT,
     * which is worth doing more memory copies in exchange for fewer system calls,
     * so concatenate these scattered buffers into a contiguous piece of memory
     * and send it away by one call to connTLSWrite(). */
    char buf[iov_bytes_len];
    size_t offset = 0;
    for (int i = 0; i < iovcnt; i++) {
        memcpy(buf + offset, iov[i].iov_base, iov[i].iov_len);
        offset += iov[i].iov_len;
    }
    return connTLSWrite(conn_, buf, iov_bytes_len);
}

static int connTLSRead(connection *conn_, void *buf, size_t buf_len) {
    tls_connection *conn = (tls_connection *)conn_;
    int ret;

    if (conn->c.state != CONN_STATE_CONNECTED) return -1;
    ERR_clear_error();
    ret = SSL_read(conn->ssl, buf, buf_len);
    updateSSLPendingFlag(conn);
    return updateStateAfterSSLIO(conn, ret, 1);
}

static const char *connTLSGetLastError(connection *conn_) {
    tls_connection *conn = (tls_connection *)conn_;

    if (conn->ssl_error) return conn->ssl_error;
    return NULL;
}

Review comment (bug):
Please correct me if I am wrong. in case we used the static buffer on the first time, falling back to use it in case the first buffer len is smaller than the conn->last_failed_write_data_len might lead to stack overflow on the following write to the the static buffer. I suggest that we also modify the write to the static buffer like this:

```
/* The amount of all buffers is less than NET_MAX_WRITES_PER_EVENT,
     * which is worth doing more memory copies in exchange for fewer system calls,
     * so concatenate these scattered buffers into a contiguous piece of memory
     * and send it away by one call to connTLSWrite(). */
    char buf[iov_bytes_len];
    size_t offset = 0;
    for (int i = 0; i < iovcnt && offset < iov_bytes_len; i++) {
        memcpy(buf + offset, iov[i].iov_base, iov[i].iov_len);
        offset += iov[i].iov_len;
    }
    return connTLSWrite(conn_, buf, iov_bytes_len);
```


Code after:
    }
    ret = SSL_write(conn->ssl, data, data_len);
    conn->last_failed_write_data_len = ret <= 0 ? data_len : 0;
    return updateStateAfterSSLIO(conn, ret, 1);
}

static int connTLSWritev(connection *conn_, const struct iovec *iov, int iovcnt) {
    tls_connection *conn = (tls_connection *)conn_;
    if (iovcnt == 1) return connTLSWrite(conn_, iov[0].iov_base, iov[0].iov_len);

    /* Accumulate the amount of bytes of each buffer and check if it exceeds NET_MAX_WRITES_PER_EVENT. */
    size_t iov_bytes_len = 0;
    for (int i = 0; i < iovcnt; i++) {
        iov_bytes_len += iov[i].iov_len;
        if (iov_bytes_len > NET_MAX_WRITES_PER_EVENT) break;
    }

    /* In case the amount of all buffers is greater than NET_MAX_WRITES_PER_EVENT,
     * it might not worth doing so much memory copying to reduce system calls,
     * therefore, invoke connTLSWrite() multiple times to avoid memory copies.
     * However, in case when last write failed we still have to repeat sending last_failed_write_data_len
     * bytes. Because of openssl implementation we cannot repeat sending writes with length smaller than
     * the last failed write (https://docs.openssl.org/master/man3/SSL_write) so in case the first io buffer
     * does not provide at least the same amount of bytes as previous failed write, we will have to fallback to
     * memory copy to a static buffer before calling SSL_write. */
    if (iov_bytes_len > NET_MAX_WRITES_PER_EVENT && iovcnt > 0 && iov[0].iov_len >= conn->last_failed_write_data_len) {
        ssize_t tot_sent = 0;
        for (int i = 0; i < iovcnt; i++) {
            ssize_t sent = connTLSWrite(conn_, iov[i].iov_base, iov[i].iov_len);
            if (sent <= 0) return tot_sent > 0 ? tot_sent : sent;
            tot_sent += sent;
            if ((size_t)sent != iov[i].iov_len) break;
        }
        return tot_sent;
    }

    /* The amount of all buffers is less than NET_MAX_WRITES_PER_EVENT,
     * which is worth doing more memory copies in exchange for fewer system calls,
     * so concatenate these scattered buffers into a contiguous piece of memory
     * and send it away by one call to connTLSWrite().
     * However, code can fallback here in case when last write failed and first
     * element of io is buffer not big enough to provide required amount of bytes
     * to retry, so iov_bytes_len may exceed NET_MAX_WRITES_PER_EVENT by the amount
     * of remaining bytes from last taken io. */
    char buf[iov_bytes_len];
    size_t offset = 0;
    for (int i = 0; i < iovcnt && offset < iov_bytes_len; i++) {
        memcpy(buf + offset, iov[i].iov_base, iov[i].iov_len);
        offset += iov[i].iov_len;
    }
    return connTLSWrite(conn_, buf, iov_bytes_len);