string memory safety #300
Description
note: great library, thanks much for writing/maintaining it
There's a minor string safety gotcha that would be good to fix:
issue:
with these functions:
void BleGamepadConfiguration::setModelNumber(char *value) { _modelNumber = value; }
void BleGamepadConfiguration::setSoftwareRevision(char *value) { _softwareRevision = value; }
void BleGamepadConfiguration::setSerialNumber(char *value) { _serialNumber = value; }
void BleGamepadConfiguration::setFirmwareRevision(char *value) { _firmwareRevision = value; }
void BleGamepadConfiguration::setHardwareRevision(char *value) { _hardwareRevision = value; }
they store the string pointer directly:
char *_modelNumber;
char *_softwareRevision;
char *_serialNumber;
char *_firmwareRevision;
char *_hardwareRevision;
which is a bit unsafe if building dynamic strings if the buffer is free'd adter the setXXX() calls. if that happens, you'll be hanging on to a dangling pointer that points to garbage at best, or crashes at worst.
in your example code you are using static strings, which is technically safe, it just rightfully gives warnings like this:
// if you do this.. warnings will happen.
// behind the scenes, this is safe, since these are pointers to static strings that never go away,
// but...
g_bleGamepadConfig.setModelNumber("1.0");
g_bleGamepadConfig.setSoftwareRevision("Software Rev 1");
g_bleGamepadConfig.setSerialNumber("9876543210");
g_bleGamepadConfig.setFirmwareRevision("2.0");
g_bleGamepadConfig.setHardwareRevision("1.7");// then you get these warnings:
src/main.cpp:138:44: warning: ISO C++ forbids converting a string constant to 'char*' [-Wwrite-strings]
g_bleGamepadConfig.setModelNumber("1.0");
^
src/main.cpp:139:60: warning: ISO C++ forbids converting a string constant to 'char*' [-Wwrite-strings]
g_bleGamepadConfig.setSoftwareRevision("Software Rev 1");
^
src/main.cpp:140:52: warning: ISO C++ forbids converting a string constant to 'char*' [-Wwrite-strings]
g_bleGamepadConfig.setSerialNumber("9876543210");
^
src/main.cpp:141:49: warning: ISO C++ forbids converting a string constant to 'char*' [-Wwrite-strings]
g_bleGamepadConfig.setFirmwareRevision("2.0");
^
src/main.cpp:142:49: warning: ISO C++ forbids converting a string constant to 'char*' [-Wwrite-strings]
g_bleGamepadConfig.setHardwareRevision("1.7");
since you're already using std::string in the lib, maybe do something like just change these members to std::string:
std::string _modelNumber;
std::string _softwareRevision;
std::string _serialNumber;
std::string _firmwareRevision;
std::string _hardwareRevision;
and change these to be const char*
void BleGamepadConfiguration::setModelNumber(const char *value) { _modelNumber = value; }
void BleGamepadConfiguration::setSoftwareRevision(const char *value) { _softwareRevision = value; }
void BleGamepadConfiguration::setSerialNumber(const char *value) { _serialNumber = value; }
void BleGamepadConfiguration::setFirmwareRevision(const char *value) { _firmwareRevision = value; }
void BleGamepadConfiguration::setHardwareRevision(const char *value) { _hardwareRevision = value; }
didn't test the above, but that will make a copy of the string data and solve the issue.
if you agree with this approach, I can test a PR together for it. it would be good to do a quick look for this issue elsewhere in the project too.