Nitrokey / libnitrokey

Communicate with Nitrokey devices in a clean and easy manner
https://nitrokey.com/
GNU Lesser General Public License v3.0
65 stars 34 forks source link

Do not return local variable address for TargetBufferSmallerThanSource message #217

Closed szszszsz closed 2 years ago

szszszsz commented 2 years ago

Use static string object for keeping the c_str message for the caller. Strings collection used as an alternative to memory leaks done via strdup(). To add, TargetBufferSmallerThanSource Exception should never happen in a correctly written library client, as this completely depends on the implementation and not on the communication with the device

Downside: if missed and when occurring too often, the memory taken by the exception messages can take too much memory. Potential improvement: replace std::vector with a kind of a ring buffer, or add simple wrapping logic over it.

Fixes #214

Edit: this should avoid breaking ABI by using static memory. Note: there might be multiple exceptions' strings collections (for each compilation unit including this header). Edit: correct that for libnitrokey 4, by keeping string as a member of the exception.

szszszsz commented 2 years ago

@monwarez Can you take a brief look?

monwarez commented 2 years ago

Seems good to me, since the exception message is a view, a simple fix for the downside would be to simply reset the vector before adding the message.