From edfb426640ec8841ddf25f29348c22e205b0438c Mon Sep 17 00:00:00 2001
From: Giovanna Lazzari Miotto <giovanna.lazzari.miotto@cern.ch>
Date: Fri, 14 Feb 2025 17:31:08 +0100
Subject: [PATCH] tidy: Improve code quality
---
src/cmssw/FRDFileHeader_v2.h | 41 ++++++++++++++++++------------------
src/config.h | 4 +++-
src/multi_pipeline.cc | 2 +-
src/orbit_processor.cc | 29 ++++++++++++-------------
src/orbit_processor.h | 2 +-
src/sink.cc | 10 ++++-----
src/slice.h | 8 +++----
src/tools.cc | 2 +-
src/tools.h | 4 ++--
9 files changed, 51 insertions(+), 51 deletions(-)
diff --git a/src/cmssw/FRDFileHeader_v2.h b/src/cmssw/FRDFileHeader_v2.h
index 65afd245..15c8a94f 100644
--- a/src/cmssw/FRDFileHeader_v2.h
+++ b/src/cmssw/FRDFileHeader_v2.h
@@ -1,33 +1,32 @@
#ifndef FRDFileHeader_v2_H
#define FRDFileHeader_v2_H
+
#include <array>
+
// Straight from CMSSW
constexpr std::array<unsigned char, 4> FRDFileHeader_id{{0x52, 0x41, 0x57, 0x5f}};
constexpr std::array<unsigned char, 4> FRDFileVersion_2{{0x30, 0x30, 0x30, 0x32}};
-class FRDFileHeader_v2 {
- public:
- FRDFileHeader_v2(uint16_t dataType, uint32_t eventCount, uint32_t runNumber, uint32_t lumiSection,
- uint64_t fileSize) {
- id_ = FRDFileHeader_id;
- version_ = FRDFileVersion_2;
- headerSize_ = sizeof(FRDFileHeader_v2);
- dataType_ = dataType;
- eventCount_ = eventCount;
- runNumber_ = runNumber;
- lumiSection_ = lumiSection;
- fileSize_ = fileSize;
- }
-
- private:
+struct FRDFileHeader_v2 {
std::array<unsigned char, 4> id_;
std::array<unsigned char, 4> version_;
- uint16_t headerSize_;
- uint16_t dataType_;
- uint32_t eventCount_;
- uint32_t runNumber_;
- uint32_t lumiSection_;
- uint64_t fileSize_;
+ uint16_t header_size_;
+ uint16_t data_type_;
+ uint32_t event_count_;
+ uint32_t run_number_;
+ uint32_t lumisection_;
+ uint64_t file_size_;
+
+ FRDFileHeader_v2(uint16_t data_type, uint32_t event_count, uint32_t run_number,
+ uint32_t lumisection, uint64_t file_size)
+ : id_(FRDFileHeader_id),
+ version_(FRDFileVersion_2),
+ header_size_(sizeof(FRDFileHeader_v2)),
+ data_type_(data_type),
+ event_count_(event_count),
+ run_number_(run_number),
+ lumisection_(lumisection),
+ file_size_(file_size) {}
};
#endif
diff --git a/src/config.h b/src/config.h
index 8a01df3e..14abaefa 100644
--- a/src/config.h
+++ b/src/config.h
@@ -66,7 +66,9 @@ class ConfigMap {
// Insert (along with other functions) takes `key` as a const-ref since `ValueType`'s operator[]
// does not support `std::string_view`.
- void Insert(const std::string &key, ValueType val) { data_[key] = std::move(val); }
+ [[maybe_unused]] void Insert(const std::string &key, ValueType val) {
+ data_[key] = std::move(val);
+ }
void Import(const ValueType &values) { data_ = values; }
diff --git a/src/multi_pipeline.cc b/src/multi_pipeline.cc
index 1e6bcad4..55d3435a 100644
--- a/src/multi_pipeline.cc
+++ b/src/multi_pipeline.cc
@@ -72,7 +72,7 @@ void MultiPipeline::MonitorRun() {
ready_to_run_ = true;
if (pipelines_.empty()) {
- // If pipelines were tore down, re-instantiate them
+ // If pipelines were torn down, re-instantiate them
try {
MakePipelines();
} catch (std::exception &e) {
diff --git a/src/orbit_processor.cc b/src/orbit_processor.cc
index 575971d8..803dffc6 100644
--- a/src/orbit_processor.cc
+++ b/src/orbit_processor.cc
@@ -73,7 +73,7 @@ bool OrbitProcessor::CheckFrameMultBlock(size_t inputSize, uint16_t nDroppedOrbi
bool OrbitProcessor::HasTrailer(Slice &input, char *&rd_ptr) const {
rd_ptr += 32 + 32 * static_cast<int>(dthHeaders); // +32 to account for orbit header
while (rd_ptr + sizeof(orbit_trailer) - 1 <= input.end()) {
- auto *ot = reinterpret_cast<orbit_trailer *>(rd_ptr);
+ auto *ot = reinterpret_cast<const orbit_trailer *>(rd_ptr);
if (ot->beefdead[0] == constants::beefdead) { // found orbit trailer
return true;
}
@@ -82,10 +82,10 @@ bool OrbitProcessor::HasTrailer(Slice &input, char *&rd_ptr) const {
return false;
}
-std::pair<uint32_t, bool> OrbitProcessor::ProcessOrbitHeader(char *rd_ptr) {
+std::pair<uint32_t, bool> OrbitProcessor::ProcessOrbitHeader(const char *rd_ptr) {
// get orbit from orbit header
- auto *bl_pre =
- reinterpret_cast<blockMuon *>(rd_ptr); // blockMuon.orbit is identical to blockCalo.frame0
+ auto *bl_pre = reinterpret_cast<const blockMuon *>(
+ rd_ptr); // blockMuon.orbit is identical to blockCalo.frame0
auto orbitN = uint32_t{bl_pre->orbit[0]};
// save warning_test_enable bit
auto mask = uint32_t(1) << 31;
@@ -178,22 +178,19 @@ OrbitProcessor::FillOrbitMetadata OrbitProcessor::FillOrbit(orbit_trailer *trail
void OrbitProcessor::ProcessSliceImpl(Slice &input, Slice &out) {
char *rd_ptr = input.begin();
char *wr_ptr = out.begin();
- char *rd_end_ptr = input.end();
- char *wr_end_ptr = out.begin() + out.avail();
+ const char *rd_end_ptr = input.end();
+ const char *wr_end_ptr = out.begin() + out.avail();
uint32_t counts = 0;
uint32_t orbit_per_packet_count = 0;
bool firstOrbit = true;
uint16_t n_dropped_orbits_in_packet = 0;
orbit_trailer *trailer = nullptr;
- packet_header *ph_header = nullptr; // packet header frame
- fragment_header *fh_header = nullptr; // fragment header frame
bool is_dropped_orbit = false;
auto run_number = input.GetRunNumber();
- FillOrbitMetadata meta{0, 0, 0};
-
if (dthHeaders) {
- ph_header = reinterpret_cast<packet_header *>(rd_ptr);
+ // Packet header frame
+ auto ph_header = reinterpret_cast<const packet_header *>(rd_ptr);
n_dropped_orbits_in_packet = ph_header->dropped_orbit_in_packet;
rd_ptr += 32; // 0; // commented for fastTcpRead implementation
}
@@ -207,7 +204,8 @@ void OrbitProcessor::ProcessSliceImpl(Slice &input, Slice &out) {
char *trailer_ptr = rd_ptr;
if (dthHeaders) {
- fh_header = reinterpret_cast<fragment_header *>(rd_ptr);
+ // Fragment header frame
+ auto fh_header = reinterpret_cast<const fragment_header *>(rd_ptr);
is_dropped_orbit = (fh_header->flag_dropped_orbit == 1);
rd_ptr += 32;
}
@@ -226,13 +224,14 @@ void OrbitProcessor::ProcessSliceImpl(Slice &input, Slice &out) {
}
size_t additional_header_size = 0;
- meta = FillOrbit(trailer, rd_ptr, wr_ptr, rd_end_ptr, wr_end_ptr, is_dropped_orbit);
+ FillOrbitMetadata meta =
+ FillOrbit(trailer, rd_ptr, wr_ptr, rd_end_ptr, wr_end_ptr, is_dropped_orbit);
if (meta.orbit <= 0) {
LOG(WARNING) << "Invalid orbit number " << meta.orbit << ". Skipping packet...";
return;
}
orbitCount = meta.counts;
- ++orbit_per_packet_count;
+ orbit_per_packet_count += 1;
// NOTE: Only relevant for GMT processor, as number of orbits is always 1 in the CALO case.
if (orbit_per_packet_count > nOrbitsPerPacket) {
@@ -263,7 +262,7 @@ void OrbitProcessor::ProcessSliceImpl(Slice &input, Slice &out) {
}
if (rd_ptr < input.end()) {
- auto *dma_trailer_word = reinterpret_cast<uint32_t *>(rd_ptr);
+ auto *dma_trailer_word = reinterpret_cast<const uint32_t *>(rd_ptr);
if (*dma_trailer_word == constants::deadbeef) {
out.set_end(wr_ptr);
out.set_counts(counts);
diff --git a/src/orbit_processor.h b/src/orbit_processor.h
index 63daf392..8f0d36a0 100644
--- a/src/orbit_processor.h
+++ b/src/orbit_processor.h
@@ -107,7 +107,7 @@ class OrbitProcessor : public Processor {
bool HasTrailer(Slice &input, char *&rd_ptr) const;
bool CheckFrameMultBlock(size_t inputSize, uint16_t nDroppedOrbitsInPacket) const;
- std::pair<uint32_t, bool> ProcessOrbitHeader(char *rd_ptr);
+ std::pair<uint32_t, bool> ProcessOrbitHeader(const char *rd_ptr);
size_t fillFRDEventHeader_V6(char *wr_ptr_FRDHead, uint32_t inputSize, uint32_t run,
uint32_t orbit) const;
diff --git a/src/sink.cc b/src/sink.cc
index ecbbb9fb..5ac25d62 100644
--- a/src/sink.cc
+++ b/src/sink.cc
@@ -25,11 +25,11 @@ template <typename TOutput>
void Sink<TOutput>::CommitFileHeader(TOutput &file) {
auto header = file.GetHeader();
if (header.has_value()) {
- size_t header_size = sizeof(header.value());
- char packed[header_size];
- memcpy(packed, &header.value(), header_size);
- const char *packed_ptr = reinterpret_cast<const char *>(packed);
- file.WriteAt(packed_ptr, sizeof(header.value()), 0);
+ static_assert(std::is_trivially_copyable<typename TOutput::HeaderType>::value,
+ "Output header type must be trivially copyable");
+ const typename TOutput::HeaderType &header_value = header.value();
+ const char *write_ptr = reinterpret_cast<const char *>(&header_value);
+ file.WriteAt(write_ptr, sizeof(header_value), 0);
}
}
diff --git a/src/slice.h b/src/slice.h
index 997c0bac..902d0fd7 100644
--- a/src/slice.h
+++ b/src/slice.h
@@ -9,11 +9,11 @@
//! Holds a slice of data.
class Slice {
//! Pointer to one past last filled byte in sequence
- char *logical_end;
+ char *logical_end{};
//! Pointer to one past last available byte in sequence.
- char *physical_end;
- uint32_t counts;
- bool output;
+ char *physical_end{};
+ uint32_t counts{};
+ bool output{};
static tbb::concurrent_bounded_queue<Slice *> free_slices;
uint32_t first_orbit_{-UINT32_MAX};
uint32_t run_number_{-UINT32_MAX};
diff --git a/src/tools.cc b/src/tools.cc
index 565bc1b5..b5e45ae9 100644
--- a/src/tools.cc
+++ b/src/tools.cc
@@ -45,7 +45,7 @@ static size_t WriteCallback(void *, size_t size, size_t nmemb, void *) { return
/*
* Helper function to reset the board via SCONE
*/
-int tools::reset_board(std::string host, std::string port, std::string board) {
+int tools::reset_board(const std::string host, const std::string port, const std::string board) {
std::string url{"http://" + host + ":" + port + "/" + board + "/reset_board/write"};
CURL *curl;
curl = curl_easy_init();
diff --git a/src/tools.h b/src/tools.h
index 2f8ef242..e568ce96 100644
--- a/src/tools.h
+++ b/src/tools.h
@@ -47,7 +47,7 @@ inline std::string strerror(const std::string &msg) { return msg + ": " + tools:
/*
* Helper function to reset the board via SCONE
*/
-int reset_board(std::string host, std::string port, std::string board);
+int reset_board(const std::string host, const std::string port, const std::string board);
/*
* Various filesystem related utilities (will be removed once moved to C++17, or
@@ -58,7 +58,7 @@ namespace filesystem {
/*
* Create the target directory and any parent directories if necessary
*/
-inline bool create_directories(std::string &path) {
+inline bool create_directories(const std::string &path) {
char tmp[PATH_MAX];
// Add terminating '/' and make a writable copy;
--
GitLab