Fix out of bounds erase in ConformalTracking#63
Conversation
The intention is to remove a cluster that was just created, but remove will end up calling std::vector::erase for the element that is after the last one. The definition is here: https://github.com/iLCSoft/ConformalTracking/blob/master/include/KDTrack.h#L37 or basically ``` void remove(int clusterN) { m_clusters.erase(m_clusters.begin() + clusterN); } ```
|
Was this uncovered by some static analysis? Or triggered by an actual issue? Also CI workflows seem way outdated here. I won't be able to fix those today (or still this week) most likely. |
|
I suspected there were some cases where the ported and the original version give different results. I ran simulation and reconstruction many times until I found one case and then when I tried to run locally it was crashing when using Gaudi, which is actually the first time it happens. Not sure why since the different results seems to be independent of this. I'll have a look at the CI. |
|
CI is passing. I think it should not change much since in key4hep/k4Reco#31 (fix in Gaudi but not in Marlin) the CI passed which means for 6 events with 10 muons each the tracks were identical. |
The intention is to remove a hit that was just added (see comment in the code a few lines above), but remove will end up calling
std::vector::erasefor the element that is after the last one. The definition is here: https://github.com/iLCSoft/ConformalTracking/blob/master/include/KDTrack.h#L37 or basicallyOther alternatives like
tempTrack.m_clusters.pop_back();are also possible, I guess it's better to use the interface.BEGINRELEASENOTES
ENDRELEASENOTES
Tagging @Zehvogel since there may be some changes in tracking for CLD