[GEOMETRY] Fixes needed for UBSAN #30232
Conversation
|
The code-checks are being triggered in jenkins. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-30232/16114
|
|
A new Pull Request was created by @smuzaffar (Malik Shahzad Muzaffar) for master. It involves the following packages: Geometry/EcalAlgo @civanch, @Dr15Jones, @makortel, @cvuosalo, @ianna, @mdhildreth, @cmsbuild can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
please test |
|
The tests are being triggered in jenkins.
|
|
+1 |
|
Comparison job queued. |
|
Comparison is ready Comparison Summary:
|
| for (int lay = 1; lay <= 2; ++lay) { | ||
| for (short iz = 0; iz < 2; ++iz) { | ||
| short zside = 2 * iz - 1; | ||
| for (short lay = 1; lay <= 2; ++lay) { |
There was a problem hiding this comment.
@smuzaffar - sprintf is not thread safe - it should be replaced with std::ostringstream:
#include <sstream> // for ostringstream
std::ostringstream out;
out << ”hist” << iz << lay;There was a problem hiding this comment.
Or even better to std::format when we switch to C++20:
std::string message = std::format("The answer is {}.", 42);There was a problem hiding this comment.
done , used ostringstream
cmsbuild
added
code-checks-pending
and removed
code-checks-approved
comparison-available
tests-approved
labels
|
The code-checks are being triggered in jenkins. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-30232/16124
|
|
Pull request #30232 was updated. @civanch, @Dr15Jones, @makortel, @cvuosalo, @ianna, @mdhildreth, @cmsbuild can you please check and sign again. |
|
please test |
|
The tests are being triggered in jenkins.
|
|
+1 |
|
Comparison job queued. |
|
Comparison is ready Comparison Summary:
|
|
+1 |
cmsbuild
added
fully-signed
geometry-approved
and removed
geometry-pending
pending-signatures
labels
|
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @silviodonato, @dpiparo (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
PR description:
Use a smaller data type to represent these small numbers. This should fix the UBSAN build errors
PR validation:
Successfully built for UBSAN and normal IBs