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

The battle agains coverity #1272

Merged
merged 1 commit into from
May 23, 2024
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion DDCond/src/ConditionsContent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ ConditionsContent::addDependency(DetElement de,
Condition::itemkey_type item,
std::shared_ptr<ConditionUpdateCall> callback)
{
ConditionDependency* dep = new ConditionDependency(de, item, callback);
ConditionDependency* dep = new ConditionDependency(de, item, std::move(callback));
return addDependency(dep);
}

2 changes: 1 addition & 1 deletion DDCond/src/plugins/ConditionsPlugins.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@ static void* create_printer(Detector& description, int argc,char** argv) {
}
PRINTER* p = (flags) ? new PRINTER(slice, prefix, flags) : new PRINTER(slice, prefix);
p->printLevel = print_level;
if ( !name.empty() ) p->name = name;
if ( !name.empty() ) p->name = std::move(name);
return (void*)dynamic_cast<WRAPPER*>(createProcessorWrapper(p));
}

Expand Down
2 changes: 1 addition & 1 deletion DDCond/src/plugins/ConditionsXmlLoader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,6 @@ std::size_t ConditionsXmlLoader::load_range(key_type key,
}
keep.emplace_back(condition);
}
m_buffer = keep;
m_buffer = std::move(keep);
return conditions.size()-len;
}
24 changes: 7 additions & 17 deletions DDCore/include/DD4hep/GeoHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,17 +51,6 @@ namespace dd4hep {
*/
class GeoHandlerTypes {
public:
#if 0
typedef std::set<const TGeoVolume*> ConstVolumeSet;
typedef std::map<SensitiveDetector, ConstVolumeSet> SensitiveVolumes;
typedef std::map<Region, ConstVolumeSet> RegionVolumes;
typedef std::map<LimitSet, ConstVolumeSet> LimitVolumes;
typedef std::map<int, std::set<const TGeoNode*> > Data;
typedef std::set<SensitiveDetector> SensitiveDetectorSet;
typedef std::set<Region> RegionSet;
typedef std::set<LimitSet> LimitSetSet;
typedef std::set<TNamed*> ObjectSet;
#endif
/// Data container to store information obtained during the geometry scan
/**
* \author M.Frank
Expand Down Expand Up @@ -97,13 +86,13 @@ namespace dd4hep {
class GeoHandler: public GeoHandlerTypes {

protected:
bool m_propagateRegions;
std::map<int, std::set<const TGeoNode*> >* m_data;

bool m_propagateRegions { false };
std::map<int, std::set<const TGeoNode*> >* m_data { nullptr };
std::map<const TGeoNode*, std::vector<TGeoNode*> >* m_daughters { nullptr };
/// Internal helper to collect geometry information from traversal
GeoHandler& i_collect(const TGeoNode* parent,
const TGeoNode* node,
int level, Region rg, LimitSet ls);
const TGeoNode* node,
int level, Region rg, LimitSet ls);

private:
/// Never call Copy constructor
Expand All @@ -118,7 +107,8 @@ namespace dd4hep {
/// Default constructor
GeoHandler();
/// Initializing constructor
GeoHandler(std::map<int, std::set<const TGeoNode*> >* ptr);
GeoHandler(std::map<int, std::set<const TGeoNode*> >* ptr,
std::map<const TGeoNode*, std::vector<TGeoNode*> >* daus = nullptr);
/// Default destructor
virtual ~GeoHandler();
/// Propagate regions. Returns the previous value
Expand Down
47 changes: 26 additions & 21 deletions DDCore/src/GeoHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,15 @@ namespace {
}

/// Default constructor
detail::GeoHandler::GeoHandler() : m_propagateRegions(false) {
detail::GeoHandler::GeoHandler() {
m_data = new std::map<int, std::set<const TGeoNode*> >();
}

/// Initializing constructor
detail::GeoHandler::GeoHandler(std::map<int, std::set<const TGeoNode*> >* ptr)
: m_propagateRegions(false), m_data(ptr) {
detail::GeoHandler::GeoHandler(std::map<int, std::set<const TGeoNode*> >* ptr,
std::map<const TGeoNode*, std::vector<TGeoNode*> >* daus)
: m_data(ptr), m_daughters(daus)
{
}

/// Default destructor
Expand Down Expand Up @@ -91,15 +93,15 @@ detail::GeoHandler& detail::GeoHandler::collect(DetElement element, GeometryInfo
TGeoNode* par_node = par.isValid() ? par.placement().ptr() : nullptr;
m_data->clear();
i_collect(par_node, element.placement().ptr(), 0, Region(), LimitSet());
for (auto i = m_data->rbegin(); i != m_data->rend(); ++i) {
for ( auto i = m_data->rbegin(); i != m_data->rend(); ++i ) {
const auto& mapped = (*i).second;
for (const TGeoNode* n : mapped ) {
for ( const TGeoNode* n : mapped ) {
TGeoVolume* v = n->GetVolume();
if (v) {
if ( v ) {
Material mat(v->GetMedium());
Volume vol(v);
// Note : assemblies and the world do not have a real volume nor a material
if (info.volumeSet.find(vol) == info.volumeSet.end()) {
if ( info.volumeSet.find(vol) == info.volumeSet.end() ) {
info.volumeSet.emplace(vol);
info.volumes.emplace_back(vol);
}
Expand Down Expand Up @@ -128,35 +130,38 @@ detail::GeoHandler& detail::GeoHandler::i_collect(const TGeoNode* /* parent */,
const TGeoNode* current,
int level, Region rg, LimitSet ls)
{
TGeoVolume* volume = current->GetVolume();
TObjArray* nodes = volume->GetNodes();
int num_children = nodes ? nodes->GetEntriesFast() : 0;
Volume vol(volume);
Region region = vol.region();
LimitSet limits = vol.limitSet();
TGeoVolume* vol = current->GetVolume();
TObjArray* nodes = vol->GetNodes();
Volume volume = vol;
Region region = volume.region();
LimitSet limits = volume.limitSet();

if ( m_propagateRegions ) {
if ( !region.isValid() && rg.isValid() ) {
region = rg;
vol.setRegion(region);
volume.setRegion(region);
}
if ( !limits.isValid() && ls.isValid() ) {
limits = ls;
vol.setLimitSet(limits);
volume.setLimitSet(limits);
}
}
/// Collect the hierarchy of placements
(*m_data)[level].emplace(current);
if (num_children > 0) {
for (int i = 0; i < num_children; ++i) {
TGeoNode* node = (TGeoNode*) nodes->At(i);
i_collect(current, node, level + 1, region, limits);
}
int num = nodes ? nodes->GetEntriesFast() : 0;
for (int i = 0; i < num; ++i)
i_collect(current, (TGeoNode*)nodes->At(i), level + 1, region, limits);
/// Now collect all the daughters of this volume, so that we can reconnect them in the correct order
if ( m_daughters && m_daughters->find(current) == m_daughters->end() ) {
auto [idau,success] = m_daughters->emplace(current, std::vector<TGeoNode*>());
for (int i = 0; i < num; ++i)
idau->second.push_back((TGeoNode*)nodes->At(i));
}
return *this;
}

/// Initializing constructor
detail::GeoScan::GeoScan(DetElement e) {
detail::GeoScan::GeoScan(DetElement e) {
m_data = GeoHandler().collect(e).release();
}

Expand Down
2 changes: 1 addition & 1 deletion DDCore/src/plugins/CodeGenerator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ namespace {
log << "\t structure[" << pointer(de) << "] = de; " << newline
<< "}" << newline;
}
for(auto d : de.children() ) {
for(const auto& d : de.children() ) {
handleStructure(log, de, d.second);
}
if ( !parent.isValid() ) {
Expand Down
8 changes: 6 additions & 2 deletions DDCore/src/plugins/DetectorChecksum.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -138,13 +138,17 @@ DetectorChecksum::~DetectorChecksum() {
template <typename T> std::string DetectorChecksum::refName(T handle) const {
std::string nam = handle->GetName();
std::size_t idx = nam.find("_0x");
return idx == std::string::npos ? nam : nam.substr(0, idx);
if ( idx == std::string::npos ) return nam;
return nam.substr(0, idx);
}

template <> std::string DetectorChecksum::refName(Segmentation handle) const {
std::string nam = handle->name();
std::size_t idx = nam.find("_0x");
return idx == std::string::npos ? nam : nam.substr(0, idx);
if ( idx == std::string::npos ) return nam;
return nam.substr(0, idx);
}

template <typename T> std::string DetectorChecksum::attr_name(T handle) const {
std::string n = " name=\"" + refName(handle)+"\"";
return n;
Expand Down
3 changes: 1 addition & 2 deletions DDCore/src/plugins/VisProcessor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,7 @@ static void* create_object(Detector& description, int argc, char** argv) {
printout(ERROR,"VisMaterialProcessor","++ Invalid DetElement path: %s",path.c_str());
}
else if ( ::strncmp(argv[i],"-name",4) == 0 ) {
std::string name = argv[++i];
proc->name = name;
proc->name = argv[++i];
continue;
}
else if ( ::strncmp(argv[i],"-show",4) == 0 ) {
Expand Down
43 changes: 22 additions & 21 deletions DDDigi/include/DDDigi/DigiData.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ namespace dd4hep {
mask_type mask;
itemkey_type item;
} values;

public:
/// Default constructor
Key();
Expand Down Expand Up @@ -884,13 +885,13 @@ namespace dd4hep {
public:
template <typename T> using map_type_t = std::map<std::string, std::vector<T> >;
struct header_data {
map_type_t<std::string> stringParams;
map_type_t<float> floatParams;
map_type_t<int> intParams;
std::int32_t run_number { -1 };
std::int32_t event_number { -1 };
std::int64_t time_stamp { 0 };
double event_weight { 0e0 };
map_type_t<std::string> stringParams;
map_type_t<float> floatParams;
map_type_t<int> intParams;
std::int32_t run_number { -1 };
std::int32_t event_number { -1 };
std::int64_t time_stamp { 0 };
double event_weight { 0e0 };
};
std::shared_ptr<header_data> data;
public:
Expand All @@ -912,35 +913,35 @@ namespace dd4hep {
std::size_t size() const;
/// Access the event number
std::int32_t getEventNumber() const {
return data->event_number;
return data->event_number;
}
/// Access the run number
std::int32_t getRunNumber() const {
return data->run_number;
return data->run_number;
}
/// Access the time stamp
std::uint64_t getTimeStamp() const {
return data->time_stamp;
return data->time_stamp;
}
/// Access the event weight
float getWeight() const {
return data->event_weight;
return data->event_weight;
}
/// Set the event number
void setEventNumber(std::int32_t value) {
data->event_number = value;
data->event_number = value;
}
/// Set the run number
void setRunNumber(std::int32_t value) {
data->run_number = value;
data->run_number = value;
}
/// Set the time stamp
void setTimeStamp(std::uint64_t value) {
data->time_stamp = value;
data->time_stamp = value;
}
/// Set the event weight
void setWeight(float value) {
data->event_weight = value;
data->event_weight = value;
}
};

Expand Down Expand Up @@ -1053,24 +1054,24 @@ namespace dd4hep {
template<typename DATA> inline DATA& DataSegment::get(Key key) {
if ( DATA* ptr = std::any_cast<DATA>(this->get_item(key, true)) )
return *ptr;
throw std::runtime_error(this->invalid_cast(key, typeid(DATA)));
throw std::runtime_error(this->invalid_cast(std::move(key), typeid(DATA)));
}
/// Access data as reference by key. If not existing, an exception is thrown
template<typename DATA> inline const DATA& DataSegment::get(Key key) const {
if ( const DATA* ptr = std::any_cast<DATA>(this->get_item(key, true)) )
return *ptr;
throw std::runtime_error(this->invalid_cast(key, typeid(DATA)));
throw std::runtime_error(this->invalid_cast(std::move(key), typeid(DATA)));
}

/// Access data as pointers by key. If not existing, nullptr is returned
template<typename DATA> inline DATA* DataSegment::pointer(Key key) {
if ( DATA* ptr = std::any_cast<DATA>(this->get_item(key, false)) )
if ( DATA* ptr = std::any_cast<DATA>(this->get_item(std::move(key), false)) )
return ptr;
return nullptr;
}
/// Access data as pointers by key. If not existing, nullptr is returned
template<typename DATA> inline const DATA* DataSegment::pointer(Key key) const {
if ( const DATA* ptr = std::any_cast<DATA>(this->get_item(key, false)) )
if ( const DATA* ptr = std::any_cast<DATA>(this->get_item(std::move(key), false)) )
return ptr;
return nullptr;
}
Expand All @@ -1080,14 +1081,14 @@ namespace dd4hep {
key.set_segment(this->id);
value.key.set_segment(this->id);
std::any item = std::make_any<DATA>(std::move(value));
return this->emplace_any(key, std::move(item));
return this->emplace_any(std::move(key), std::move(item));
}

/// Helper to place data to data segment
template <typename KEY, typename DATA>
bool put_data(DataSegment& segment, KEY key, DATA&& value) {
std::any item = std::make_any<DATA>(std::move(value));
return segment.emplace_any(key, std::move(item));
return segment.emplace_any(std::move(key), std::move(item));
}

/// User event data for DDDigi
Expand Down
6 changes: 2 additions & 4 deletions DDDigi/io/DigiIO.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -498,18 +498,16 @@ namespace dd4hep {
Key history_key;
EnergyDeposit dep { };
const auto* h = depo.second.get();
Position pos = h->position;
pos *= 1./dd4hep::mm;

dep.flag = h->flag;
dep.deposit = h->energyDeposit;
dep.position = pos;
dep.position = (h->position / dd4hep::mm);

history_key.set_mask(key.mask());
history_key.set_item(out.size());
history_key.set_segment(key.segment());
dep.history.hits.emplace_back(history_key, dep.deposit);
add_particle_history(h, history_key, dep.history);
add_particle_history(h, std::move(history_key), dep.history);
out.emplace(depo.first, std::move(dep));
}

Expand Down
Loading
Loading