Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

new expression being used in src/zephyr/concrete_ble_transmitter.cpp #52

Closed
liammcnair-fdh opened this issue Mar 19, 2021 · 3 comments
Closed
Labels
bug Something isn't working

Comments

@liammcnair-fdh
Copy link

I am seeing the new expression being used in the following line:

src/ble/zephyr/concrete_ble_transmitter.cpp:144:      char* newvalue = new char[payload->size()];

I don't believe zephyr supports using the new operator as per documentation here.

@liammcnair-fdh liammcnair-fdh added the verify New bug report that has not yet been verified label Mar 19, 2021
@adamfowleruk
Copy link
Contributor

It does, but are right in raising it. We're trying to remove all dynamic allocation (which will also mean removing most STL use).

@adamfowleruk adamfowleruk added bug Something isn't working and removed verify New bug report that has not yet been verified labels Mar 19, 2021
@adamfowleruk
Copy link
Contributor

Note: Zephyr docs do not reflect current feature status. See zephyrproject-rtos/zephyr#31281

@adamfowleruk
Copy link
Contributor

This is correct usage according to the samples, but it was not clearing up the newvalue correctly and so leaking memory over time. A fix for this will be committed momentarily. newvalue dynamic allocation is required as the payload changes over time, and the length could change too. Once read into the attribute cache in Zephyr the value can safely be deleted.

adamfowleruk added a commit that referenced this issue Apr 26, 2021
Fixes #52. Clears newly allocated payload temporary copy once used.
Signed-off-by: Adam Fowler <adam@adamfowler.org>
adamfowleruk added a commit that referenced this issue Aug 30, 2021
Zephyr requires a char* allocation to copy to its attribute cache. Usage is correct.
We now also clear up the temporary value. I have confirmed Zephyr doesn't do this itself.
Tested on nRF52832DK.
Signed-off-by: Adam Fowler <adam@adamfowler.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants