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

Static analysis changes for DataFormats/FWLite #28631

Merged
merged 3 commits into from Dec 17, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
28 changes: 16 additions & 12 deletions DataFormats/FWLite/interface/DataGetterHelper.h
Expand Up @@ -24,6 +24,7 @@
#include "DataFormats/FWLite/interface/InternalDataKey.h"
#include "FWCore/FWLite/interface/BranchMapReader.h"
#include "FWCore/Utilities/interface/propagate_const.h"
#include "FWCore/Utilities/interface/thread_safety_macros.h"

#include "Rtypes.h"

Expand Down Expand Up @@ -52,12 +53,13 @@ namespace fwlite {
class DataGetterHelper {
public:
// DataGetterHelper() {};
DataGetterHelper(TTree* tree,
std::shared_ptr<HistoryGetterBase> historyGetter,
std::shared_ptr<BranchMapReader> branchMap = std::shared_ptr<BranchMapReader>(),
std::shared_ptr<edm::EDProductGetter> getter = std::shared_ptr<edm::EDProductGetter>(),
bool useCache = false,
std::function<void(TBranch const&)> baFunc = [](TBranch const&) {});
DataGetterHelper(
TTree* tree,
std::shared_ptr<HistoryGetterBase> historyGetter,
std::shared_ptr<BranchMapReader> branchMap = std::shared_ptr<BranchMapReader>(),
std::shared_ptr<edm::EDProductGetter> getter = std::shared_ptr<edm::EDProductGetter>(),
bool useCache = false,
std::function<void(TBranch const&)> baFunc = [](TBranch const&) {});
virtual ~DataGetterHelper();

// ---------- const member functions ---------------------
Expand Down Expand Up @@ -96,16 +98,18 @@ namespace fwlite {

// ---------- member data --------------------------------
TTree* tree_;
mutable std::shared_ptr<BranchMapReader> branchMap_;
mutable KeyToDataMap data_;
mutable std::vector<char const*> labels_;
//This class is not inteded to be used across different threads
CMS_SA_ALLOW mutable std::shared_ptr<BranchMapReader> branchMap_;
CMS_SA_ALLOW mutable KeyToDataMap data_;
CMS_SA_ALLOW mutable std::vector<char const*> labels_;
const edm::ProcessHistory& history() const;

mutable std::map<std::pair<edm::ProductID, edm::BranchListIndex>, std::shared_ptr<internal::Data>> idToData_;
mutable std::map<edm::BranchID, std::shared_ptr<internal::Data>> bidToData_;
CMS_SA_ALLOW mutable std::map<std::pair<edm::ProductID, edm::BranchListIndex>, std::shared_ptr<internal::Data>>
idToData_;
CMS_SA_ALLOW mutable std::map<edm::BranchID, std::shared_ptr<internal::Data>> bidToData_;
edm::propagate_const<std::shared_ptr<fwlite::HistoryGetterBase>> historyGetter_;
std::shared_ptr<edm::EDProductGetter const> getter_;
mutable bool tcTrained_;
CMS_SA_ALLOW mutable bool tcTrained_;
/// Use internal TTreeCache.
const bool tcUse_;
/// Branch-access-function gets called whenever a branch data is accessed.
Expand Down
30 changes: 17 additions & 13 deletions DataFormats/FWLite/interface/Event.h
Expand Up @@ -36,6 +36,7 @@
}
\endcode

NOTE: This class is not safe to use across threads.
*/
//
// Original Author: Chris Jones
Expand All @@ -62,6 +63,7 @@
#include "DataFormats/Provenance/interface/EventProcessHistoryID.h"
#include "DataFormats/Provenance/interface/EventAuxiliary.h"
#include "DataFormats/Provenance/interface/EventID.h"
#include "FWCore/Utilities/interface/thread_safety_macros.h"

// forward declarations
namespace edm {
Expand Down Expand Up @@ -92,7 +94,8 @@ namespace fwlite {
// DataGetterHelper caching is enabled. When user sets useCache to
// false no cache is created unless user attaches and controls it
// himself.
Event(TFile* iFile, bool useCache = true, std::function<void(TBranch const&)> baFunc = [](TBranch const&) {});
Event(
TFile* iFile, bool useCache = true, std::function<void(TBranch const&)> baFunc = [](TBranch const&) {});
~Event() override;

///Advance to next event in the TFile
Expand Down Expand Up @@ -192,29 +195,30 @@ namespace fwlite {
void setGetter(std::shared_ptr<edm::EDProductGetter const> getter) { return dataHelper_.setGetter(getter); }

// ---------- member data --------------------------------
mutable TFile* file_;
//This class is not inteded to be used across different threads
CMS_SA_ALLOW mutable TFile* file_;
// TTree* eventTree_;
TTree* eventHistoryTree_;
// Long64_t eventIndex_;
mutable std::shared_ptr<fwlite::LuminosityBlock> lumi_;
mutable std::shared_ptr<fwlite::Run> run_;
mutable fwlite::BranchMapReader branchMap_;
CMS_SA_ALLOW mutable std::shared_ptr<fwlite::LuminosityBlock> lumi_;
CMS_SA_ALLOW mutable std::shared_ptr<fwlite::Run> run_;
CMS_SA_ALLOW mutable fwlite::BranchMapReader branchMap_;

//takes ownership of the strings used by the DataKey keys in data_
mutable std::vector<char const*> labels_;
mutable edm::ProcessHistoryMap historyMap_;
mutable std::vector<edm::EventProcessHistoryID> eventProcessHistoryIDs_;
mutable std::vector<std::string> procHistoryNames_;
mutable edm::EventAuxiliary aux_;
mutable EntryFinder entryFinder_;
CMS_SA_ALLOW mutable std::vector<char const*> labels_;
CMS_SA_ALLOW mutable edm::ProcessHistoryMap historyMap_;
CMS_SA_ALLOW mutable std::vector<edm::EventProcessHistoryID> eventProcessHistoryIDs_;
CMS_SA_ALLOW mutable std::vector<std::string> procHistoryNames_;
CMS_SA_ALLOW mutable edm::EventAuxiliary aux_;
CMS_SA_ALLOW mutable EntryFinder entryFinder_;
edm::EventAuxiliary const* pAux_;
edm::EventAux const* pOldAux_;
TBranch* auxBranch_;
int fileVersion_;
mutable bool parameterSetRegistryFilled_;
CMS_SA_ALLOW mutable bool parameterSetRegistryFilled_;

fwlite::DataGetterHelper dataHelper_;
mutable std::shared_ptr<RunFactory> runFactory_;
CMS_SA_ALLOW mutable std::shared_ptr<RunFactory> runFactory_;
};

} // namespace fwlite
Expand Down
7 changes: 5 additions & 2 deletions DataFormats/FWLite/interface/EventSetup.h
Expand Up @@ -37,6 +37,7 @@
std::cout << fooHandle->value()<<std::endl;
}

NOTE: This class is not safe to use across threads
*/
//
// Original Author:
Expand All @@ -51,6 +52,7 @@
#include "DataFormats/Provenance/interface/EventID.h"
#include "DataFormats/Provenance/interface/Timestamp.h"
#include "FWCore/Utilities/interface/propagate_const.h"
#include "FWCore/Utilities/interface/thread_safety_macros.h"

// forward declarations
class TFile;
Expand Down Expand Up @@ -103,9 +105,10 @@ namespace fwlite {
edm::EventID m_syncedEvent;
edm::Timestamp m_syncedTimestamp;

mutable TFile* m_file;
//This class is not inteded to be used across different threads
CMS_SA_ALLOW mutable TFile* m_file;

mutable std::vector<Record*> m_records;
CMS_SA_ALLOW mutable std::vector<Record*> m_records;
};
} // namespace fwlite

Expand Down
20 changes: 11 additions & 9 deletions DataFormats/FWLite/interface/LuminosityBlock.h
Expand Up @@ -10,7 +10,7 @@
Description: <one line class summary>

Usage:
<usage>
This class is not safe to use across threads

*/
//
Expand All @@ -33,6 +33,7 @@
#include "DataFormats/FWLite/interface/EntryFinder.h"
#include "DataFormats/Provenance/interface/ProcessHistoryRegistry.h"
#include "DataFormats/Provenance/interface/LuminosityBlockAuxiliary.h"
#include "FWCore/Utilities/interface/thread_safety_macros.h"

// forward declarations
namespace edm {
Expand Down Expand Up @@ -111,23 +112,24 @@ namespace fwlite {
void updateAux(Long_t lumiIndex) const;

// ---------- member data --------------------------------
mutable std::shared_ptr<BranchMapReader> branchMap_;
//This class is not inteded to be used across different threads
CMS_SA_ALLOW mutable std::shared_ptr<BranchMapReader> branchMap_;

mutable std::shared_ptr<fwlite::Run> run_;
CMS_SA_ALLOW mutable std::shared_ptr<fwlite::Run> run_;

//takes ownership of the strings used by the DataKey keys in data_
mutable std::vector<char const*> labels_;
mutable edm::ProcessHistoryMap historyMap_;
mutable std::vector<std::string> procHistoryNames_;
mutable edm::LuminosityBlockAuxiliary aux_;
mutable EntryFinder entryFinder_;
CMS_SA_ALLOW mutable std::vector<char const*> labels_;
CMS_SA_ALLOW mutable edm::ProcessHistoryMap historyMap_;
CMS_SA_ALLOW mutable std::vector<std::string> procHistoryNames_;
CMS_SA_ALLOW mutable edm::LuminosityBlockAuxiliary aux_;
CMS_SA_ALLOW mutable EntryFinder entryFinder_;
edm::LuminosityBlockAuxiliary const* pAux_;
edm::LuminosityBlockAux const* pOldAux_;
TBranch* auxBranch_;
int fileVersion_;

DataGetterHelper dataHelper_;
mutable std::shared_ptr<RunFactory> runFactory_;
CMS_SA_ALLOW mutable std::shared_ptr<RunFactory> runFactory_;
};

} // namespace fwlite
Expand Down
4 changes: 3 additions & 1 deletion DataFormats/FWLite/interface/Record.h
Expand Up @@ -28,6 +28,7 @@
// user include files
#include "DataFormats/FWLite/interface/IOVSyncValue.h"
#include "FWCore/Utilities/interface/TypeID.h"
#include "FWCore/Utilities/interface/thread_safety_macros.h"

// forward declarations
class TTree;
Expand Down Expand Up @@ -78,7 +79,8 @@ namespace fwlite {
IOVSyncValue m_start;
IOVSyncValue m_end;

mutable std::map<std::pair<edm::TypeID, std::string>, std::pair<TBranch*, void*>> m_branches;
//This class is not inteded to be used across different threads
CMS_SA_ALLOW mutable std::map<std::pair<edm::TypeID, std::string>, std::pair<TBranch*, void*>> m_branches;
};

template <typename HANDLE>
Expand Down
16 changes: 9 additions & 7 deletions DataFormats/FWLite/interface/Run.h
Expand Up @@ -10,7 +10,7 @@
Description: <one line class summary>

Usage:
<usage>
This class is not safe to use across different threads

*/
//
Expand All @@ -35,6 +35,7 @@
#include "DataFormats/Provenance/interface/RunID.h"
#include "FWCore/FWLite/interface/BranchMapReader.h"
#include "DataFormats/FWLite/interface/DataGetterHelper.h"
#include "FWCore/Utilities/interface/thread_safety_macros.h"

// forward declarations
namespace edm {
Expand Down Expand Up @@ -107,14 +108,15 @@ namespace fwlite {
void updateAux(Long_t runIndex) const;

// ---------- member data --------------------------------
mutable std::shared_ptr<BranchMapReader> branchMap_;
//This class is not inteded to be used across different threads
CMS_SA_ALLOW mutable std::shared_ptr<BranchMapReader> branchMap_;

//takes ownership of the strings used by the DataKey keys in data_
mutable std::vector<char const*> labels_;
mutable edm::ProcessHistoryMap historyMap_;
mutable std::vector<std::string> procHistoryNames_;
mutable edm::RunAuxiliary aux_;
mutable EntryFinder entryFinder_;
CMS_SA_ALLOW mutable std::vector<char const*> labels_;
CMS_SA_ALLOW mutable edm::ProcessHistoryMap historyMap_;
CMS_SA_ALLOW mutable std::vector<std::string> procHistoryNames_;
CMS_SA_ALLOW mutable edm::RunAuxiliary aux_;
CMS_SA_ALLOW mutable EntryFinder entryFinder_;
edm::RunAuxiliary const* pAux_;
edm::RunAux const* pOldAux_;
TBranch* auxBranch_;
Expand Down
4 changes: 3 additions & 1 deletion DataFormats/FWLite/interface/RunFactory.h
Expand Up @@ -21,6 +21,7 @@
#include <memory>

#include "DataFormats/FWLite/interface/Run.h"
#include "FWCore/Utilities/interface/thread_safety_macros.h"

namespace fwlite {
class RunFactory {
Expand All @@ -35,7 +36,8 @@ namespace fwlite {
RunFactory(const RunFactory&) = delete; // stop default

const RunFactory& operator=(const RunFactory&) = delete; // stop default
mutable std::shared_ptr<fwlite::Run> run_;
//This class is not inteded to be used across different threads
CMS_SA_ALLOW mutable std::shared_ptr<fwlite::Run> run_;

// ---------- member data --------------------------------
};
Expand Down
23 changes: 13 additions & 10 deletions DataFormats/FWLite/src/DataGetterHelper.cc
Expand Up @@ -43,8 +43,8 @@ namespace fwlite {
// static data member definitions
//
// empty object used to signal that the branch requested was not found
static internal::Data branchNotFound;
static char kEmptyString[1] = {0};
static internal::Data branchNotFound{};
static const char kEmptyString[1] = {0};

//
// constructors and destructor
Expand Down Expand Up @@ -208,18 +208,20 @@ namespace fwlite {
std::strncpy(newModule, iModuleLabel, moduleLabelLen);
labels_.push_back(newModule);

char* newProduct = kEmptyString;
char const* newProduct = kEmptyString;
if (key.product()[0] != 0) {
size_t newProductLen = strlen(key.product()) + 1;
newProduct = new char[newProductLen];
std::strncpy(newProduct, key.product(), newProductLen);
auto newProductTmp = new char[newProductLen];
std::strncpy(newProductTmp, key.product(), newProductLen);
newProduct = newProductTmp;
labels_.push_back(newProduct);
}
char* newProcess = kEmptyString;
char const* newProcess = kEmptyString;
if (key.process()[0] != 0) {
size_t newProcessLen = strlen(key.process()) + 1;
newProcess = new char[newProcessLen];
std::strncpy(newProcess, key.process(), newProcessLen);
auto newProcessTmp = new char[newProcessLen];
std::strncpy(newProcessTmp, key.process(), newProcessLen);
newProcess = newProcessTmp;
labels_.push_back(newProcess);
}
internal::DataKey newKey(edm::TypeID(iInfo), newModule, newProduct, newProcess);
Expand Down Expand Up @@ -251,8 +253,9 @@ namespace fwlite {

if (!foundProcessLabel.empty()) {
//also remember it with the process label
newProcess = new char[foundProcessLabel.size() + 1];
std::strcpy(newProcess, foundProcessLabel.c_str());
auto newProcessTmp = new char[foundProcessLabel.size() + 1];
std::strcpy(newProcessTmp, foundProcessLabel.c_str());
newProcess = newProcessTmp;
labels_.push_back(newProcess);
internal::DataKey newKeyWithProcess(edm::TypeID(iInfo), newModule, newProduct, newProcess);

Expand Down
6 changes: 3 additions & 3 deletions DataFormats/FWLite/src/IOVSyncValue.cc
Expand Up @@ -70,17 +70,17 @@ namespace fwlite {
// static member functions
//
const IOVSyncValue& IOVSyncValue::invalidIOVSyncValue() {
static IOVSyncValue s_invalid;
static const IOVSyncValue s_invalid;
return s_invalid;
}
const IOVSyncValue& IOVSyncValue::endOfTime() {
static IOVSyncValue s_endOfTime(
static const IOVSyncValue s_endOfTime(
edm::EventID(0xFFFFFFFFUL, edm::LuminosityBlockID::maxLuminosityBlockNumber(), edm::EventID::maxEventNumber()),
edm::Timestamp::endOfTime());
return s_endOfTime;
}
const IOVSyncValue& IOVSyncValue::beginOfTime() {
static IOVSyncValue s_beginOfTime(edm::EventID(1, 0, 0), edm::Timestamp::beginOfTime());
static const IOVSyncValue s_beginOfTime(edm::EventID(1, 0, 0), edm::Timestamp::beginOfTime());
return s_beginOfTime;
}
} // namespace fwlite