Bugfix: in STS unpackers, some temporary storage time variables were too small or signed
Bug 1: Size was too small which led to some digis with TS_MSB above 65555 escaping the duplicates filter.
No effect on the digi themselves (time is computed from the original 64b/32b value).
In the legacy
case we can stay with 32b (instead of the original 64b or the old 16b) as the field in the raw message is 29b, while in the current
version we need 64b to account for potential TS_MSB cycles and large Timeslice start offset.
Bug 2: In both Legacy
and current
unpackers, a signed integer was used at some point to store/return the raw timestamp in clock cycles. While the last bit would probably never be reached in standard usage, it could lead to negative values and crazy results during the conversion through double (full precision time in ns) and back to unsigned integer (ns precision time) for data coming from badly synchronized setups
Bug 3: In the current
unpacker, the subtraction of the Timeslice start time from the TS_MSB value was not protected against the case of badly synchronized data where the TS_MSb does not match the Timeslice index. It now triggers a Fatal
as these data are not recoverable with this unpacker.
Merge request reports
Activity
added BugFix label
added 1 commit
- 4b7fe41e - Fix the size of the TS MSB storage for the dupli filter
Dear @v.friese,
you have been identified as code owner of at least one file which was changed with this merge request.
Please check the changes and approve them or request changes.
added CodeOwners label
- Resolved by Florian Uhlig
what is MSB?
- Resolved by Florian Uhlig
Please wait before merging, we found other occurrence where signed/unsigned flips or range limits may happen. I want to check them to see if more needs to be added to this MR
=> Done with the second commit
Edited by Pierre-Alain Loizeau
added 1 commit
- a26b9476 - In STS unpackers, use only unsigned for time storage and add fatal where neg. could happen
added 1 commit
- 2902f9b6 - In STS unpackers, use only unsigned for time storage and add fatal where neg. could happen
enabled an automatic merge when the pipeline for f036d5fd succeeds