From d9d1e679799135adb262d6fcc2bd41303a688bac Mon Sep 17 00:00:00 2001 From: Philippe Canal <pcanal@fnal.gov> Date: Fri, 23 Aug 2013 09:05:14 -0500 Subject: [PATCH] Fix fOffset calculation when encountering cached elements Update the StreamerElement search in TBranchElement::InitializeOffsets to always try to find a StreamerElement that points to one of the member of the actual class. This allows for properly finding the offset of the local object within the outer object. The previous version was somtimes looking at 'cached' StreamerElement; however their local offset are (often) different that the non-cached version and the main offset is very often different. The previous code was then applying this offset to both the actions for the cached StreamerElement and for the regular StreamerElement. For the cached element the two actions compensated each other. For the regular element if the local object had an object within the main object, this was leading to the 'reading' of the local object overwriting content in the main object. Now the offset is calculated for the local object in regard to the outer object using one the StreamerElement that points to one of the member of the actual class *and* AddToOffset is not applied to the cached object (since they are never part of an outer object the offset is always zero). This fixes ROOT-5437 (cherry picked from commit 4fe9faedfc78269b8e835c652ef4cedc74f7a650) --- io/io/src/TStreamerInfoActions.cxx | 9 ++++--- tree/tree/src/TBranchElement.cxx | 43 +++++++++++++++++++++++++++++- 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/io/io/src/TStreamerInfoActions.cxx b/io/io/src/TStreamerInfoActions.cxx index a8a5e9ed2c5..dcd3575d5f0 100644 --- a/io/io/src/TStreamerInfoActions.cxx +++ b/io/io/src/TStreamerInfoActions.cxx @@ -3133,7 +3133,8 @@ void TStreamerInfoActions::TActionSequence::AddToOffset(Int_t delta) iter != end; ++iter) { - iter->fConfiguration->AddToOffset(delta); + if (!iter->fConfiguration->fInfo->GetElements()->At(iter->fConfiguration->fElemId)->TestBit(TStreamerElement::kCache)) + iter->fConfiguration->AddToOffset(delta); } } @@ -3173,7 +3174,8 @@ TStreamerInfoActions::TActionSequence *TStreamerInfoActions::TActionSequence::Cr ++iter) { TConfiguration *conf = iter->fConfiguration->Copy(); - conf->AddToOffset(offset); + if (!iter->fConfiguration->fInfo->GetElements()->At(iter->fConfiguration->fElemId)->TestBit(TStreamerElement::kCache)) + conf->AddToOffset(offset); sequence->AddAction( iter->fAction, conf ); } } else { @@ -3183,7 +3185,8 @@ TStreamerInfoActions::TActionSequence *TStreamerInfoActions::TActionSequence::Cr ++iter) { if ( iter->fConfiguration->fElemId == (UInt_t)element_ids[id] ) { TConfiguration *conf = iter->fConfiguration->Copy(); - conf->AddToOffset(offset); + if (!iter->fConfiguration->fInfo->GetElements()->At(iter->fConfiguration->fElemId)->TestBit(TStreamerElement::kCache)) + conf->AddToOffset(offset); sequence->AddAction( iter->fAction, conf ); } } diff --git a/tree/tree/src/TBranchElement.cxx b/tree/tree/src/TBranchElement.cxx index f3b9a68b56f..fed5503aada 100644 --- a/tree/tree/src/TBranchElement.cxx +++ b/tree/tree/src/TBranchElement.cxx @@ -2671,6 +2671,12 @@ void TBranchElement::InitializeOffsets() Warning("InitializeOffsets", "Cannot get streamer element for branch: %s!", GetName()); fInitOffsets = kTRUE; return; + } else if (branchElem->TestBit(TStreamerElement::kRepeat)) { + // If we have a repeating streamerElement, use the next + // one as it actually hold the 'real' data member('s offset) + if (elems[fID+1]) { + branchElem = (TStreamerElement*) elems[fID+1]; + } } localOffset = branchElem->GetOffset(); branchClass = branchElem->GetClassPointer(); @@ -2704,6 +2710,8 @@ void TBranchElement::InitializeOffsets() // Loop over our sub-branches and compute their offsets. for (Int_t subBranchIdx = 0; subBranchIdx < nbranches; ++subBranchIdx) { + bool alternateElement = false; + fBranchOffset[subBranchIdx] = 0; TBranchElement* subBranch = dynamic_cast<TBranchElement*> (fBranches[subBranchIdx]); if (subBranch == 0) { @@ -2735,6 +2743,31 @@ void TBranchElement::InitializeOffsets() Warning("InitializeOffsets", "No streamer element for branch: %s subbranch: %s", GetName(), subBranch->GetName()); fInitOffsets = kTRUE; return; + } else if (subBranchElement->TestBit(TStreamerElement::kRepeat)) { + // If we have a repeating streamerElement, use the next + // one as it actually hold the 'real' data member('s offset) + if (subBranchElems[subBranch->fID+1]) { + subBranchElement = (TStreamerElement*) subBranchElems[subBranch->fID+1]; + } + } else if (subBranchElement->TestBit(TStreamerElement::kCache)) { + // We have a cached item which is not a repeated but we might still + // have some Actions triggered by a rule that affect real + // data member(s). + if (subBranch->fReadActionSequence && subBranch->fReadActionSequence->fActions.size() > 1) { + typedef TStreamerInfoActions::ActionContainer_t::iterator iterator; + iterator end = subBranch->fReadActionSequence->fActions.end(); + for(iterator iter = subBranch->fReadActionSequence->fActions.begin(); + iter != end; ++iter) { + TStreamerInfoActions::TConfiguration *config = iter->fConfiguration; + UInt_t id = config->fElemId; + TStreamerElement *e = (TStreamerElement*)config->fInfo->GetElements()->At(id); + if (e && !e->TestBit(TStreamerElement::kCache)) { + subBranchElement = e; + alternateElement = true; + break; + } + } + } } localOffset = subBranchElement->GetOffset(); @@ -2969,7 +3002,8 @@ void TBranchElement::InitializeOffsets() // First check whether this sub-branch is part of the 'cache' (because the data member it // represents is no longer in the current class layout. TStreamerInfo *subInfo = subBranch->GetInfoImp(); - if (subInfo && subBranch->TestBit(kCache)) { // subInfo->GetElements()->At(subBranch->GetID())->TestBit(TStreamerElement::kCache)) { + //if (subInfo && subBranch->TestBit(kCache)) { // subInfo->GetElements()->At(subBranch->GetID())->TestBit(TStreamerElement::kCache)) { + if (subBranchElement->TestBit(TStreamerElement::kCache)) { pClass = ((TStreamerElement*)subInfo->GetElements()->At(0))->GetClassPointer(); } // FIXME: Do we need the other base class tests here? @@ -3044,6 +3078,13 @@ void TBranchElement::InitializeOffsets() // Find our offset in our parent class using // a lookup by name in the dictionary meta info // for our parent class. + + if (alternateElement) { + Ssiz_t dotpos = dataName.Last('.'); + Ssiz_t endpos = dataName.Length(); + if (dotpos != kNPOS) ++dotpos; else dotpos = 0; + dataName.Replace(dotpos,endpos-dotpos,subBranchElement->GetFullName()); + } TRealData* rd = pClass->GetRealData(dataName); if (rd && !rd->TestBit(TRealData::kTransient)) { // -- Data member exists in the dictionary meta info, get the offset. -- GitLab