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.

Sign up for GitHub

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

Jump to bottom

Fixed shifting a negative signed value in CondFormats/L1TObjects #25428

Merged
merged 1 commit into from Dec 11, 2018
Merged

Fixed shifting a negative signed value in CondFormats/L1TObjects #25428

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 5 additions & 4 deletions CondFormats/L1TObjects/interface/L1MuPacking.h
View file
Open in desktop
Expand Up @@ -26,6 +26,7 @@
#include "FWCore/MessageLogger/interface/MessageLogger.h"

#include <cstdlib>
#include <limits>

/**
* \class L1MuPacking
Expand Down Expand Up @@ -100,11 +101,11 @@ class L1MuSignedPacking : public L1MuPacking {
int idxFromPacked(unsigned packed) const override { return packed & (1 << (Bits-1)) ? (packed - (1 << Bits) ) : packed;};
/// get the packed notation of a value, check range
unsigned packedFromIdx(int idx) const override {
unsigned maxabs = 1 << (Bits-1) ;
unsigned maxabs = 1U << (Bits-1) ;
if (idx < -(int)maxabs && idx >= (int)maxabs) edm::LogWarning("ScaleRangeViolation")
<< "L1MuSignedPacking::packedFromIdx: warning value " << idx
<< "exceeds " << Bits << "-bit range !!!";
return ~(~0 << Bits) & (idx < 0 ? (1 << Bits) + idx : idx);
return ~(std::numeric_limits<unsigned>::max() << Bits) & (idx < 0 ? (1U << Bits) + idx : idx);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

NOTE: ~0 is the old way of turning on all the bits of the value. The problem could also have been fixed by changing to ~0U to explicitly make the constant an unsigned valued. However, using std::numeric_limits gives the same value but expresses the intent of the code clearly.

Copy link
Contributor

Choose a reason for hiding this comment

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

I suspect ~0U would have worked as well (and preserved the good old C style)

};
};

Expand All @@ -117,11 +118,11 @@ class L1MuSignedPackingGeneric : public L1MuPacking {
static int idxFromPacked(unsigned packed, unsigned int nbits) { return packed & (1 << (nbits-1)) ? (packed - (1 << nbits) ) : packed;};
/// get the packed notation of a value, check range
static unsigned packedFromIdx(int idx, unsigned int nbits) {
unsigned maxabs = 1 << (nbits-1) ;
unsigned maxabs = 1U << (nbits-1) ;
if (idx < -(int)maxabs && idx >= (int)maxabs) edm::LogWarning("ScaleRangeViolation")
<< "L1MuSignedPacking::packedFromIdx: warning value " << idx
<< "exceeds " << nbits << "-bit range !!!";
return ~(~0 << nbits) & (idx < 0 ? (1 << nbits) + idx : idx);
return ~(std::numeric_limits<unsigned>::max() << nbits) & (idx < 0 ? (1U << nbits) + idx : idx);
};
};

Expand Down