search

ros / genmsg

Standalone Python library for generating ROS message and service data structures for various languages
http://wiki.ros.org/genmsg
29 stars 74 forks source link

Recurrent hash collision #66

Closed akru closed 8 years ago

akru commented 8 years ago

The message

std_msgs/Empty data

and

std_msgs/Empty[1000] data

has the same hash.

Because gentools.py code ignore arrays.

In my opinion it's ugly bug.

dirk-thomas commented 8 years ago

Thank you for reporting the issue. I can confirm your result and would also expect a different hash for those two messages. I don't know why the hash algorithm was written the way. Maybe someone else knows @ros/ros_team?

But I fear we can't change the behavior now since that would change the hash for a lot of messages and would make them incompatible with previous releases.

eric-wieser commented 8 years ago

I thought we already decided elsewhere that changing the has was acceptable?

Also, why is having a hash collision here a bug? Hashes are allowed to be the same.

akru commented 8 years ago

I'm not sure that it's a hash collision, seems like an implementation bug:

dirk-thomas commented 8 years ago

@eric-wieser Changing the algorithm to compute the hash needs to happens across all language specific implementations. It also implies that whenever it is being changed that older and newer versions of ROS can't exchange that message type anymore (since they don't agree on the md5).

Two different message definitions having the same md5 means a publisher publishes one type while a subscriber trying to subscribe to another type with the same md5 doesn't prevent a connection between them. But the actual serialized data are not compatible and will very likely result in a problem at runtime.

yguo18 commented 8 years ago

well ! where are the module 'em'? I can't find out

dirk-thomas commented 8 years ago

It's available as a Debian package: http://packages.ubuntu.com/xenial/python-empy

progtologist commented 8 years ago

This bug was first reported in #50 and was marked as wontfix.

dirk-thomas commented 8 years ago

As mentioned above as well as in #50 the changing the current behavior implies changing the hash for many messages. This implies that they couldn't be exchange between newer ROS distros (implementing the new behavior) and older ROS distros (using the old behavior). Therefore I will also close this ticket and mark it as "wontfix".