From ad34e9c47ffc249956f2716c403e32a9fa660f9c Mon Sep 17 00:00:00 2001
From: scott snyder <scott.snyder@cern.ch>
Date: Sat, 23 Dec 2017 01:54:21 +0100
Subject: [PATCH] TileRecUtils is abusing a DataVector bug to alter constant
objects in StoreGate.
A long-standing bug in DataVector has meant that it was possible to get
a non-const pointer back from a DataVector::const_iterator.
TileCellBuilder retrieves a TileRawChannelContainer from SG and then
possibly modifies it.
There are also issues in TileRawChannelBuilder, TileRawChannelNoiseFilter,
TileRawChannelOF1Corrector
This of course won't work with AthenaMT.
In preparation for trying to finally remove this bug from DataVector,
this has been adjusted so that the const_cast is explicit.
---
.../TileRecUtils/TileRecUtils/ITileRawChannelTool.h | 2 +-
.../TileRecUtils/TileRawChannelNoiseFilter.h | 2 +-
.../TileRecUtils/TileRawChannelOF1Corrector.h | 2 +-
TileCalorimeter/TileRecUtils/src/TileCellBuilder.cxx | 7 +++++--
.../TileRecUtils/src/TileRawChannelBuilder.cxx | 10 ++++++----
.../TileRecUtils/src/TileRawChannelNoiseFilter.cxx | 6 ++++--
.../TileRecUtils/src/TileRawChannelOF1Corrector.cxx | 5 +++--
7 files changed, 21 insertions(+), 13 deletions(-)
diff --git a/TileCalorimeter/TileRecUtils/TileRecUtils/ITileRawChannelTool.h b/TileCalorimeter/TileRecUtils/TileRecUtils/ITileRawChannelTool.h
index 8389b927aee..27b58b3894e 100644
--- a/TileCalorimeter/TileRecUtils/TileRecUtils/ITileRawChannelTool.h
+++ b/TileCalorimeter/TileRecUtils/TileRecUtils/ITileRawChannelTool.h
@@ -19,7 +19,7 @@ class ITileRawChannelTool: virtual public IAlgTool {
public:
// update TileRawChannelContainer, subtract common mode noise for example
- virtual StatusCode process( const TileRawChannelContainer * rchCnt)=0 ;
+ virtual StatusCode process(TileRawChannelContainer * rchCnt)=0 ;
static const InterfaceID& interfaceID() { return IID_ITileRawChannelTool;}
};
diff --git a/TileCalorimeter/TileRecUtils/TileRecUtils/TileRawChannelNoiseFilter.h b/TileCalorimeter/TileRecUtils/TileRecUtils/TileRawChannelNoiseFilter.h
index 72eac34f9a9..4b77aea3255 100644
--- a/TileCalorimeter/TileRecUtils/TileRecUtils/TileRawChannelNoiseFilter.h
+++ b/TileCalorimeter/TileRecUtils/TileRecUtils/TileRawChannelNoiseFilter.h
@@ -51,7 +51,7 @@ class TileRawChannelNoiseFilter: public AthAlgTool, virtual public ITileRawChann
virtual StatusCode finalize();
/** proceed the coherent noise subtruction algorithm and correct TileRawChannel amplitudes */
- virtual StatusCode process(const TileRawChannelContainer *rchCnt);
+ virtual StatusCode process(TileRawChannelContainer *rchCnt);
private:
diff --git a/TileCalorimeter/TileRecUtils/TileRecUtils/TileRawChannelOF1Corrector.h b/TileCalorimeter/TileRecUtils/TileRecUtils/TileRawChannelOF1Corrector.h
index a1a2bbaaa77..17f8abcfcbe 100644
--- a/TileCalorimeter/TileRecUtils/TileRecUtils/TileRawChannelOF1Corrector.h
+++ b/TileCalorimeter/TileRecUtils/TileRecUtils/TileRawChannelOF1Corrector.h
@@ -49,7 +49,7 @@ class TileRawChannelOF1Corrector: public AthAlgTool, virtual public ITileRawChan
virtual StatusCode finalize() override;
/** Correct TileRawChannel amplitudes if pedestal changed */
- virtual StatusCode process(const TileRawChannelContainer* rawChannelContainer) override;
+ virtual StatusCode process(TileRawChannelContainer* rawChannelContainer) override;
private:
diff --git a/TileCalorimeter/TileRecUtils/src/TileCellBuilder.cxx b/TileCalorimeter/TileRecUtils/src/TileCellBuilder.cxx
index d5d741b513a..b89cdd8b609 100644
--- a/TileCalorimeter/TileRecUtils/src/TileCellBuilder.cxx
+++ b/TileCalorimeter/TileRecUtils/src/TileCellBuilder.cxx
@@ -383,7 +383,8 @@ StatusCode TileCellBuilder::process(CaloCellContainer * theCellContainer) {
ToolHandleArray<ITileRawChannelTool>::iterator endTool=m_noiseFilterTools.end();
for (; itrTool != endTool; ++itrTool) {
- if ((*itrTool)->process(dspChannels).isFailure()) {
+ /// FIXME: const_cast; tools can change the container!
+ if ((*itrTool)->process(const_cast<TileRawChannelContainer*>(dspChannels)).isFailure()) {
ATH_MSG_ERROR( " Error status returned from noise filter " );
} else {
ATH_MSG_DEBUG( "Noise filter applied to the container" );
@@ -495,7 +496,8 @@ StatusCode TileCellBuilder::process(CaloCellContainer * theCellContainer) {
ToolHandleArray<ITileRawChannelTool>::iterator endTool = m_noiseFilterTools.end();
for (; itrTool != endTool; ++itrTool) {
- if ((*itrTool)->process(rawChannels).isFailure()) {
+ /// FIXME: const_cast; tools can change the container!
+ if ((*itrTool)->process(const_cast<TileRawChannelContainer*>(rawChannels)).isFailure()) {
ATH_MSG_ERROR( " Error status returned from noise filter " );
} else {
ATH_MSG_DEBUG( "Noise filter applied to the container" );
@@ -648,6 +650,7 @@ StatusCode TileCellBuilder::process(CaloCellContainer * theCellContainer) {
}
xAOD::EventInfo* eventInfo = 0;
if (eventInfo_c) {
+ /// FIXME: const_cast; changing EventInfo.
eventInfo = const_cast<xAOD::EventInfo*>(eventInfo_c);
if (!eventInfo->getStore()) {
const SG::IAuxStore* store = dynamic_cast<const SG::IAuxStore*> (eventInfo->getConstStore());
diff --git a/TileCalorimeter/TileRecUtils/src/TileRawChannelBuilder.cxx b/TileCalorimeter/TileRecUtils/src/TileRawChannelBuilder.cxx
index c6aec277d3a..12bbe42cce7 100644
--- a/TileCalorimeter/TileRecUtils/src/TileRawChannelBuilder.cxx
+++ b/TileCalorimeter/TileRecUtils/src/TileRawChannelBuilder.cxx
@@ -504,7 +504,8 @@ StatusCode TileRawChannelBuilder::commitContainer() {
ATH_MSG_DEBUG( "Noise filter was already applied to DSP container before, use it as it is" );
} else {
for (;itrTool!=endTool;++itrTool){
- if ((*itrTool)->process(dspCnt).isFailure()) {
+ /// FIXME: const_cast
+ if ((*itrTool)->process(const_cast<TileRawChannelContainer*>(dspCnt)).isFailure()) {
ATH_MSG_ERROR( " Error status returned from noise filter " );
} else {
ATH_MSG_DEBUG( "Noise filter applied to DSP container" );
@@ -540,7 +541,7 @@ StatusCode TileRawChannelBuilder::commitContainer() {
TileRawChannelCollection::const_iterator dspLast=dcoll->end();
for(; rchItr!=lastRch; ++rchItr) {
- TileRawChannel* rch = (*rchItr);
+ const TileRawChannel* rch = (*rchItr);
HWIdentifier adc_id = rch->adc_HWID();
while (dspItr != dspLast && adc_id != (*dspItr)->adc_HWID()) {
++dspItr;
@@ -550,8 +551,9 @@ StatusCode TileRawChannelBuilder::commitContainer() {
ATH_MSG_VERBOSE( "Ch "<<m_tileHWID->to_string(adc_id)
<<" amp " << rch->amplitude() << " ped " << rch->pedestal()
<< " corr " << corr );
- rch->setAmplitude (rch->amplitude() - corr);
- rch->setPedestal (rch->pedestal() + corr);
+ /// FIXME: const_cast
+ const_cast<TileRawChannel*>(rch)->setAmplitude (rch->amplitude() - corr);
+ const_cast<TileRawChannel*>(rch)->setPedestal (rch->pedestal() + corr);
} else {
ATH_MSG_WARNING(" Problem in applying noise corrections "
<< " can not find channel in DSP container with HWID "
diff --git a/TileCalorimeter/TileRecUtils/src/TileRawChannelNoiseFilter.cxx b/TileCalorimeter/TileRecUtils/src/TileRawChannelNoiseFilter.cxx
index 860b7b5b516..bde807b6800 100644
--- a/TileCalorimeter/TileRecUtils/src/TileRawChannelNoiseFilter.cxx
+++ b/TileCalorimeter/TileRecUtils/src/TileRawChannelNoiseFilter.cxx
@@ -88,7 +88,8 @@ StatusCode TileRawChannelNoiseFilter::initialize() {
// ============================================================================
// process container
-StatusCode TileRawChannelNoiseFilter::process(const TileRawChannelContainer *rchCont) {
+StatusCode TileRawChannelNoiseFilter::process(
+ TileRawChannelContainer *rchCont) {
ATH_MSG_DEBUG("in process()");
@@ -281,7 +282,8 @@ StatusCode TileRawChannelNoiseFilter::process(const TileRawChannelContainer *rch
// iterate over all channels in a collection again
for (rchItr = coll->begin(); rchItr != lastRch; ++rchItr) {
- TileRawChannel* rch = (*rchItr);
+ /// FIXME: const_cast
+ TileRawChannel* rch = const_cast<TileRawChannel*>(*rchItr);
int chan = m_tileHWID->channel(rch->adc_HWID());
int gain = m_tileHWID->adc(rch->adc_HWID());
diff --git a/TileCalorimeter/TileRecUtils/src/TileRawChannelOF1Corrector.cxx b/TileCalorimeter/TileRecUtils/src/TileRawChannelOF1Corrector.cxx
index ec3f5b09d9f..d0da887c067 100644
--- a/TileCalorimeter/TileRecUtils/src/TileRawChannelOF1Corrector.cxx
+++ b/TileCalorimeter/TileRecUtils/src/TileRawChannelOF1Corrector.cxx
@@ -85,7 +85,7 @@ StatusCode TileRawChannelOF1Corrector::initialize() {
// ============================================================================
// process container
-StatusCode TileRawChannelOF1Corrector::process(const TileRawChannelContainer* rawChannelContainer) {
+StatusCode TileRawChannelOF1Corrector::process(TileRawChannelContainer* rawChannelContainer) {
ATH_MSG_DEBUG("in process()");
@@ -127,7 +127,8 @@ StatusCode TileRawChannelOF1Corrector::process(const TileRawChannelContainer* ra
}
}
- for (TileRawChannel* rawChannel : *rawChannelCollection) {
+ /// FIXME: const_cast
+ for (TileRawChannel* rawChannel : const_cast<TileRawChannelCollection&>(*rawChannelCollection)) {
HWIdentifier adcId = rawChannel->adc_HWID();
int channel = m_tileHWID->channel(adcId);
int gain = m_tileHWID->adc(adcId);
--
GitLab