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

I think the use of std::string as a container for data is wrong #48

Closed
Jeroen88 opened this issue May 17, 2020 · 2 comments
Closed

I think the use of std::string as a container for data is wrong #48

Jeroen88 opened this issue May 17, 2020 · 2 comments

Comments

@Jeroen88
Copy link

Everywhere in the library a std::string is used to store data as some kind of buffer, e.g service data, remote characteristic values, manufacturer data, etc. I believe that this data is binary, and so may include bytes of 0x00. But if a std::string is created with a zero somewhere in the middle, it is truncated at this zero as demonstrated by below sketch:

#include <string>

void setup() {
  Serial.begin(115200);
  Serial.println("\n\nStarted");

  std::string demonstrate("\x35\x34\x33\x32\x31\x00\x31\x32\x33\x34\x35");
  Serial.printf("The length of the string is %u, it's size is %u and it's value is '%s'\n", demonstrate.length(), demonstrate.size(), demonstrate.c_str());
}

void loop() {
  // put your main code here, to run repeatedly:

}

The output of this small program is:

Started
The length of the string is 5, it's size is 5 and it's value is '54321'

It is fully logical that length() == 5. If size() would be 11, then the underlying data could be fully retrieved. But since this is also 5, the zero byte and the 5 bytes following the zero will be truncated, if such data was sent by a device.

I think there are two options:

  • Use a uint8_t buffer[] everywhere. Also return a (const) uint8_t * pointer for e.g. the readValue() functions. Pro: easy, con: memory mgt is needed
  • Use a vector<uint8_t> everywhere. Also return a (const) vector<uint8_t> & for e.g. readValue(). Pro: no memory mgt is needed, the getSize() functions can be removed because the vector::size() can be used. Con: a STL container is used instead of a plain array.
    Both options will break @h2zero's strategy to remain compatible with the original BLE library. But if I am right I think this is needed!
@h2zero
Copy link
Owner

h2zero commented May 17, 2020

While std:string isn't ideal for this, it is easy to use. If you wanted to send that data that way you could just specify the length, the only thing that wouldn't work is the c_str(), but if you're working with binary data you probably won't be printing it as a string.

@Jeroen88
Copy link
Author

you could just specify the length

I wasn't aware of that feature of std::string, but I checked it and you are right. Luckily! I'll change PR #44.

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

No branches or pull requests

2 participants