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

fastcat::Manager::GetDeviceStatePointers dynamically allocates memory #30

Open
d-loret opened this issue Apr 27, 2022 · 2 comments
Open
Assignees
Labels
bug Something isn't working

Comments

@d-loret
Copy link
Contributor

d-loret commented Apr 27, 2022

Unlike fastcat::Manager::GetDeviceStates, fastcat::Manager::GetDeviceStatePointers dynamically allocates memory each time it is called. The vector is constructed and resized with each call of the function. This makes the function non real-time safe.

@alex-brinkman alex-brinkman self-assigned this Sep 22, 2022
@alex-brinkman
Copy link
Contributor

The idea behind this function is to get fixed, shared pointers to device state data and that it would be called once by the application. Once the application has this array of pointer, it doesn't need to call this again and can just dereference it as needed to get the current state data.

I agree allocating the vector of array points is dynamically allocating memory which should be corrected. I think the best thing would be to pass a reference to this array like so:

void fastcat::Manager::GetDeviceStatePointers(std::vector<std::shared_ptr<const fastcat::DeviceState>>& device_state_ptrs);

I'd probably overload this function name and asset if an application uses the version that dynamically allocates memory.

Do you agree with this fix?

@alex-brinkman alex-brinkman added the bug Something isn't working label Sep 22, 2022
@d-loret
Copy link
Contributor Author

d-loret commented Apr 6, 2023

pass a reference to this array

Yes, that would be an easy way to fix it. It should probably be coupled with a function that allows the client application know how many states are present in the manager so that it can pre-allocate properly once.

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