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

Return a pointer to the Value object #111

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tenderlove
Copy link

Otherwise we get a copy, and the copy seems to clobber the data we want

Before this commit, calling getValue() would return a copy of the underlying NimBLEAttValue object. It seems like the copy doesn't necessarily copy the underlying data stored in the value. In this commit, I changed it to just return a pointer (which fixes my application), but maybe the class should know how to copy its underlying data.

I'm not sure if this is the best fix (I'm not a pro C++ developer), but as I said this fixes my app.

Thanks!

Otherwise we get a copy, and the copy seems to clobber the data we want
@h2zero
Copy link
Owner

h2zero commented May 29, 2023

Providing a pointer to the underlying data structure is dangerous because it could change at any time, thus the reason for the copy. Could you provide an example of the data being "clobbered" to help find the cause?

@tenderlove
Copy link
Author

@h2zero (I need to preface this by saying I'm not a BLE expert, nor C++ expert). My application is trying to read HID data written to USB from my computer. I'm using libusb to send data to the bluetooth device. I think the problem is that since you can send an arbitrary number of bytes from the computer to the bluetooth device, doing the copy doesn't work as it doesn't know how many bytes to copy (since it's not known at compile time).

I'm using the pointer here.

Without this change, I get an object back with not clobbered, but uninitialized memory (sorry I should have been more clear in the original post).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants