Use stricter code quality rules

cpp/base/primary_device.h

@@ -24,10 +24,10 @@ class PrimaryDevice : public Device
 
 public:
 
-    using ProductData = struct _ProductData {
-        string vendor = "SCSI2Pi";
+    using ProductData = struct {
+        string vendor;
         string product;
-        string revision = fmt::format("{0:02}{1:1}{2:1}", s2p_major_version, s2p_minor_version, s2p_revision);;
+        string revision;
     };
 
     string Init();
@@ -183,7 +183,8 @@ class PrimaryDevice : public Device
 
     vector<byte> HandleRequestSense() const;
 
-    ProductData product_data;
+    ProductData product_data = ProductData(
+        { "SCSI2Pi", "", fmt::format("{0:02}{1:1}{2:1}", s2p_major_version, s2p_minor_version, s2p_revision) });
 
     ScsiLevel level = ScsiLevel::SCSI_2;
     ScsiLevel response_data_format = ScsiLevel::SCSI_2;

cpp/buses/rpi_bus.cpp

@@ -207,8 +207,8 @@ void RpiBus::Reset()
     SetControl(PIN_ACT, false);
 
     // Set all signals to off
-    for (const int signal : SIGNAL_TABLE) {
-        SetSignal(signal, false);
+    for (const int s : SIGNAL_TABLE) {
+        SetSignal(s, false);
     }
 
     // Set target signal to input for all modes
@@ -234,15 +234,6 @@ void RpiBus::Reset()
     signals = 0;
 }
 
-void RpiBus::InitializeSignals(int pull_mode)
-{
-    for (const int signal : SIGNAL_TABLE) {
-        PinSetSignal(signal, false);
-        PinConfig(signal, GPIO_INPUT);
-        PullConfig(signal, pull_mode);
-    }
-}
-
 bool RpiBus::WaitForSelection()
 {
 #ifndef __linux__
@@ -335,6 +326,15 @@ inline void RpiBus::SetDAT(uint8_t dat)
     gpio[GPIO_FSEL_1] = fsel;
 }
 
+void RpiBus::InitializeSignals(int pull_mode)
+{
+    for (const int s : SIGNAL_TABLE) {
+        PinSetSignal(s, false);
+        PinConfig(s, GPIO_INPUT);
+        PullConfig(s, pull_mode);
+    }
+}
+
 void RpiBus::CreateWorkTable()
 {
     array<bool, 256> tblParity;

cpp/command/command_context.cpp

@@ -72,10 +72,10 @@ bool CommandContext::ReturnLocalizedError(LocalizationKey key, PbErrorCode error
     return ReturnStatus(false, command_localizer.Localize(key, locale, arg1, arg2, arg3), error_code, false);
 }
 
-bool CommandContext::ReturnStatus(bool status, const string &msg, PbErrorCode error_code, bool log) const
+bool CommandContext::ReturnStatus(bool status, const string &msg, PbErrorCode error_code, bool enable_log) const
 {
     // Do not log twice if logging has already been done in the localized error handling above
-    if (log && !status && !msg.empty()) {
+    if (enable_log && !status && !msg.empty()) {
         s2p_logger.error(msg);
     }
 

cpp/devices/daynaport.cpp

@@ -145,7 +145,7 @@ int DaynaPort::GetMessage(data_in_t buf)
     // If we didn't receive anything, return size of 0
     if (rx_packet_size <= 0) {
         LogTrace("No network packet received");
-        auto *response = (scsi_resp_read_t*)buf.data();
+        auto *response = (ScsiRespRead*)buf.data();
         response->length = 0;
         response->flags = ReadDataFlagsType::NO_MORE_DATA;
         return DAYNAPORT_READ_HEADER_SZ;

cpp/devices/daynaport.h

@@ -81,21 +81,21 @@ class DaynaPort : public PrimaryDevice
         DROPPED_PACKETS = 0xFFFFFFFF,
     };
 
-    using scsi_resp_read_t = struct __attribute__((packed)) {
+    using ScsiRespRead = struct __attribute__((packed)) {
         uint32_t length;
         ReadDataFlagsType flags;
         byte pad;
         array<byte, ETH_FRAME_LEN + sizeof(uint32_t)> data; // Frame length + 4 byte CRC
     };
 
-    using scsi_resp_link_stats_t = struct __attribute__((packed)) {
+    using ScsiRespLinkStats = struct __attribute__((packed)) {
         array<byte, 6> mac_address;
         uint32_t frame_alignment_errors;
         uint32_t crc_errors;
         uint32_t frames_lost;
     };
 
-    static constexpr scsi_resp_link_stats_t SCSI_LINK_STATS = {
+    static constexpr ScsiRespLinkStats SCSI_LINK_STATS = {
         // The last 3 bytes of this MAC address are replaced by those of the bridge interface
         .mac_address = { byte { 0x00 }, byte { 0x80 }, byte { 0x19 }, byte { 0x10 }, byte { 0x98 }, byte { 0xe3 } },
         .frame_alignment_errors = 0,

cpp/devices/disk_cache.cpp

@@ -16,31 +16,31 @@
 #include <cassert>
 #include "disk_track.h"
 
-DiskCache::DiskCache(const string &path, int size, uint64_t sectors) : sec_path(path), sec_blocks(
+DiskCache::DiskCache(const string &path, int size, uint64_t sectors) : sec_path(path), blocks(
     static_cast<int>(sectors))
 {
-    while ((1 << sec_size) != size) {
-        ++sec_size;
+    while ((1 << shift_count) != size) {
+        ++shift_count;
     }
-    assert(sec_size >= 8 && sec_size <= 12);
+    assert(shift_count >= 8 && shift_count <= 12);
 }
 
 bool DiskCache::Init()
 {
-    return sec_blocks && !sec_path.empty();
+    return blocks && !sec_path.empty();
 }
 
 bool DiskCache::Flush()
 {
     // Save valid tracks
-    return ranges::none_of(cache, [this](const cache_t &c)
+    return ranges::none_of(cache, [this](const CacheData &c)
         {   return c.disktrk && !c.disktrk->Save(sec_path, cache_miss_write_count);});
 }
 
 shared_ptr<DiskTrack> DiskCache::GetTrack(uint32_t block)
 {
     // Update first
-    UpdateSerialNumber();
+    UpdateSerial();
 
     // Calculate track (fixed to 256 sectors/track)
     int track = block >> 8;
@@ -87,7 +87,7 @@ shared_ptr<DiskTrack> DiskCache::Assign(int track)
     assert(track >= 0);
 
     // First, check if it is already assigned
-    for (cache_t &c : cache) {
+    for (CacheData &c : cache) {
         if (c.disktrk && c.disktrk->GetTrack() == track) {
             // Track match
             c.serial = serial;
@@ -153,7 +153,7 @@ bool DiskCache::Load(int index, int track, shared_ptr<DiskTrack> disktrk)
     assert(!cache[index].disktrk);
 
     // Get the number of sectors on this track
-    int sectors = sec_blocks - (track << 8);
+    int sectors = blocks - (track << 8);
     assert(sectors > 0);
     if (sectors > 0x100) {
         sectors = 0x100;
@@ -163,7 +163,7 @@ bool DiskCache::Load(int index, int track, shared_ptr<DiskTrack> disktrk)
         disktrk = make_shared<DiskTrack>();
     }
 
-    disktrk->Init(track, sec_size, sectors);
+    disktrk->Init(track, shift_count, sectors);
 
     // Try loading
     if (!disktrk->Load(sec_path, cache_miss_read_count)) {
@@ -178,7 +178,7 @@ bool DiskCache::Load(int index, int track, shared_ptr<DiskTrack> disktrk)
     return true;
 }
 
-void DiskCache::UpdateSerialNumber()
+void DiskCache::UpdateSerial()
 {
     // Update and do nothing except 0
     ++serial;
@@ -187,7 +187,7 @@ void DiskCache::UpdateSerialNumber()
     }
 
     // Clear serial of all caches
-    for (cache_t &c : cache) {
+    for (CacheData &c : cache) {
         c.serial = 0;
     }
 }

cpp/devices/disk_cache.h

@@ -5,8 +5,6 @@
 // Copyright (C) 2001-2006 ＰＩ．([email protected])
 // Copyright (C) 2014-2020 GIMONS
 //
-// XM6i
-// Copyright (C) 2010-2015 [email protected]
 // Copyright (C) 2022-2024 Uwe Seimet
 //
 //---------------------------------------------------------------------------
@@ -34,28 +32,30 @@ class DiskCache : public Cache
 
 private:
 
-    using cache_t = struct {
+    using CacheData = struct {
         shared_ptr<DiskTrack> disktrk;
         uint32_t serial;
     };
 
     shared_ptr<DiskTrack> Assign(int);
     shared_ptr<DiskTrack> GetTrack(uint32_t);
     bool Load(int index, int track, shared_ptr<DiskTrack>);
-    void UpdateSerialNumber();
+    void UpdateSerial();
 
     // Number of tracks to cache
     static constexpr int CACHE_MAX = 16;
 
-    array<cache_t, CACHE_MAX> cache = { };
+    array<CacheData, CACHE_MAX> cache = { };
+
     // Last serial number
     uint32_t serial = 0;
-    // Path
+
     string sec_path;
-    // Sector size shift (8=256, 9=512, 10=1024, 11=2048, 12=4096)
-    int sec_size = 8;
-    // Blocks
-    int sec_blocks;
+
+    // Sector size shift  (8 = 256, 9 = 512, 10 = 1024, 11 = 2048, 12 = 4096)
+    int shift_count = 8;
+
+    int blocks;
 
     uint64_t read_error_count = 0;
     uint64_t write_error_count = 0;

cpp/devices/host_services.cpp

@@ -253,23 +253,23 @@ void HostServices::SetUpModePages(map<int, vector<byte>> &pages, int page, bool
 
 void HostServices::AddRealtimeClockPage(map<int, vector<byte>> &pages, bool changeable) const
 {
-    pages[32] = vector<byte>(sizeof(mode_page_datetime) + 2);
+    pages[32] = vector<byte>(sizeof(ModePageDateTime) + 2);
 
     if (!changeable) {
         const time_t &t = system_clock::to_time_t(system_clock::now());
-        tm localtime;
-        localtime_r(&t, &localtime);
+        tm local_time;
+        localtime_r(&t, &local_time);
 
-        mode_page_datetime datetime;
+        ModePageDateTime datetime;
         datetime.major_version = 0x01;
         datetime.minor_version = 0x00;
-        datetime.year = static_cast<uint8_t>(localtime.tm_year);
-        datetime.month = static_cast<uint8_t>(localtime.tm_mon);
-        datetime.day = static_cast<uint8_t>(localtime.tm_mday);
-        datetime.hour = static_cast<uint8_t>(localtime.tm_hour);
-        datetime.minute = static_cast<uint8_t>(localtime.tm_min);
+        datetime.year = static_cast<uint8_t>(local_time.tm_year);
+        datetime.month = static_cast<uint8_t>(local_time.tm_mon);
+        datetime.day = static_cast<uint8_t>(local_time.tm_mday);
+        datetime.hour = static_cast<uint8_t>(local_time.tm_hour);
+        datetime.minute = static_cast<uint8_t>(local_time.tm_min);
         // Ignore leap second for simplicity
-        datetime.second = static_cast<uint8_t>(localtime.tm_sec < 60 ? localtime.tm_sec : 59);
+        datetime.second = static_cast<uint8_t>(local_time.tm_sec < 60 ? local_time.tm_sec : 59);
 
         memcpy(&pages[32][2], &datetime, sizeof(datetime));
     }

cpp/devices/host_services.h

@@ -44,7 +44,7 @@ class HostServices : public PrimaryDevice
 
 private:
 
-    using mode_page_datetime = struct __attribute__((packed)) {
+    using ModePageDateTime = struct __attribute__((packed)) {
         // Major and minor version of this data structure (e.g. 1.0)
         uint8_t major_version;
         uint8_t minor_version;

cpp/devices/scsi_generic.cpp

@@ -184,7 +184,7 @@ int ScsiGeneric::WriteData(cdb_t, data_out_t buf, int, int length)
     return ReadWriteData(span((uint8_t*)buf.data(), buf.size()), length, true); // NOSONAR Cast required for SG driver API
 }
 
-int ScsiGeneric::ReadWriteData(span<uint8_t> buf, int chunk_size, bool log)
+int ScsiGeneric::ReadWriteData(span<uint8_t> buf, int chunk_size, bool enable_log)
 {
     int length = remaining_count < chunk_size ? remaining_count : chunk_size;
     length = length < MAX_TRANSFER_LENGTH ? length : MAX_TRANSFER_LENGTH;
@@ -218,11 +218,11 @@ int ScsiGeneric::ReadWriteData(span<uint8_t> buf, int chunk_size, bool log)
             TIMEOUT_FORMAT_SECONDS : TIMEOUT_DEFAULT_SECONDS) * 1000;
 
     // Check the log level in order to avoid an unnecessary time-consuming string construction
-    if (log && GetLogger().level() <= level::debug) {
+    if (enable_log && GetLogger().level() <= level::debug) {
         LogDebug(command_meta_data.LogCdb(local_cdb, "SG driver"));
     }
 
-    if (log && write && GetLogger().level() == level::trace) {
+    if (enable_log && write && GetLogger().level() == level::trace) {
         LogTrace(fmt::format("Transferring {0} byte(s) to SG driver:\n{1}", length,
             GetController()->FormatBytes(buf, length)));
     }
@@ -231,11 +231,11 @@ int ScsiGeneric::ReadWriteData(span<uint8_t> buf, int chunk_size, bool log)
 
     format_header.clear();
 
-    EvaluateStatus(status, span(buf.data(), length), sense_data, write, log);
+    EvaluateStatus(status, span(buf.data(), length), sense_data, write, enable_log);
 
     const int transferred_length = length - io_hdr.resid;
 
-    if (log && !write && GetLogger().level() == level::trace) {
+    if (enable_log && !write && GetLogger().level() == level::trace) {
         LogTrace(fmt::format("Transferred {0} byte(s) from SG driver:\n{1}", transferred_length,
             GetController()->FormatBytes(buf, transferred_length)));
     }
@@ -252,17 +252,17 @@ int ScsiGeneric::ReadWriteData(span<uint8_t> buf, int chunk_size, bool log)
         remaining_count = 0;
     }
 
-    if (log) {
+    if (enable_log) {
         LogTrace(fmt::format("{0} byte(s) transferred, {1} byte(s) remaining", transferred_length, remaining_count));
     }
 
     return transferred_length;
 }
 
-void ScsiGeneric::EvaluateStatus(int status, span<uint8_t> buf, span<uint8_t> sense_data, bool write, bool log)
+void ScsiGeneric::EvaluateStatus(int status, span<uint8_t> buf, span<uint8_t> sense_data, bool write, bool enable_log)
 {
     if (status == -1) {
-        if (log) {
+        if (enable_log) {
             LogError(fmt::format("Transfer of {0} byte(s) failed: {1}", buf.size(), strerror(errno)));
         }
 

cpp/initiator/initiator_executor.cpp

@@ -17,13 +17,13 @@ using namespace s2p_util;
 using namespace initiator_util;
 
 int InitiatorExecutor::Execute(ScsiCommand cmd, span<uint8_t> cdb, span<uint8_t> buffer, int length, int timeout,
-    bool log)
+    bool enable_log)
 {
     cdb[0] = static_cast<uint8_t>(cmd);
-    return Execute(cdb, buffer, length, timeout, log);
+    return Execute(cdb, buffer, length, timeout, enable_log);
 }
 
-int InitiatorExecutor::Execute(span<uint8_t> cdb, span<uint8_t> buffer, int length, int timeout, bool log)
+int InitiatorExecutor::Execute(span<uint8_t> cdb, span<uint8_t> buffer, int length, int timeout, bool enable_log)
 {
     bus.Reset();
 
@@ -83,7 +83,7 @@ int InitiatorExecutor::Execute(span<uint8_t> cdb, span<uint8_t> buffer, int leng
         initiator_logger.error("Timeout");
     }
 
-    if (log) {
+    if (enable_log) {
         LogStatus();
     }
 

cpp/s2pdump/s2pdump_core.h

@@ -27,8 +27,7 @@ class S2pDump
 
     int Run(span<char*>, bool);
 
-    struct scsi_device_info
-    {
+    using ScsiDeviceInfo = struct {
         bool removable;
         byte type;
         byte scsi_level;
@@ -38,7 +37,6 @@ class S2pDump
         uint32_t sector_size;
         uint64_t capacity;
     };
-    using scsi_device_info_t = struct scsi_device_info;
 
 private:
 
@@ -72,7 +70,7 @@ class S2pDump
 
     shared_ptr<S2pDumpExecutor> s2pdump_executor;
 
-    scsi_device_info_t scsi_device_info = { };
+    ScsiDeviceInfo scsi_device_info = { };
 
     int sasi_capacity = 0;
     int sasi_sector_size = 0;