sr-ros-interface icon indicating copy to clipboard operation
sr-ros-interface copied to clipboard

[INFO] printing too many bytes of UBI0 tactile sensor version

Open Rocketmagnet opened this issue 11 years ago • 1 comments

To reproduce: Connect a hand running the 0230 version of the palm firmware (Bielefeld's). Connect a UBI0 tactile sensor to the hand. Run the driver. While the driver is starting, it prints:

[ INFO] [1392114653.808606489]: Tactile version: 30 32 31 36 0A 30 32 31 36 0A 59 73 0A 00 65 00

This is 16 bytes. It should print:

[ INFO] [1392114653.808606489]: Tactile version: 30 32 31 36 0A 30 32 31 36 0A 59 73 0A

This is thirteen bytes.

I believe the problem lies in the function void set_software_version( std::string version ) The code here is also unsafe because it's using [] to access string elements before first checking the length of the string. This is why we should be using static analysis on our code.

Rocketmagnet avatar Feb 11 '14 10:02 Rocketmagnet

The 16 comes from the value TACTILE_DATA_LENGTH_BYTES_v1

https://github.com/shadow-robot/sr-ros-interface-ethercat/blob/R_groovy/sr_external_dependencies/released/external/common/tactile_edc_ethercat_protocol.h

Although this value is 32 in TACTILE_DATA_LENGTH_BYTES_v2 (Bielefeld).

The way we pass the pointer to this function is not the best, really

set_software_version( status_data->tactile[id_sensor].string );

the .string field is a pointer to char, so we are doing a copy of the contents to a string (it considers the array to be null-terminated, so it will copy until it finds a 0.

We should fix it, but given that this is printed only in DEBUG, maybe it's not a priority.

Well, actually if the array contains a 0 before the position 16 it would crash the program. SO maybe it is a priority. :(

toliver avatar Feb 11 '14 12:02 toliver