bloguetronica / cp2130

A C++ class that uses libusb to interface with CP2130 devices. It can be used to configure the OTP ROM of such devices, as well as to control them. This class was originally derived from the corresponding class for Qt. Please refer to https://github.com/bloguetronica/cp2130-qt if you wish to use the original class instead.
2 stars 0 forks source link

Unnecessary use of std::vector::push_back() in "cp2130.cpp", line 958 #21

Closed samuelfmlourenco closed 2 years ago

samuelfmlourenco commented 2 years ago

Inside the file "cp2130.cpp", in line 958, there is an instance where std::vector::push_back() is being used in order to add elements to retdata from writeReadInputBuffer. An excerpt of spiWriteRead(), the function which contains said line, follows:

// Writes to the SPI bus while reading back, returning a vector of the same size as the one given
// This is the prefered method of writing and reading, if both endpoint addresses are known
std::vector<uint8_t> CP2130::spiWriteRead(const std::vector<uint8_t> &data, uint8_t endpointInAddr, uint8_t endpointOutAddr, int &errcnt, std::string &errstr)
{
    size_t bytesToWriteRead = data.size();
    size_t bytesLeft = bytesToWriteRead;
    std::vector<uint8_t> retdata;
...
        int bytesRead = 0;  // Important!
        bulkTransfer(endpointInAddr, writeReadInputBuffer, payload, &bytesRead, errcnt, errstr);
        for (int i = 0; i < bytesRead; ++i) {
            retdata.push_back(writeReadInputBuffer[i]);
        }
...
    return retdata;
}

However, std::vector::push_back() is a slower operation than just assigning a value to an element that already exists. Once the number of bytes read is known, retdata could be resized and then the values assigned to it:

// Writes to the SPI bus while reading back, returning a vector of the same size as the one given
// This is the prefered method of writing and reading, if both endpoint addresses are known
std::vector<uint8_t> CP2130::spiWriteRead(const std::vector<uint8_t> &data, uint8_t endpointInAddr, uint8_t endpointOutAddr, int &errcnt, std::string &errstr)
{
    size_t bytesToWriteRead = data.size();
    size_t bytesLeft = bytesToWriteRead;
    std::vector<uint8_t> retdata;
...
        int bytesRead = 0;  // Important!
        bulkTransfer(endpointInAddr, writeReadInputBuffer, payload, &bytesRead, errcnt, errstr);
        retdata.resize(static_cast<size_t>(bytesRead));
        for (int i = 0; i < bytesRead; ++i) {
            retdata[i] = writeReadInputBuffer[i];
        }
...
    return retdata;
}
samuelfmlourenco commented 2 years ago

Implemented in version 1.2.2.