search

srl-freiburg / pedsim_ros

Pedestrian simulator powered by the social force model
https://github.com/srl-freiburg/pedsim_ros
BSD 2-Clause "Simplified" License
460 stars 170 forks source link

Possible segfault culprit #77

Open ksatyaki opened 2 years ago

ksatyaki commented 2 years ago

https://github.com/srl-freiburg/pedsim_ros/blob/de3772630ae174b278da00bcf26e5cf6c2db5ab2/pedsim_simulator/src/scene.cpp#L260

Perhaps needs a continue after deletion? Because currentGroup is accessed immediately afterwards. It's bound to fail, right? Or am I missing something?

tlind commented 2 years ago

Where do you think it is accessed immediately afterwards? The expression in line 261 and the statement in line 263 are inside an "else" block. Thus they cannot be executed at the same time in which the delete statement (inside the if that belongs to this else) is invoked, as they are in alternative code paths.

tlind commented 2 years ago

Ah, I just realized that the outer for-each loop extends further. In line 267, 270 etc. this might indeed cause trouble, I think you're right...

ksatyaki commented 2 years ago

Also, most foreach can be safely converted to for loops, I think. As far as I am aware, there are only specific cases where the Qt foreach is mandatory. Possibly unrelated, I saw that someone else had ported most of pedsim_ros to ROS2. I am completing just that. :-)