[CondFormats/SiPixelObjects] Solve warning found by gcc 5.3.0

[ghost]

https://cmssdt.cern.ch/SDT/cgi-bin/buildlogs/slc6_amd64_gcc530/CMSSW_8_1_X_2016-03-02-1100/CondFormats/SiPixelObjects
The behavior is unchanged, the parentheses makes clear the order of the logical operation while suppressing the warning.

[ghost]

please test

[cmsbuild]

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

[cmsbuild]

A new Pull Request was created by @Degano (Alessandro Degano) for CMSSW_8_1_X.

It involves the following packages:

CondFormats/SiPixelObjects

@diguida, @cerminar, @cmsbuild, @franzoni, @ggovi, @mmusich, @davidlange6 can you please review it and eventually sign? Thanks.
@apfeiffer1, @dkotlins, @tocheng, @VinInn 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

[mmusich]

please test

[davidlt]

Wrong here. There is not (or I don't see) a point in converting rawId() to 1 or 0 (if operator is not overloaded). What author wanted was most likely != instead of ==. I.e. (! roc->rawId() == detector.rawId) to (roc->rawId() != detector.rawId).

[VinInn]

agree with @davidlt
move ( after the ! and close it after the last )

[dkotlins]

Vincenzo,
If you give me a day I can fix it.
I also do not know what the intention was but I think I can figure it out.
I could have it ready by Monday.
Or shall I wait that this pull request is done?
Danek

On 3 Mar 2016, at 14:31, Vincenzo Innocente notifications@github.com wrote:

In CondFormats/SiPixelObjects/src/SiPixelFrameConverter.cc:

@@ -53,7 +53,7 @@ int SiPixelFrameConverter::toCabling(
for (IT it = path.begin(); it != path.end(); ++it) {
const PixelROC * roc = theMap->findItem(*it);
if (!roc) return 2;

• if (! roc->rawId() == detector.rawId) return 3;
• if ((! roc->rawId()) == detector.rawId) return 3;

agree with @davidlt
move ( after the ! and close it after the last )

�
Reply to this email directly or view it on GitHub.

[cmsbuild]

Pull request #13577 was updated. @diguida, @cerminar, @cmsbuild, @franzoni, @ggovi, @mmusich, @davidlange6 can you please check and sign again.

[ghost]

@VinInn @davidlt This change the current (but wrong?) behavior according to your suggestions.

[VinInn]

then do not modify and leave the warning there

[davidlt]

@Degano correct, but these warnings are introduced in GCC for a reason and hiding them under the carpet is not a right way to deal with them.

[cmsbuild]

+1
Tested at: 1b3b190
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-13577/11691/summary.html

[smuzaffar]

agree that https://github.com/degano/cmssw/commit/a73b83b9eaa0c3f398eace753f73ba6f82728d8f is the correct change (though it changes current behaviour). Lets test it.

[smuzaffar]

please test

[cmsbuild]

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

[cmsbuild]

+1
Tested at: a73b83b
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-13577/11697/summary.html

[cmsbuild]

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-13577/11697/summary.html

[mmusich]

@ggovi ping

[davidlange6]

Adding @bartokm, @dkotlins to possibly help answer the questions in this pull request.

[bartokm]

Unfortunately I was not involved in the development of this code, besides that I'm not sure what is the question...

[dkotlins]

Looks fine to me. Please proceed.
Danek

[mmusich]

+1

[ggovi]

+1

[cmsbuild]

This pull request is fully signed and it will be integrated in one of the next CMSSW_8_1_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @Degano, @smuzaffar