hannescam / NeoSPI

A simplistic C++ SPI NeoPixel driver that works on any linux device with SPI
GNU General Public License v3.0
3 stars 1 forks source link

Small bug in show() #3

Open jcelerier opened 1 month ago

jcelerier commented 1 month ago

https://github.com/hannescam/NeoSPI/blob/b7ef5877db186c2ddd5c08d6d3ba1897c60fd8f6/src/NeoSPI.cpp#L149

Here this code likely does the right thing but for very wrong reasons :

vector<uint8_t> col(24); // And a new raw SPI data value
int cntTotal = 0; // Counter initializations
int cntCol = 0;
...
while (cntCol < sizeof(col) / sizeof(col.at(0))) { 
  ...
}

this gives:

while (cntCol < (24 / 1)). But the reason sizeof(col) is 24 is not because it was initialized with 24 bytes : most likely you'll be able to confirm that

std::vector<uint8_t> vec_small(10);
std::vector<uint8_t> vec_big(10000);
std::cout << sizeof(vec_small) << " " << sizeof(vec_big) << "\n" ;

also prints 24 24.. that's because sizeof(x) returns the "C" size of the class, not the logical size / number of elements in the container... and it happens that std::vector is implemented with three pointers:

template<typename T>
struct vector { 
    T* begin;
    T* end;
    T* capacity;
};

and on 64 bit systems, sizeof(T) == 8 bytes, thus 8 3 == 24 and sizeof(vector) == 24 for any std::vector instance no matter how many elements there are. except...

thus I'd really recommend to just spell out

while (cntCol < col.size()) { 
  ...
}

or even directly

while (cntCol < 24) { 
  ...
}

as it's never supposed to not be 24 from the code

hannescam commented 1 month ago

Thankyou for pointing this out! I noticed this too a few days ago because I am in the middle of a rewrite for this library but I moved it to GitLab (I forgot to mention this in the README) And I will include this fix in the rewrite