From a91c4810f6877ad57f01c77cf5a0dfadb3adf457 Mon Sep 17 00:00:00 2001 From: Uwe Seimet Date: Mon, 30 Dec 2024 21:52:02 +0100 Subject: [PATCH] Fix cppcheck warnings, use stricter code quality rules --- cpp/base/device_factory.h | 2 +- cpp/base/property_handler.cpp | 2 +- cpp/base/property_handler.h | 2 +- cpp/command/command_image_support.h | 2 +- cpp/devices/disk.cpp | 4 ++-- cpp/devices/scsi_generic.h | 2 +- cpp/devices/storage_device.h | 4 ++-- cpp/devices/tap_driver.cpp | 2 +- cpp/devices/tape.cpp | 4 ++-- cpp/s2pexec/s2pexec_core.cpp | 4 +--- cpp/shared/s2p_util.cpp | 10 ++++------ cpp/test/primary_device_test.cpp | 6 ++---- cpp/test/tape_test.cpp | 4 ++-- 13 files changed, 21 insertions(+), 27 deletions(-) diff --git a/cpp/base/device_factory.h b/cpp/base/device_factory.h index 44c782f7..bc65b033 100644 --- a/cpp/base/device_factory.h +++ b/cpp/base/device_factory.h @@ -33,7 +33,7 @@ class DeviceFactory shared_ptr CreateDevice(PbDeviceType, int, const string&) const; PbDeviceType GetTypeForFile(const string&) const; - auto GetExtensionMapping() const + const auto& GetExtensionMapping() const { return mapping; } diff --git a/cpp/base/property_handler.cpp b/cpp/base/property_handler.cpp index f41e0cd8..6cd7febb 100644 --- a/cpp/base/property_handler.cpp +++ b/cpp/base/property_handler.cpp @@ -93,7 +93,7 @@ property_map PropertyHandler::GetProperties(const string &filter) const return filtered_properties; } -property_map PropertyHandler::GetUnknownProperties() const +const property_map& PropertyHandler::GetUnknownProperties() const { return unknown_properties; } diff --git a/cpp/base/property_handler.h b/cpp/base/property_handler.h index f99e6ddf..49cca0d6 100644 --- a/cpp/base/property_handler.h +++ b/cpp/base/property_handler.h @@ -55,7 +55,7 @@ class PropertyHandler void Init(const string&, const property_map&, bool); property_map GetProperties(const string& = "") const; - property_map GetUnknownProperties() const; + const property_map& GetUnknownProperties() const; const string& RemoveProperty(const string&, const string& = ""); void AddProperty(const string&, string_view); void RemoveProperties(const string&); diff --git a/cpp/command/command_image_support.h b/cpp/command/command_image_support.h index 6429943d..a81032ee 100644 --- a/cpp/command/command_image_support.h +++ b/cpp/command/command_image_support.h @@ -36,7 +36,7 @@ class CommandImageSupport { return depth; } - string GetDefaultFolder() const + const string& GetDefaultFolder() const { return default_folder; } diff --git a/cpp/devices/disk.cpp b/cpp/devices/disk.cpp index 2d5484e2..56a06154 100644 --- a/cpp/devices/disk.cpp +++ b/cpp/devices/disk.cpp @@ -223,7 +223,7 @@ void Disk::Write(AccessMode mode) void Disk::Verify(AccessMode mode) { // A transfer length of 0 is legal - const auto& [_, start, count] = CheckAndGetStartAndCount(mode); + const auto& [valid, start, count] = CheckAndGetStartAndCount(mode); // Flush the cache according to the specification FlushCache(); @@ -431,7 +431,7 @@ int Disk::WriteData(cdb_t cdb, data_out_t buf, int, int l) void Disk::Seek(AccessMode mode) { - const auto& [valid, _, __] = CheckAndGetStartAndCount(mode); + const auto& [valid, start, count] = CheckAndGetStartAndCount(mode); if (valid) { CheckReady(); } diff --git a/cpp/devices/scsi_generic.h b/cpp/devices/scsi_generic.h index bae3bda7..0805a084 100644 --- a/cpp/devices/scsi_generic.h +++ b/cpp/devices/scsi_generic.h @@ -30,7 +30,7 @@ class ScsiGeneric : public PrimaryDevice return device + " (" + GetPaddedName() + ")"; } - string GetDevice() const + const string& GetDevice() const { return device; } diff --git a/cpp/devices/storage_device.h b/cpp/devices/storage_device.h index 56f50b22..fa256413 100644 --- a/cpp/devices/storage_device.h +++ b/cpp/devices/storage_device.h @@ -49,7 +49,7 @@ class StorageDevice : public PrimaryDevice { filename = filesystem::path(file); } - string GetLastFilename() const + const string& GetLastFilename() const { return last_filename; } @@ -93,7 +93,7 @@ class StorageDevice : public PrimaryDevice medium_changed = b; } - static auto GetReservedFiles() + static const auto& GetReservedFiles() { return reserved_files; } diff --git a/cpp/devices/tap_driver.cpp b/cpp/devices/tap_driver.cpp index 5cb2b653..d9adadae 100644 --- a/cpp/devices/tap_driver.cpp +++ b/cpp/devices/tap_driver.cpp @@ -221,7 +221,7 @@ pair TapDriver::ExtractAddressAndMask(logger &logger) const address = components[0]; const int m = ParseAsUnsignedInt(components[1]); - if (m == -1 || m < 8 || m > 32) { + if (m < 8 || m > 32) { logger.error("Invalid CIDR netmask notation '{}'", components[1]); return {"", ""}; } diff --git a/cpp/devices/tape.cpp b/cpp/devices/tape.cpp index fb28a33e..a58c25d4 100644 --- a/cpp/devices/tape.cpp +++ b/cpp/devices/tape.cpp @@ -537,7 +537,7 @@ bool Tape::Locate(bool locate_16) throw ScsiException(SenseKey::ILLEGAL_REQUEST, Asc::INVALID_FIELD_IN_CDB); } - auto identifier = static_cast(locate_16 ? GetCdbInt64(4) : GetCdbInt32(3)); + auto identifier = locate_16 ? GetCdbInt64(4) : GetCdbInt32(3); const bool bt = GetCdbByte(1) & 0x04; if (tar_file) { @@ -561,7 +561,7 @@ bool Tape::Locate(bool locate_16) } else { ResetPositions(); if (identifier) { - return FindObject(identifier); + return FindObject(static_cast(identifier)); } } } diff --git a/cpp/s2pexec/s2pexec_core.cpp b/cpp/s2pexec/s2pexec_core.cpp index 6d174010..dc0e3fa3 100644 --- a/cpp/s2pexec/s2pexec_core.cpp +++ b/cpp/s2pexec/s2pexec_core.cpp @@ -330,9 +330,7 @@ bool S2pExec::ParseArguments(span args, bool in_process) if (buffer_size = ParseAsUnsignedInt(buf); buffer_size <= 0) { throw ParserException("Invalid receive buffer size: '" + buf + "'"); } - else { - buffer.resize(buffer_size); - } + buffer.resize(buffer_size); } return true; diff --git a/cpp/shared/s2p_util.cpp b/cpp/shared/s2p_util.cpp index 40157b52..6bfc89d5 100644 --- a/cpp/shared/s2p_util.cpp +++ b/cpp/shared/s2p_util.cpp @@ -176,12 +176,10 @@ string s2p_util::ParseIdAndLun(const string &id_spec, int &id, int &lun) } if (const auto &components = Split(id_spec, COMPONENT_SEPARATOR, 2); !components.empty()) { - if (components.size() >= 1) { - id = ParseAsUnsignedInt(components[0]); - if (id == -1 || id > 7) { - id = -1; - return "Invalid device ID: '" + components[0] + "' (0-7)"; - } + id = ParseAsUnsignedInt(components[0]); + if (id == -1 || id > 7) { + id = -1; + return "Invalid device ID: '" + components[0] + "' (0-7)"; } if (components.size() >= 2) { diff --git a/cpp/test/primary_device_test.cpp b/cpp/test/primary_device_test.cpp index d1fe1de9..4f8ee768 100644 --- a/cpp/test/primary_device_test.cpp +++ b/cpp/test/primary_device_test.cpp @@ -474,10 +474,8 @@ TEST(PrimaryDeviceTest, ReportLuns) TEST(PrimaryDeviceTest, Dispatch) { - auto [__, device] = CreatePrimaryDevice(); - - Dispatch(device, static_cast(0x1f), SenseKey::ILLEGAL_REQUEST, Asc::INVALID_COMMAND_OPERATION_CODE, - "Unsupported SCSI command"); + Dispatch(make_shared(0), static_cast(0x1f), SenseKey::ILLEGAL_REQUEST, + Asc::INVALID_COMMAND_OPERATION_CODE, "Unsupported SCSI command"); } TEST(PrimaryDeviceTest, Init) diff --git a/cpp/test/tape_test.cpp b/cpp/test/tape_test.cpp index 97e2f30b..eaa37796 100644 --- a/cpp/test/tape_test.cpp +++ b/cpp/test/tape_test.cpp @@ -530,7 +530,7 @@ TEST(TapeTest, Erase6_simh) TEST(TapeTest, Erase6_tar) { - auto [__, tape] = CreateTape(); + auto [controller, tape] = CreateTape(); CreateImageFile(*tape, 512, "tar"); Dispatch(tape, ScsiCommand::ERASE_6, SenseKey::ILLEGAL_REQUEST, @@ -765,7 +765,7 @@ TEST(TapeTest, Space6_simh) TEST(TapeTest, Space6_tar) { - auto [___, tape] = CreateTape(); + auto [controller, tape] = CreateTape(); CreateImageFile(*tape, 512, "tar"); Dispatch(tape, ScsiCommand::SPACE_6, SenseKey::ILLEGAL_REQUEST,