adafruit / Adafruit_TLC5947

Adafruit's Arduino driver for the TLC5947
http://www.adafruit.com/products/1429
Other
40 stars 22 forks source link

malloc'ed memory does not get freed #10

Closed positron96 closed 3 years ago

positron96 commented 3 years ago

The constructor mallocs memory for PWM array, but the destructor (there is none) does not free it. If the object is not global, there is a huge (~50 bytes) leak every time it is destroyed.

It would actually be better to not use dynamic RAM at all, e.g. specify output count as a template parameter and bake the array at compile-time.

caternuson commented 3 years ago

What's a use case where the instance goes out of scope and the dtor gets called?

positron96 commented 3 years ago

Any usecase when the author does not want to have this object in global scope). Nowhere in the docs does the library object to this usage. E.g. when the main purpose of the program is not LEDs but something else. In this case LED functionality might be located in a utility function with no global objects. Or if your program has different "screens" of LEDs and you create several objects to store the pixel data.

Anyway, I agree that these usecases might not be widely spread, but having a malloc without a free just shouts for a fix, frequent bug or not. Besides, the basic fix is trivial.

caternuson commented 3 years ago

Anyway, I agree that these usecases might not be widely spread

Yep, it's sort of Arduino specific - most sketches are just "set things up, loop forever".

but having a malloc without a free just shouts for a fix, frequent bug or not. Besides, the basic fix is trivial.

Also agree. Want to make a PR for this?