Address potential Sonar issues

cpp/s2pdump/board_executor.cpp

@@ -45,7 +45,7 @@ set<int> BoardExecutor::ReportLuns(vector<uint8_t> &cdb, span<uint8_t> buf)
 
     set<int> luns;
     int offset = 8;
-    for (size_t i = 0; i < lun_count && static_cast<size_t>(offset) + 8 < buf.size(); i++, offset += 8) {
+    for (size_t i = 0; i < lun_count && static_cast<size_t>(offset) + 8 < buf.size(); ++i, offset += 8) {
         const uint64_t lun = GetInt64(buf, offset);
         if (lun < 32) {
             luns.insert(static_cast<int>(lun));

cpp/s2pdump/board_executor.h

@@ -27,7 +27,6 @@ class BoardExecutor : public S2pDumpExecutor
         make_unique<InitiatorExecutor>(bus, id, l))
     {
     }
-    virtual ~BoardExecutor() = default;
 
     // Disk and tape support
     void TestUnitReady(vector<uint8_t>&) const override;

cpp/s2pdump/s2pdump_core.cpp

@@ -262,7 +262,8 @@ bool S2pDump::ParseArguments(span<char*> args) // NOSONAR Acceptable complexity
     }
 
     if (!initiator.empty()) {
-        if (initiator_id = ParseAsUnsignedInt(initiator); initiator_id < 0 || initiator_id > 7) {
+        initiator_id = ParseAsUnsignedInt(initiator);
+        if (initiator_id < 0 || initiator_id > 7) {
             throw ParserException("Invalid initiator ID '" + initiator + "' (0-7)");
         }
     }
@@ -284,36 +285,42 @@ bool S2pDump::ParseArguments(span<char*> args) // NOSONAR Acceptable complexity
         }
 
         if (!buf.empty()) {
-            if (buffer_size = ParseAsUnsignedInt(buf); buffer_size < MINIMUM_BUFFER_SIZE) {
+            buffer_size = ParseAsUnsignedInt(buf);
+            if (buffer_size < MINIMUM_BUFFER_SIZE) {
                 throw ParserException(
                     "Buffer size must be at least " + to_string(MINIMUM_BUFFER_SIZE / 1024) + " KiB");
             }
         }
 
         if (!sector_count.empty()) {
-            if (count = ParseAsUnsignedInt(sector_count); count <= 0) {
+            count = ParseAsUnsignedInt(sector_count);
+            if (count <= 0) {
                 throw ParserException("Invalid sector count: '" + sector_count + "'");
             }
         }
 
         if (!start_sector.empty()) {
-            if (start = ParseAsUnsignedInt(start_sector); start < 0) {
+            start = ParseAsUnsignedInt(start_sector);
+            if (start < 0) {
                 throw ParserException("Invalid start sector: " + string(optarg));
             }
         }
 
         if (!retry_count.empty()) {
-            if (retries = ParseAsUnsignedInt(retry_count); retries < 0) {
+            retries = ParseAsUnsignedInt(retry_count);
+            if (retries < 0) {
                 throw ParserException("Invalid retry count: " + string(optarg));
             }
         }
 
         if (sasi) {
-            if (sasi_capacity = ParseAsUnsignedInt(capacity); sasi_capacity <= 0) {
+            sasi_capacity = ParseAsUnsignedInt(capacity);
+            if (sasi_capacity <= 0) {
                 throw ParserException("Invalid SASI hard drive capacity: '" + capacity + "'");
             }
 
-            if (sasi_sector_size = ParseAsUnsignedInt(sector_size); sasi_sector_size != 256 && sasi_sector_size != 512
+            sasi_sector_size = ParseAsUnsignedInt(sector_size);
+            if (sasi_sector_size != 256 && sasi_sector_size != 512
                 && sasi_sector_size != 1024) {
                 throw ParserException("Invalid SASI hard drive sector size: '" + sector_size + "'");
             }
@@ -423,7 +430,7 @@ void S2pDump::ScanBus()
 {
     DisplayBoardId();
 
-    for (target_id = 0; target_id < 8; target_id++) {
+    for (target_id = 0; target_id < 8; ++target_id) {
         if (initiator_id == target_id) {
             continue;
         }
@@ -664,10 +671,10 @@ string S2pDump::DumpRestoreTape(fstream &file)
     return "";
 }
 
-string S2pDump::ReadWrite(fstream &file, int sector_offset, uint32_t sector_count, int sector_size, int byte_count)
+string S2pDump::ReadWrite(fstream &file, int sector_offset, uint32_t sector_count, int sector_size, int bytes)
 {
     if (restore) {
-        file.read((char*)buffer.data(), byte_count);
+        file.read((char*)buffer.data(), bytes);
         if (file.fail()) {
             return "Can't read from file '" + filename + "': " + strerror(errno);
         }
@@ -696,7 +703,7 @@ string S2pDump::ReadWrite(fstream &file, int sector_offset, uint32_t sector_coun
             return "Can't read from device";
         }
 
-        file.write((const char*)buffer.data(), byte_count);
+        file.write((const char*)buffer.data(), bytes);
         if (file.fail()) {
             return "Can't write to file '" + filename + "': " + strerror(errno);
         }
@@ -963,7 +970,7 @@ void S2pDump::DisplayProperties(int id, int lun) const
             ++offset;
         }
 
-        for (int i = 0; i < page_length && offset < length; i++, offset++) {
+        for (int i = 0; i < page_length && offset < length; ++i, ++offset) {
             cout << fmt::format(":{:02x}", buf[offset]);
         }
 

cpp/s2pdump/s2pdump_executor.cpp

@@ -171,10 +171,10 @@ int S2pDumpExecutor::ReadWrite(span<uint8_t> buf, int length)
             throw IoException(fmt::format("Unknown error status {}", status));
         }
 
-        const SenseKey sense_key = static_cast<SenseKey>(sense_data[2] & 0x0f);
+        const SenseKey sense_key = static_cast<SenseKey>(static_cast<int>(sense_data[2]) & 0x0f);
 
         // EOD or EOM?
-        if (sense_key == SenseKey::BLANK_CHECK || sense_data[2] & 0x40) {
+        if (sense_key == SenseKey::BLANK_CHECK || static_cast<int>(sense_data[2]) & 0x40) {
             GetLogger().debug("No more data");
             return NO_MORE_DATA;
         }
@@ -191,13 +191,13 @@ int S2pDumpExecutor::ReadWrite(span<uint8_t> buf, int length)
             continue;
         }
 
-        if (sense_data[2] & 0x80) {
+        if (static_cast<int>(sense_data[2]) & 0x80) {
             GetLogger().debug("Encountered filemark");
             return 0;
         }
 
         // VALID and ILI?
-        if (sense_data[0] & 0x80 && sense_data[2] & 0x20) {
+        if (static_cast<int>(sense_data[0]) & 0x80 && static_cast<int>(sense_data[2]) & 0x20) {
             length = default_length;
 
             default_length -= GetInt32(sense_data, 3);

cpp/s2pdump/s2pdump_executor.h

@@ -62,9 +62,10 @@ class S2pDumpExecutor
 
 protected:
 
-    S2pDumpExecutor(logger &l) : s2pdump_logger(l)
+    explicit S2pDumpExecutor(logger &l) : s2pdump_logger(l)
     {
     }
+    virtual ~S2pDumpExecutor() = default;
 
     void SpaceBack() const;
     virtual void SpaceBack(vector<uint8_t>&) const = 0;
@@ -76,5 +77,6 @@ class S2pDumpExecutor
         return s2pdump_logger;
     }
 
+private:
     int default_length = 0xffffff;
 };

cpp/s2pdump/sg_executor.h

@@ -27,7 +27,6 @@ class SgExecutor : public S2pDumpExecutor
     SgExecutor(SgAdapter &adapter, logger &l) : S2pDumpExecutor(l), sg_adapter(adapter)
     {
     }
-    virtual ~SgExecutor() = default;
 
     // Disk and tape support
     void TestUnitReady(vector<uint8_t>&) const override;

cpp/s2pexec/s2pexec_core.cpp

@@ -255,7 +255,8 @@ bool S2pExec::ParseArguments(span<char*> args, bool in_process)
     }
 
     if (!initiator.empty()) {
-        if (initiator_id = ParseAsUnsignedInt(initiator); initiator_id < 0 || initiator_id > 7) {
+        initiator_id = ParseAsUnsignedInt(initiator);
+        if (initiator_id < 0 || initiator_id > 7) {
             throw ParserException("Invalid initiator ID: '" + initiator + "' (0-7)");
         }
     }
@@ -333,7 +334,8 @@ bool S2pExec::ParseArguments(span<char*> args, bool in_process)
 
     int buffer_size = DEFAULT_BUFFER_SIZE;
     if (!buf.empty()) {
-        if (buffer_size = ParseAsUnsignedInt(buf); buffer_size <= 0) {
+        buffer_size = ParseAsUnsignedInt(buf);
+        if (buffer_size <= 0) {
             throw ParserException("Invalid receive buffer size: '" + buf + "'");
         }
     }
@@ -373,7 +375,7 @@ void S2pExec::RunInteractive(bool in_process)
         vector<char*> interactive_args;
         interactive_args.emplace_back(strdup(APP_NAME.c_str()));
         interactive_args.emplace_back(strdup(args[0].c_str()));
-        for (size_t i = 1; i < args.size(); i++) {
+        for (size_t i = 1; i < args.size(); ++i) {
             if (!args[i].empty()) {
                 interactive_args.emplace_back(strdup(args[i].c_str()));
             }
@@ -452,7 +454,7 @@ int S2pExec::Run()
             }
         }
     }
-    catch (const execution_exception &e) {
+    catch (const ExecutionException &e) {
         cerr << "Error: " << e.what() << '\n';
         result = -1;
     }
@@ -469,20 +471,20 @@ tuple<SenseKey, Asc, int> S2pExec::ExecuteCommand()
     }
     catch (const out_of_range&)
     {
-        throw execution_exception("Invalid CDB input format: '" + command + "'");
+        throw ExecutionException("Invalid CDB input format: '" + command + "'");
     }
 
     vector<uint8_t> cdb;
     ranges::transform(cmd_bytes, back_inserter(cdb), [](const byte b) {return static_cast<uint8_t>(b) & 0xff;});
 
     if (!data.empty()) {
         if (const string &error = ConvertData(data); !error.empty()) {
-            throw execution_exception(error);
+            throw ExecutionException(error);
         }
     }
     else if (!binary_input_filename.empty() || !hex_input_filename.empty()) {
         if (const string &error = ReadData(); !error.empty()) {
-            throw execution_exception(error);
+            throw ExecutionException(error);
         }
     }
 
@@ -493,10 +495,10 @@ tuple<SenseKey, Asc, int> S2pExec::ExecuteCommand()
                 return executor->GetSenseData();
             }
 
-            throw execution_exception(GetStatusString(status_code));
+            throw ExecutionException(GetStatusString(status_code));
         }
         else {
-            throw execution_exception(fmt::format("Can't execute command {0} (${1:2x})",
+            throw ExecutionException(fmt::format("Can't execute command {0} (${1:2x})",
                 CommandMetaData::GetInstance().GetCommandName(static_cast<ScsiCommand>(cdb[0])), cdb[0]));
         }
     }
@@ -512,7 +514,7 @@ tuple<SenseKey, Asc, int> S2pExec::ExecuteCommand()
         if (const int count = executor->GetByteCount(); count) {
             s2pexec_logger->debug("Received {} data byte(s)", count);
             if (const string &error = WriteData(span<const uint8_t>(buffer.begin(), buffer.begin() + count)); !error.empty()) {
-                throw execution_exception(error);
+                throw ExecutionException(error);
             }
         }
 

cpp/s2pexec/s2pexec_core.h

@@ -16,7 +16,7 @@ using namespace std;
 
 class S2pExec
 {
-    class execution_exception : public runtime_error
+    class ExecutionException : public runtime_error
     {
         using runtime_error::runtime_error;
     };

cpp/s2pformat/s2pformat_core.cpp

@@ -173,7 +173,7 @@ vector<S2pFormat::FormatDescriptor> S2pFormat::GetFormatDescriptors()
 
     for (auto i = 12; i < buf[3]; i += 8) {
         // Ignore other format types than 0
-        if (!(buf[i + 4] & 0x03)) {
+        if (!(static_cast<int>(buf[i + 4]) & 0x03)) {
             descriptors.push_back( { GetInt32(buf, i), GetInt32(buf, i + 4) & 0xffffff });
         }
     }
@@ -199,7 +199,7 @@ int S2pFormat::SelectFormat(span<const S2pFormat::FormatDescriptor> descriptors)
     try {
         n = stoi(input);
     }
-    catch (const invalid_argument&)
+    catch (const invalid_argument&) // NOSONAR The exception details do not matter
     {
         // Fall through
     }

cpp/s2pproto/s2pproto_core.cpp

@@ -181,7 +181,8 @@ bool S2pProto::ParseArguments(span<char*> args)
         throw ParserException("Invalid log level: '" + log_level + "'");
     }
 
-    if (initiator_id = ParseAsUnsignedInt(initiator); initiator_id < 0 || initiator_id > 7) {
+    initiator_id = ParseAsUnsignedInt(initiator);
+    if (initiator_id < 0 || initiator_id > 7) {
         throw ParserException("Invalid initiator ID: '" + initiator + "' (0-7)");
     }