Skip to content

Commit

Permalink
Merge pull request #2284 from wmtan/AddAssertsToMaybeAvoidSegfaults
Browse files Browse the repository at this point in the history
Framework cleanup -- Add asserts to avoid null pointer dereferences
  • Loading branch information
ktf committed Feb 4, 2014
2 parents 88d89de + fb8ee2f commit e858790
Showing 1 changed file with 27 additions and 8 deletions.
35 changes: 27 additions & 8 deletions IOPool/Input/src/RootInputFileSequence.cc
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,7 @@ namespace edm {
usingGoToEvent_,
enablePrefetching_));

assert(rootFile_);
fileIterLastOpened_ = fileIter_;
indexesIntoFiles_[currentIndexIntoFile] = rootFile_->indexIntoFileSharedPtr();
char const* inputType = 0;
Expand Down Expand Up @@ -355,21 +356,25 @@ namespace edm {

boost::shared_ptr<RunAuxiliary>
RootInputFileSequence::readRunAuxiliary_() {
assert(rootFile_);
return rootFile_->readRunAuxiliary_();
}

boost::shared_ptr<LuminosityBlockAuxiliary>
RootInputFileSequence::readLuminosityBlockAuxiliary_() {
assert(rootFile_);
return rootFile_->readLuminosityBlockAuxiliary_();
}

void
RootInputFileSequence::readRun_(RunPrincipal& runPrincipal) {
assert(rootFile_);
rootFile_->readRun_(runPrincipal);
}

void
RootInputFileSequence::readLuminosityBlock_(LuminosityBlockPrincipal& lumiPrincipal) {
assert(rootFile_);
rootFile_->readLuminosityBlock_(lumiPrincipal);
}

Expand All @@ -388,6 +393,7 @@ namespace edm {

void
RootInputFileSequence::readEvent(EventPrincipal& eventPrincipal) {
assert(rootFile_);
rootFile_->readEvent(eventPrincipal);
}

Expand Down Expand Up @@ -418,7 +424,7 @@ namespace edm {

bool
RootInputFileSequence::containedInCurrentFile(RunNumber_t run, LuminosityBlockNumber_t lumi, EventNumber_t event) const {
if(!rootFile_) return false;
if(!rootFile_) return false;
return rootFile_->containsItem(run, lumi, event);
}

Expand Down Expand Up @@ -454,6 +460,7 @@ namespace edm {
// we would have to implement synchronization if a new file is opened.
// To avoid this, just assert.
assert(inputType_ != InputType::SecondaryFile);
assert(rootFile_);
while(offset != 0) {
bool atEnd = rootFile_->skipEvents(offset);
if((offset > 0 || atEnd) && !nextFile()) {
Expand Down Expand Up @@ -492,6 +499,7 @@ namespace edm {
fileIter_ = fileIterBegin_ + (it - indexesIntoFiles_.begin());
initFile(false);
// Now get the item from the correct file.
assert(rootFile_);
bool found = rootFile_->goToEvent(eventID);
assert(found);
return true;
Expand All @@ -504,7 +512,8 @@ namespace edm {
initFile(false);
closedOriginalFile = true;
if((*it)->containsItem(eventID.run(), eventID.luminosityBlock(), eventID.event())) {
if (rootFile_->goToEvent(eventID)) {
assert(rootFile_);
if(rootFile_->goToEvent(eventID)) {
return true;
}
}
Expand All @@ -513,6 +522,7 @@ namespace edm {
if(closedOriginalFile) {
fileIter_ = originalFile;
initFile(false);
assert(rootFile_);
rootFile_->setPosition(originalPosition);
}
}
Expand All @@ -527,6 +537,7 @@ namespace edm {
if(!*it) {
fileIter_ = fileIterBegin_ + (it - indexesIntoFiles_.begin());
initFile(false);
assert(rootFile_);
bool found = rootFile_->setEntryAtItem(run, lumi, event);
if(found) {
return true;
Expand Down Expand Up @@ -557,6 +568,7 @@ namespace edm {
initFile(false);
}
// Now get the item from the correct file.
assert(rootFile_);
found = rootFile_->setEntryAtItem(run, lumi, event);
assert(found);
return true;
Expand Down Expand Up @@ -608,9 +620,8 @@ namespace edm {
std::vector<std::string> rules;
rules.reserve(wantedBranches.size() + 1);
rules.emplace_back("drop *");
for(std::vector<std::string>::const_iterator it = wantedBranches.begin(), itEnd = wantedBranches.end();
it != itEnd; ++it) {
rules.push_back("keep " + *it + "_*");
for(std::string const& branch : wantedBranches) {
rules.push_back("keep " + branch + "_*");
}
ParameterSet pset;
pset.addUntrackedParameter("inputCommands", rules);
Expand All @@ -626,8 +637,10 @@ namespace edm {
}
fileIter_ = fileIterBegin_;
initFile(false);
assert(rootFile_);
rootFile_->setAtEventEntry(IndexIntoFile::invalidEntry);
}
assert(rootFile_);
rootFile_->nextEventEntry();
bool found = rootFile_->readCurrentEvent(cache);
if(!found) {
Expand All @@ -636,6 +649,7 @@ namespace edm {
return false;
}
initFile(false);
assert(rootFile_);
rootFile_->setAtEventEntry(IndexIntoFile::invalidEntry);
return readOneSequential(cache);
}
Expand All @@ -649,21 +663,22 @@ namespace edm {
}
skipBadFiles_ = false;
if(fileIter_ == fileIterEnd_ || !rootFile_ ||
rootFile_->indexIntoFileIter().run() != id.run() ||
rootFile_->indexIntoFileIter().run() != id.run() ||
rootFile_->indexIntoFileIter().lumi() != id.luminosityBlock()) {
bool found = skipToItem(id.run(), id.luminosityBlock(), 0, false);
if(!found) {
return false;
}
}
assert(rootFile_);
bool found = rootFile_->setEntryAtNextEventInLumi(id.run(), id.luminosityBlock());
if(found) {
found = rootFile_->readCurrentEvent(cache);
}
if(!found) {
found = skipToItemInNewFile(id.run(), id.luminosityBlock(), 0);
if(!found) {
return false;
return false;
}
return readOneSequentialWithID(cache, id);
}
Expand All @@ -680,6 +695,7 @@ namespace edm {
fileIter_->fileName() <<
" does not contain specified event:\n" << id << "\n";
}
assert(rootFile_);
found = rootFile_->readCurrentEvent(cache);
assert(found);
}
Expand All @@ -694,6 +710,7 @@ namespace edm {
CLHEP::HepRandomEngine& engine = rng->getEngine();
flatDistribution_.reset(new CLHEP::RandFlat(engine));
}
assert(rootFile_);
skipBadFiles_ = false;
unsigned int currentSeqNumber = fileIter_ - fileIterBegin_;
while(eventsRemainingInFile_ == 0) {
Expand Down Expand Up @@ -735,13 +752,14 @@ namespace edm {
}
skipBadFiles_ = false;
if(fileIter_ == fileIterEnd_ || !rootFile_ ||
rootFile_->indexIntoFileIter().run() != id.run() ||
rootFile_->indexIntoFileIter().run() != id.run() ||
rootFile_->indexIntoFileIter().lumi() != id.luminosityBlock()) {
bool found = skipToItem(id.run(), id.luminosityBlock(), 0);
if(!found) {
return false;
}
int eventsInLumi = 0;
assert(rootFile_);
while(rootFile_->setEntryAtNextEventInLumi(id.run(), id.luminosityBlock())) ++eventsInLumi;
found = skipToItem(id.run(), id.luminosityBlock(), 0);
assert(found);
Expand All @@ -751,6 +769,7 @@ namespace edm {
assert(found);
}
}
assert(rootFile_);
bool found = rootFile_->setEntryAtNextEventInLumi(id.run(), id.luminosityBlock());
if(found) {
found = rootFile_->readCurrentEvent(cache);
Expand Down

0 comments on commit e858790

Please sign in to comment.