SICKAG / sick_safetyscanners

ROS driver for SICK safety laser scanners
https://www.sick.com/de/en/opto-electronic-protective-devices/safety-laser-scanners/c/g187225
Apache License 2.0
61 stars 59 forks source link

Device name: std::vector error in Debug mode #49

Closed robertwil closed 4 years ago

robertwil commented 4 years ago

Hi,

trying to debug the project I get an error at

std::string ParseDeviceName::readDeviceName(std::vector<uint8_t>::const_iterator data_ptr) const{ uint16_t string_length = read_write_helper::readUint16LittleEndian(data_ptr + 0) ; std::string name; for (uint16_t i = 0; i < string_length; i++) { name.push_back(read_write_helper::readUint16LittleEndian(data_ptr + 2 + i)); } return name; } in ParseDeviceName.cpp, which causes the program to fail:

can't dereference out of range vector iterator

This only happens in Debug mode as the _STL_VERIFYassertion is shut off otherwise. So the problem was, that the string_length variable was bigger (by 1) than the actual string length of the device name. I "solved" this by substracting 1 from the string_length variable and still got my full device name.

Is this an error in data access ?

Best regards

Robert

puck-fzi commented 4 years ago

This as well is behavior I can not reproduce. The data access should be correct. If i Subtract 1, I am missing the last char of my device name.

Further from the loop it should exactly match the given length of the device name for (uint16_t i = 0; i < string_length; i++) {...}

When i build the driver using the flag -DCMAKE_BUILD_TYPE=Debug if still get not the error you are mentioning. What steps did you take to debug?

robertwil commented 4 years ago

Hi, thanks for the quick response.

I build with set(CMAKE_BUILD_TYPE Debug) im the CMakeList.

So you were right about the correct string length but the problem persists: I have my scanner named "microScan3_1", a length of 12, which is correctly parsed by the driver. So I get the const std::shared_ptr<std::vector<uint8_t> const> vec_ptr = buffer.getBuffer(); of size/capacity 14 with its content: 0x0c '\f' -->size 12;correct 0x00 '\0' --> MSB size 0x6d 'm' --> letter 1 0x69 'i' --> 2 0x63 'c' --> 3 0x72 'r' --> 4 0x6f 'o' -->5 0x53 'S' --> 6 0x63 'c' --> 7 0x61 'a' --> 8 0x6e 'n' -->9 0x33 '3' -->10 0x5f '_' -->11 0x31 '1' -->12

So I'm looping from 0 to (including) 11 calling every iteration name.push_back(read_write_helper::readUint16LittleEndian(data_ptr + 2 + i)); which leads to inline uint16_t readUint16LittleEndian(std::vector<uint8_t>::const_iterator it){ return (*(it + 1) << 8) + *(it + 0); }

So at the last iteration with i being 11 adding + 2 +1 from inside the loop and from readUint16LittleEndian the driver tries to access the vector at position 14, which is out of its range leading to the error above.

It works fine with name.push_back(read_write_helper::readUint**8**LittleEndian(data_ptr + 2 + i)); and as we're trying to get char reading 8Bit should be the right thing (?)

puck-fzi commented 4 years ago

Hi Robert, you are absolutely right. It should be reading of 8bit segments. I fixed this in commit d39aaaebb98b25c9dfbb68b6c79178eac8bc7ff3 . Thanks for spotting this.