Fixing gcc491 compilation

[rappoccio]

Fixing signed to unsigned comparison error for gcc 4.9.1.

[cmsbuild]

A new Pull Request was created by @rappoccio for CMSSW_7_3_X.

Fixing gcc491 compilation

It involves the following packages:

CommonTools/RecoAlgos

@cmsbuild, @nclopezo, @StoyanStoynev, @slava77 can you please review it and eventually sign? Thanks.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
@nclopezo, @ktf you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

[StoyanStoynev]

@cmsbuild test

[cmsbuild]

+1
Tested at: 140417a
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-5676/154/summary.html

[cmsbuild]

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

[StoyanStoynev]

@rappoccio I am just wondering why "static_cast(matched_index)" is preferred to "(unsigned int) matched_index" in this case.

[davidlt]

Because it's C++ and _cast functions are well defined comparing to C type casts/conversions. Read: http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Casting Avoid mixing C and C++ too much.

[StoyanStoynev]

+1
For 140417a . Based on jenkins, no effect on RECO.
It was not an objection but for int to unsigned int I would certainly prefer the C-style in this trivial case.

[cmsbuild]

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_3_X IBs unless changes (tests are also fine). @nclopezo, @ktf can you please take care of it?

[davidlt]

It probably should be static_cast<size_t>, I assume that size() returns a type of size_t (e.g. std containers).

[Dr15Jones]

I very strongly agree with David. We should avoid C style casts in CMSSW.

[slava77]

why is this one still open and not merged?

[davidlange6]

did we address the comments of David/Chris?

On Oct 7, 2014, at 11:57 AM, Slava Krutelyov notifications@github.com
wrote:

why is this one still open and not merged?

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

[slava77]

The comment from David/Chris was essentially just confirming that the fix here is done correctly (using c++ cast). So, no action was needed.

[slava77]

... except that size_t was not applied

[davidlt]

@slava77 I would like to get size_t in, because size_t has different sizes on 32-bit and 64-bit systems. Let's play safe.

[slava77]

Sal, could you please make a change (possibly a separate branch)

David,
I guess we have hundreds of places with unsigned int used instead of size_t.
Is there a way to keep track of this with a compiler/analyzer?

[davidlt]

@slava77 UBSan can detect this kind of issues, but only the one already happening. Clang Static Analyzer maybe could do so, also maybe some compiler flags. I don't know concrete answer.

[rappoccio]

OK I just pushed another change to this branch to make it a "size_t", but I don't see it registering because this is closed. I will make another PR.

[rappoccio]

New PR #5728