Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] Utilities/XrdAdaptor: fix -Wunused-result errors #13045

Merged
merged 1 commit into from
Jan 27, 2016

Conversation

davidlt
Copy link
Contributor

@davidlt davidlt commented Jan 24, 2016

Some xrootd methods are marked with XRD_WARN_UNUSED_RESULT
(__attribute__((warn_unused_result))) and -Wunused-result forces us
to not ignore return values.

src/Utilities/XrdAdaptor/src/XrdSource.cc:327:9: error: ignoring return value of function declared with warn_unused_result attribute [-Werror,-Wunused-result]
src/Utilities/XrdAdaptor/src/XrdSource.cc:338:9: error: ignoring return value of function declared with warn_unused_result attribute [-Werror,-Wunused-result]
src/Utilities/XrdAdaptor/src/XrdRequestManager.cc:102:9: error: ignoring return value of function declared with warn_unused_result attribute [-Werror,-Wunused-result]
src/Utilities/XrdAdaptor/plugins/XrdStorageMaker.cc:86:5: error: ignoring return value of function declared with warn_unused_result attribute [-Werror,-Wunused-result]

Feel free to take it and modify as needed as I might not have picked a correct way of handling issues (warnings/error/exception/cleanups before exception/etc).

Signed-off-by: David Abdurachmanov David.Abdurachmanov@cern.ch

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @davidlt for CMSSW_8_0_X.

It involves the following packages:

Utilities/XrdAdaptor

@cmsbuild, @smuzaffar, @Dr15Jones, @davidlange6 can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @wddgit this is something you requested to watch as well.
@slava77, @Degano, @smuzaffar you are the release manager for this.

cms-bot commands are list here #13028

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@Dr15Jones
Copy link
Contributor

@bbockelm

@@ -83,7 +83,16 @@ class XrdStorageMaker final : public StorageMaker
XrdCl::URL url(fullpath);
XrdCl::FileSystem fs(url);
std::vector<std::string> fileList; fileList.push_back(url.GetPath());
fs.Prepare(fileList, XrdCl::PrepareFlags::Stage, 0, &m_null_handler);
XrdCl::XRootDStatus status;
status = fs.Prepare(fileList, XrdCl::PrepareFlags::Stage, 0, &m_null_handler);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

auto status = ...
would be more concise.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just followed patterns and style in this package :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

auto is better... no time like the current to improve things!

@bbockelm
Copy link
Contributor

Thanks for the patch!

For what it's worth, I asked Xrootd upstream to add __attribute__((warn_unused_result)); IIRC, ignoring failures can lead unexpected to deadlock.

So, this is definitely A Good Thing to do.

Some xrootd methods are marked with `XRD_WARN_UNUSED_RESULT`
(`__attribute__((warn_unused_result))`) and `-Wunused-result` forces us
to not ignore return values.

    src/Utilities/XrdAdaptor/src/XrdSource.cc:327:9: error: ignoring return value of function declared with warn_unused_result attribute [-Werror,-Wunused-result]
    src/Utilities/XrdAdaptor/src/XrdSource.cc:338:9: error: ignoring return value of function declared with warn_unused_result attribute [-Werror,-Wunused-result]
    src/Utilities/XrdAdaptor/src/XrdRequestManager.cc:102:9: error: ignoring return value of function declared with warn_unused_result attribute [-Werror,-Wunused-result]
    src/Utilities/XrdAdaptor/plugins/XrdStorageMaker.cc:86:5: error: ignoring return value of function declared with warn_unused_result attribute [-Werror,-Wunused-result]

Signed-off-by: David Abdurachmanov <David.Abdurachmanov@cern.ch>
@cmsbuild
Copy link
Contributor

Pull request #13045 was updated. @cmsbuild, @smuzaffar, @Dr15Jones, @davidlange6 can you please check and sign again.

@davidlt
Copy link
Contributor Author

davidlt commented Jan 26, 2016

Done, auto + downgrading to edm::LogWarning.

@bbockelm
Copy link
Contributor

Looks good to me.

@Dr15Jones
Copy link
Contributor

please test

@Dr15Jones
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/10750/console

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_8_0_X IBs after it passes the integration tests. This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @Degano, @smuzaffar

@davidlt
Copy link
Contributor Author

davidlt commented Jan 26, 2016

Once this gets in, we can move to Clang 3.7.1 in stable and 3.8.0 RC1 in next branch.

@cmsbuild
Copy link
Contributor

-1

Tested at: 7134e88
I found errors in the following addon tests:

cmsDriver.py RelVal -s L1REPACK:GT2 --data --scenario=HeavyIons -n 10 --conditions auto:run2_hlt_HIon --relval 9000,50 --datatier "RAW" --eventcontent RAW --customise=HLTrigger/Configuration/CustomConfigs.L1T --era Run2_HI --magField 38T_PostLS1 --fileout file:RelVal_Raw_HIon_DATA.root --filein /store/hidata/HIRun2015/HIHardProbes/RAW-RECO/HighPtJet-PromptReco-v1/000/263/689/00000/1802CD9A-DDB8-E511-9CF9-02163E0138CA.root : FAILED - time: date Wed Jan 27 00:25:16 2016-date Wed Jan 27 00:24:33 2016 s - exit: 23552
cmsRun /tmp/cmsbuild/workspace/ib-any-integration/CMSSW_8_0_X_2016-01-25-2300/src/HLTrigger/Configuration/test/OnLine_HLT_HIon.py realData=True globalTag=@ inputFiles=@ : FAILED - time: date Wed Jan 27 00:25:16 2016-date Wed Jan 27 00:24:33 2016 s - exit: 21504
cmsDriver.py RelVal -s HLT:HIon,RAW2DIGI,L1Reco,RECO --data --scenario=HeavyIons -n 10 --conditions auto:run2_data_HIon --relval 9000,50 --datatier "RAW-HLT-RECO" --eventcontent FEVTDEBUGHLT --customise=HLTrigger/Configuration/CustomConfigs.L1THLT --era Run2_HI --magField 38T_PostLS1 --processName=HLTRECO --filein file:RelVal_Raw_HIon_DATA.root --fileout file:RelVal_Raw_HIon_DATA_HLT_RECO.root : FAILED - time: date Wed Jan 27 00:25:16 2016-date Wed Jan 27 00:24:33 2016 s - exit: 21504

you can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-13045/10750/summary.html

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
807e5b9
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-13045/10750/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-13045/10750/git-merge-result

@cmsbuild
Copy link
Contributor

davidlange6 added a commit that referenced this pull request Jan 27, 2016
[RFC] Utilities/XrdAdaptor: fix -Wunused-result errors
@davidlange6 davidlange6 merged commit 1aabae2 into cms-sw:CMSSW_8_0_X Jan 27, 2016
@bbockelm
Copy link
Contributor

IB failures appear to be unrelated ("File not found" at EOS; tried opening from MIT but lacked appropriate credentials).

@davidlt
Copy link
Contributor Author

davidlt commented Jan 27, 2016

Files were deleted from sites, I already made several transfer requests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants