Deal with cases in which file is a nullptr in DQMRootSource.cc

[perrotta]

PR description:

As hinted by the static analyzer, see also #39852 (comment), there is a logic error in the code of DQMRootSource.cc (this line) when file = nullptr

If I understand correctly the logic behind, this PR should be the fix for it

I write a PR instead of a github issue because in this case it looks faster to me. But please @Dr15Jones and/or @cms-sw/dqm-l2 comment here if you think that the fix is correct, or if you suggest something else instead

PR validation:

It builds.
If the fix is correct, there should be no differences in outputs

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39895/32812

• This PR adds an extra 16KB to repository

[cmsbuild]

A new Pull Request was created by @perrotta (Andrea Perrotta) for master.

It involves the following packages:

• DQMServices/FwkIO (dqm)

@emanueleusai, @ahmad3213, @cmsbuild, @jfernan2, @syuvivida, @pmandrik, @micsucmed, @rvenditti can you please review it and eventually sign? Thanks.
@barvic this is something you requested to watch as well.
@perrotta, @dpiparo, @rappoccio you are the release manager for this.

cms-bot commands are listed here

[perrotta]

@cmsbuild please test

[cmsbuild]

-1

Failed Tests: RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-50caee/28592/summary.html
COMMIT: e3d7bce
CMSSW: CMSSW_12_6_X_2022-10-27-2300/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/39895/28592/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals-INPUT

The relvals timed out after 4 hours.

Comparison Summary

Summary:

• No significant changes to the logs found
• Reco comparison results: 11 differences found in the comparisons
• DQMHistoTests: Total files compared: 48
• DQMHistoTests: Total histograms compared: 3384029
• DQMHistoTests: Total failures: 9
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 3383998
• DQMHistoTests: Total skipped: 22
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
• Checked 201 log files, 48 edm output root files, 48 DQM output files
• TriggerResults: no differences found

[Dr15Jones]

It seems to me that file ==nullptr and (!isGoodFile && m_skipBadFiles) being false (without already throwing an exception) can only happen if fNames is empty. If that is the case, then one should also change line 488 to
TFile* file = nullptr;

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39895/32832

• This PR adds an extra 16KB to repository

[cmsbuild]

Pull request #39895 was updated. @emanueleusai, @ahmad3213, @cmsbuild, @jfernan2, @syuvivida, @pmandrik, @micsucmed, @rvenditti can you please check and sign again.

[perrotta]

please test

[cmsbuild]

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-50caee/28617/summary.html
COMMIT: ecb7f8c
CMSSW: CMSSW_12_6_X_2022-10-28-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/39895/28617/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

• No significant changes to the logs found
• Reco comparison results: 8 differences found in the comparisons
• DQMHistoTests: Total files compared: 48
• DQMHistoTests: Total histograms compared: 3416356
• DQMHistoTests: Total failures: 6
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 3416328
• DQMHistoTests: Total skipped: 22
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
• Checked 206 log files, 48 edm output root files, 48 DQM output files
• TriggerResults: no differences found

[emanueleusai]

+1

• new logic is correct

[cmsbuild]

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. @perrotta, @dpiparo, @rappoccio (and backports should be raised in the release meeting by the corresponding L2)

[perrotta]

+1