ros / ros_comm

ROS communications-related packages, including core client libraries (roscpp, rospy, roslisp) and graph introspection tools (rostopic, rosnode, rosservice, rosparam).
http://wiki.ros.org/ros_comm
752 stars 911 forks source link

Uninitialized member in ros::AdvertiseOptions #2245

Open peci1 opened 2 years ago

peci1 commented 2 years ago

https://github.com/ros/ros_comm/blob/f1156c15a42482d5e762c9e7a2a307681a00adcf/clients/roscpp/include/ros/advertise_options.h#L43-L47

https://github.com/ros/ros_comm/blob/f1156c15a42482d5e762c9e7a2a307681a00adcf/clients/roscpp/include/ros/advertise_options.h#L131

Field has_header is of trivial type and the no-arg constructor does not initialize it. This has hit me with a hard-to-debug issue when creating advertiseoptions myself not using the all-arg constructor. If you create AdvertiseOptions using the no-arg constructor, has_header gets a random value and the code path in Publication tries to increment the seq field even if the message does not have a header.

Would a PR adding the missing initializer be accepted in Melodic and Noetic? I think adding an initializer should not break ABI or API.