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

technical fix for compiler issue on gcc490. #3308

Merged
merged 4 commits into from Apr 14, 2014

Conversation

apfeiffer1
Copy link
Contributor

+1
... as requested by DavidA. :)

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @apfeiffer1 (Andreas Pfeiffer) for CMSSW_7_1_X.

technical fix for compiler issue on gcc490.

It involves the following packages:

CondCore/CondDB

@ggovi, @cmsbuild, @apfeiffer1, @Degano, @nclopezo can you please review it and eventually sign? Thanks.
@ghellwig this is something you requested to watch as well.
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.

@apfeiffer1
Copy link
Contributor Author

+1

On Fri, Apr 11, 2014 at 8:32 PM, cmsbuild notifications@github.com wrote:

A new Pull Request was created by @apfeiffer1https://github.com/apfeiffer1(Andreas Pfeiffer) for CMSSW_7_1_X.

technical fix for compiler issue on gcc490.

It involves the following packages:

CondCore/CondDB

@ggovi https://github.com/ggovi, @cmsbuild https://github.com/cmsbuild,
@apfeiffer1 https://github.com/apfeiffer1, @deganohttps://github.com/degano,
@nclopezo https://github.com/nclopezo can you please review it and
eventually sign? Thanks.
@ghellwig https://github.com/ghellwig this is something you requested
to watch as well.
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 https://github.com/nclopezo, @ktf https://github.com/ktfyou are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of
your comment.

Reply to this email directly or view it on GitHubhttps://github.com//pull/3308#issuecomment-40238755
.

Thanks,
cheers, andreas

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_1_X IBs unless changes or unless it breaks tests. @nclopezo, @ktf can you please take care of it?

@@ -37,8 +37,8 @@ namespace cond {

template<typename T>
boost::shared_ptr<T> get(size_t n) const {
if( n> (size()-1) ) throwException( "Index outside the bounds of the key array.",
"KeyList::get");
if( n >= size() ) throwException( "Index outside the bounds of the key array.",
Copy link
Contributor

Choose a reason for hiding this comment

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

does not compile
use "int(n)"

@VinInn
Copy link
Contributor

VinInn commented Apr 12, 2014

490 IBs are broken w/o this and the real problem is that size() is signed and n is unsigned...

@apfeiffer1
Copy link
Contributor Author

sorry, did not check - and assumed size() would return size_t as it should
be (and that somehow the subtraction "reduced" the result to signed). I'll
change size() to return size_t as it should be - and then we'll see what
else breaks. OK ?

On Sat, Apr 12, 2014 at 11:42 AM, Vincenzo Innocente <
notifications@github.com> wrote:

490 IBs are broken w/o this and the real problem is that size() is
signed and n is unsigned...

Reply to this email directly or view it on GitHubhttps://github.com//pull/3308#issuecomment-40276311
.

Thanks,
cheers, andreas

@davidlt
Copy link
Contributor

davidlt commented Apr 12, 2014

There are another 3-4 places (unsigned and signed comparison, trivial ones), which needs fixing after you change it to size_t. Do git cms-checkdeps to see them.

@apfeiffer1
Copy link
Contributor Author

ok, I just updated the ones I found after merging the PR into a clean 71x
IB (CMSSW_7_1_X_2014-04-11-1400).
Two more places which needed a "size_t" in a loop. Did not see any further
one, @davidlt where did you see 3-4 ??

On Sat, Apr 12, 2014 at 11:54 AM, davidlt notifications@github.com wrote:

There are another 3-4 places (unsigned and signed comparison, trivial
ones), which needs fixing after you change it to size_t. Do git
cms-checkdeps to see them.

Reply to this email directly or view it on GitHubhttps://github.com//pull/3308#issuecomment-40276476
.

Thanks,
cheers, andreas

@apfeiffer1
Copy link
Contributor Author

+1
just in case it is needed - I'll be offline for a while now ... :)

On Sat, Apr 12, 2014 at 4:21 PM, Andreas Pfeiffer
andreas.pfeiffer@cern.chwrote:

ok, I just updated the ones I found after merging the PR into a clean 71x
IB (CMSSW_7_1_X_2014-04-11-1400).
Two more places which needed a "size_t" in a loop. Did not see any further
one, @davidlt where did you see 3-4 ??

On Sat, Apr 12, 2014 at 11:54 AM, davidlt notifications@github.comwrote:

There are another 3-4 places (unsigned and signed comparison, trivial
ones), which needs fixing after you change it to size_t. Do git
cms-checkdeps to see them.

Reply to this email directly or view it on GitHubhttps://github.com//pull/3308#issuecomment-40276476
.

Thanks,
cheers, andreas

Thanks,
cheers, andreas

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

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

ktf added a commit that referenced this pull request Apr 14, 2014
DB -- Technical fix for compiler issue on gcc490.
@ktf ktf merged commit b2d1fcf into cms-sw:CMSSW_7_1_X Apr 14, 2014
@cmsbuild
Copy link
Contributor

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.

None yet

6 participants