Remove dead assignments inside CSCRawToDigi #26574
Conversation
|
The code-checks are being triggered in jenkins. |
| bool fTMB_MiniScope = false; | ||
| bool fTMB_RPC = false; | ||
| bool fTMB_BlockedCFEBs = false; | ||
|
|
||
| fTMB_Scope = fTMB_Scope && true; // WARNING in 5_0_X |
There was a problem hiding this comment.
What was the original intention of this line?
| int NRPCtbins = 0; // =VB= number of RPC tbins | ||
|
|
||
| int b0cLine=0;///assumes that buf starts at the tmb data | ||
| ///this is not true if something is wrong in the data | ||
| ///before TMB - then we skip the whole event | ||
|
|
||
| NHeaderFrames++; NHeaderFrames--; |
There was a problem hiding this comment.
What was the original intention of this line?
| @@ -277,7 +271,6 @@ void CSCDDUEventData::unpack_data(const uint16_t *buf, CSCDCCExaminer* examiner) | |||
|
|
|||
| } | |||
| theSizeInWords = dduBufSize+12; | |||
| buf=inputBuf+dduBufSize; | |||
There was a problem hiding this comment.
If the intention was to modify the pointer passed to this function, then it should have been passed by reference.
Otherwise, I agree with the static analyzer that this is a dead assignment as such, and it can be removed
|
please test |
|
Hi,
Victor Barashko is the CSC Unpacker expert. I've already emailed him to
ask his opinion on all these 'cleanups'. I am fairly sure that in the
past he maintained a 'master' version outside CMSSW because it is also
required for online and CSC stand-alone running at Point 5, as well as
various commissioning level tasks. It could indeed be that many of the
features noted here are associated with operation external to CMSSW. So
let's wait for Victor's comments.
Regards, Tim
perrotta wrote on 4/30/19 12:46:
…
PR description:
A few dead assignments are reported by the static analyzer in the
CSCRawToDigi package.
They are removed here.
However, they are sometimes so trivial, that I wonder whether they
were left as such on purpose, for a reason that escapes my
comprehension. That's why I would like to ask for the endorsement of
the CSC reco contact, @ptcox <https://github.com/ptcox> , before
signing this PR for reco.
PR validation:
It builds
------------------------------------------------------------------------
You can view, comment on, or merge this pull request online at:
#26574
Commit Summary
* Remove dead assignments inside CSCRawToDigi
File Changes
* *M* EventFilter/CSCRawToDigi/src/CSCDCCExaminer.cc
<https://github.com/cms-sw/cmssw/pull/26574/files#diff-0> (19)
* *M* EventFilter/CSCRawToDigi/src/CSCDDUEventData.cc
<https://github.com/cms-sw/cmssw/pull/26574/files#diff-1> (8)
* *M* EventFilter/CSCRawToDigi/src/CSCTMBData.cc
<https://github.com/cms-sw/cmssw/pull/26574/files#diff-2> (5)
Patch Links:
* https://github.com/cms-sw/cmssw/pull/26574.patch
* https://github.com/cms-sw/cmssw/pull/26574.diff
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#26574>, or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABGYLHUASCMNZHZGWT5BG23PTAPPPANCNFSM4HJKPDNA>.
|
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-26574/9493
|
|
The tests are being triggered in jenkins. |
|
A new Pull Request was created by @perrotta for master. It involves the following packages: EventFilter/CSCRawToDigi @perrotta, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
Comparison job queued. |
|
@perrotta It looks like most of those old code lines were used as a way to prevent compiler warnings about currently unused variables, instead of commenting it out. Most likely those variables are for not fully implemented features, which were in plans initially, but were later considered as not really needed or important for actual system operation. |
|
Comparison is ready Comparison Summary:
|
Thank you @barvic |
@perotta As I see that comparison tests are passed so far, then it is OK with me. |
|
+1
|
|
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. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
PR description:
A few dead assignments are reported by the static analyzer in the CSCRawToDigi package.
They are removed here.
However, they are sometimes so trivial, that I wonder whether they were left as such on purpose, for a reason that escapes my comprehension. That's why I would like to ask for the endorsement of the CSC reco contact, @ptcox , before signing this PR for reco.
PR validation:
It builds