Should use QVector::at() instead of QVector::operator[] when reading a value from a given index position

[samuelfmlourenco]

Currently, QVector::operator[] is being used for accessing a value from a given index position, on several instances, regardless if the value just needs to be read from or written to. However, Qt's QVector class offers an at() function, which is const and doesn't perform a deep copy function, according to the documentation:

"at() can be faster than operator[], because it never causes a deep copy to occur."

Also, it is guaranteed through the class that a value is never read out of bounds, because hidTransfer() will always return a response with a length of 64 bytes, and the functions that use its returned QVector are guaranteed not to read past byte 63.

For example, inside getDescGeneric():

// Private generic function that is used to get any descriptor
QString MCP2210::getDescGeneric(quint8 subcomid, int &errcnt, QString &errstr)
{
    QVector<quint8> command{
        GET_NVRAM_SETTINGS, subcomid  // Header
    };
    QVector<quint8> response = hidTransfer(command, errcnt, errstr);
    size_t maxSize = 2 * DESC_MAXLEN;  // Maximum size of the descriptor in bytes (the zero padding at the end takes two more bytes)
    size_t size = response[4] - 2;  // Descriptor actual size, excluding the padding
    size = size > maxSize ? maxSize : size;  // This also fixes an erroneous result due to a possible unsigned integer rollover (bug fixed in version 1.2.0)
    QString descriptor;
    for (size_t i = 0; i < size; i += 2) {
        descriptor += QChar(response[i + PREAMBLE_SIZE + 3] << 8 | response[i + PREAMBLE_SIZE + 2]);  // UTF-16LE conversion as per the USB 2.0 specification
    }
    return descriptor;
}

Since size can never be greater than 56 (2 * DESC_MAXLEN[28]) and PREAMBLE_SIZE equals 4, the maximum index that is possible read is 61, which is still within boundaries. Therefore QVector::at() can be safely used and will not throw an exception (mind that QVector::operator[] is slower, but safer in the regards of not throwing an exception):

// Private generic function that is used to get any descriptor
QString MCP2210::getDescGeneric(quint8 subcomid, int &errcnt, QString &errstr)
{
    QVector<quint8> command{
        GET_NVRAM_SETTINGS, subcomid  // Header
    };
    QVector<quint8> response = hidTransfer(command, errcnt, errstr);
    size_t maxSize = 2 * DESC_MAXLEN;  // Maximum size of the descriptor in bytes (the zero padding at the end takes two more bytes)
    size_t size = response[4] - 2;  // Descriptor actual size, excluding the padding
    size = size > maxSize ? maxSize : size;  // This also fixes an erroneous result due to a possible unsigned integer rollover (bug fixed in version 1.2.0)
    QString descriptor;
    for (size_t i = 0; i < size; i += 2) {
        descriptor += QChar(response.at(i + PREAMBLE_SIZE + 3) << 8 | response.at(i + PREAMBLE_SIZE + 2));  // UTF-16LE conversion as per the USB 2.0 specification
    }
    return descriptor;
}

Notice that getDescGeneric() is the only function that reads the QVector returned by hidTransfer() using an arbitrary index, thus being the only case that is more "failure prone" (albeit not really, as explained above). The remaining functions use fixed indexes that are always within bounds.

[samuelfmlourenco]

Applied in version 1.2.2.