From 304511d758fb81815eae0d60ad525956bc005289 Mon Sep 17 00:00:00 2001 From: Uwe Seimet Date: Wed, 5 Jun 2024 09:17:05 +0200 Subject: [PATCH] Release 3.3 --- .github/workflows/analyze.yml | 2 +- cpp/Makefile | 116 +++++++++------ cpp/base/device.h | 10 +- cpp/base/device_factory.h | 2 +- cpp/base/memory_util.cpp | 26 +++- cpp/base/memory_util.h | 21 +-- cpp/base/primary_device.cpp | 22 +++ cpp/base/primary_device.h | 22 +-- cpp/base/property_handler.cpp | 2 +- cpp/base/property_handler.h | 2 +- cpp/buses/bus.cpp | 7 +- cpp/buses/rpi_bus.cpp | 4 +- cpp/buses/rpi_bus.h | 6 +- cpp/command/command_executor.cpp | 39 +++-- cpp/command/command_executor.h | 6 +- cpp/command/command_image_support.cpp | 23 +-- cpp/command/command_image_support.h | 5 +- cpp/command/command_localizer.cpp | 3 +- cpp/controllers/abstract_controller.cpp | 2 +- cpp/controllers/abstract_controller.h | 3 +- cpp/controllers/controller.cpp | 183 ++++++++++++------------ cpp/controllers/controller.h | 5 +- cpp/controllers/controller_factory.cpp | 1 - cpp/controllers/phase_handler.cpp | 23 ++- cpp/controllers/phase_handler.h | 14 +- cpp/devices/disk_cache.cpp | 14 +- cpp/devices/disk_cache.h | 39 +++-- cpp/devices/host_services.cpp | 17 ++- cpp/devices/optical_memory.h | 6 - cpp/devices/printer.cpp | 10 +- cpp/devices/sasi_hd.h | 1 - cpp/devices/scsi_hd.h | 3 - cpp/devices/storage_device.cpp | 2 +- cpp/s2p/s2p_core.cpp | 8 +- cpp/s2p/s2p_core.h | 2 +- cpp/s2p/s2p_parser.cpp | 2 +- cpp/s2p/s2p_thread.cpp | 5 + cpp/s2p/s2p_thread.h | 5 +- cpp/s2pctl/s2pctl_display.cpp | 2 +- cpp/shared/s2p_util.cpp | 2 +- cpp/shared/s2p_util.h | 8 +- cpp/shared/s2p_version.cpp | 2 +- cpp/shared/scsi.h | 4 +- cpp/test/command_context_test.cpp | 10 +- cpp/test/device_factory_test.cpp | 32 +---- cpp/test/device_test.cpp | 27 ---- cpp/test/storage_device_test.cpp | 2 +- cpp/test/test_shared.cpp | 6 +- cpp/test/test_shared.h | 2 +- 49 files changed, 371 insertions(+), 389 deletions(-) diff --git a/.github/workflows/analyze.yml b/.github/workflows/analyze.yml index affd2279..8573010c 100644 --- a/.github/workflows/analyze.yml +++ b/.github/workflows/analyze.yml @@ -23,7 +23,7 @@ jobs: SCAN_HOST: ${{ secrets.SCAN_HOST }} GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} - SONAR_SCANNER_VERSION: 5.0.1.3006 + SONAR_SCANNER_VERSION: 6.0.0.4432 BUILD_WRAPPER_OUT_DIR: "$HOME/.build_wrapper_out" APT_PACKAGES: protobuf-compiler libspdlog-dev libpcap-dev libgmock-dev diff --git a/cpp/Makefile b/cpp/Makefile index 1abc580e..c8c3cb5d 100644 --- a/cpp/Makefile +++ b/cpp/Makefile @@ -43,7 +43,7 @@ CXXFLAGS += $(EXTRA_FLAGS) ## Installation path ## The path to install binaries and manpages in, default is /opt/scsi2pi -TARGET=/opt/scsi2pi +TARGET := /opt/scsi2pi ## BOARD=FULLSPEC ## Specifies the board type that you are using. Typically this is @@ -52,55 +52,91 @@ BOARD ?= FULLSPEC CXXFLAGS += -DBOARD_$(BOARD) -## BUILD_*=1 -## Specifies the device types to build, default is all. Supported types: -## SCHD SCSI Removable and non-removable Hard Drive +## ENABLE_*=1 +## DISABLE_*=1 +## Specifies the device types to build or not to build, default is to build all. Supported types: +## SCHD SCSI Removable and Non-removable Hard Drive ## SCCD SCSI CD-Rom Drive ## SCMO SCSI Optical Memory ## SCDP DaynaPort Network Adapter ## SCLP SCSI Printer ## SCHS Host Services ## SAHD SASI Hard Drive -ifeq ($(or $(BUILD_SCHD),$(BUILD_SCMO),$(BUILD_SCCD),$(BUILD_SCDP),$(BUILD_SCLP),$(BUILD_SCHS),$(BUILD_SAHD)),) - BUILD_SCHD = 1 - BUILD_SCMO = 1 - BUILD_SCCD = 1 - BUILD_SCDP = 1 - BUILD_SCLP = 1 - BUILD_SCHS = 1 - BUILD_SAHD = 1 +ifeq ($(or $(ENABLE_SCHD),$(ENABLE_SCMO),$(ENABLE_SCCD),$(ENABLE_SCDP),$(ENABLE_SCLP),$(ENABLE_SCHS),$(ENABLE_SAHD)),) + ENABLE_SCHD = 1 + ENABLE_SCMO = 1 + ENABLE_SCCD = 1 + ENABLE_SCDP = 1 + ENABLE_SCLP = 1 + ENABLE_SCHS = 1 + ENABLE_SAHD = 1 endif -ifdef BUILD_SCHD - CXXFLAGS += -DBUILD_MODE_PAGE_DEVICE -DBUILD_DISK -DBUILD_SCHD + +ifdef ENABLE_SCRM + ENABLE_SCHD = 1 +endif + +ifdef DISABLE_SCHD + ENABLE_SCHD = +endif +ifdef DISABLE_SCMO + ENABLE_SCMO = endif -ifdef BUILD_SCMO - CXXFLAGS += -DBUILD_MODE_PAGE_DEVICE -DBUILD_DISK -DBUILD_SCMO +ifdef DISABLE_SCCD + ENABLE_SCCD = endif -ifdef BUILD_SCCD - CXXFLAGS += -DBUILD_MODE_PAGE_DEVICE -DBUILD_DISK -DBUILD_SCCD +ifdef DISABLE_SCDP + ENABLE_SCDP = endif -ifdef BUILD_SCDP +ifdef DISABLE_SCLP + ENABLE_SCLP = +endif +ifdef DISABLE_SCHS + ENABLE_SCHS = +endif +ifdef DISABLE_SAHD + ENABLE_SAHD = +endif + +ifdef ENABLE_SCHD + CXXFLAGS += -DBUILD_SCHD +endif +ifdef ENABLE_SCMO + CXXFLAGS += -DBUILD_SCMO +endif +ifdef ENABLE_SCCD + CXXFLAGS += -DBUILD_SCCD +endif +ifdef ENABLE_SCDP CXXFLAGS += -DBUILD_SCDP endif -ifdef BUILD_SCLP +ifdef ENABLE_SCLP CXXFLAGS += -DBUILD_SCLP endif -ifdef BUILD_SCHS - CXXFLAGS += -DBUILD_MODE_PAGE_DEVICE -DBUILD_SCHS +ifdef ENABLE_SCHS + CXXFLAGS += -DBUILD_SCHS +endif +ifdef ENABLE_SAHD + CXXFLAGS += -DBUILD_SAHD endif -ifdef BUILD_SAHD - CXXFLAGS += -DBUILD_DISK -DBUILD_SAHD + +ifeq ($(or $(ENABLE_SCHD),$(ENABLE_SCMO),$(ENABLE_SCCD),$(ENABLE_SAHD),$(ENABLE_SCHS)),1) + CXXFLAGS += -DBUILD_MODE_PAGE_DEVICE +endif + +ifeq ($(or $(ENABLE_SCHD),$(ENABLE_SCMO),$(ENABLE_SCCD),$(ENABLE_SAHD)),1) + CXXFLAGS += -DBUILD_DISK endif -S2P = s2p -S2PCTL = s2pctl -S2PDUMP = s2pdump -S2PEXEC = s2pexec -S2PPROTO = s2pproto -S2P_TEST = s2p_test -IN_PROCESS_TEST = in_process_test +S2P := s2p +S2PCTL := s2pctl +S2PDUMP := s2pdump +S2PEXEC := s2pexec +S2PPROTO := s2pproto +S2P_TEST := s2p_test +IN_PROCESS_TEST := in_process_test -INSTALL_BIN = $(TARGET)/bin +INSTALL_BIN := $(TARGET)/bin OBJDIR := obj LIBDIR := lib @@ -126,7 +162,7 @@ DIR_BUSES := buses DIR_CONTROLLERS := controllers DIR_DEVICES := devices -SRC_PROTOC = ../proto/s2p_interface.proto +SRC_PROTOC := ../proto/s2p_interface.proto SRC_GENERATED = $(GENERATED_DIR)/s2p_interface.pb.cpp @@ -146,42 +182,42 @@ SRC_DISK = \ $(DIR_DEVICES)/storage_device.cpp \ $(DIR_DEVICES)/mode_page_device.cpp -ifdef BUILD_SCHD +ifeq ($(ENABLE_SCHD),1) SRC_SCHD = \ $(DIR_DEVICES)/scsi_hd.cpp SRC_SCHD += $(SRC_DISK) endif -ifdef BUILD_SCMO +ifeq ($(ENABLE_SCMO),1) SRC_SCMO = \ $(DIR_DEVICES)/optical_memory.cpp SRC_SCMO += $(SRC_DISK) endif -ifdef BUILD_SCCD +ifeq ($(ENABLE_SCCD),1) SRC_SCCD = \ $(DIR_DEVICES)/scsi_cd.cpp SRC_SCCD += $(SRC_DISK) endif -ifdef BUILD_SCDP +ifeq ($(ENABLE_SCDP),1) SRC_SCDP = \ $(DIR_DEVICES)/daynaport.cpp \ $(DIR_DEVICES)/tap_driver.cpp endif -ifdef BUILD_SCLP +ifeq ($(ENABLE_SCLP),1) SRC_SCLP = \ $(DIR_DEVICES)/printer.cpp endif -ifdef BUILD_SCHS +ifeq ($(ENABLE_SCHS),1) SRC_SCHS = \ $(DIR_DEVICES)/host_services.cpp \ $(DIR_DEVICES)/mode_page_device.cpp endif -ifdef BUILD_SAHD +ifeq ($(ENABLE_SAHD),1) SRC_SAHD = \ $(DIR_DEVICES)/sasi_hd.cpp SRC_SAHD += $(SRC_DISK) diff --git a/cpp/base/device.h b/cpp/base/device.h index e04e82b2..cecc72dd 100644 --- a/cpp/base/device.h +++ b/cpp/base/device.h @@ -27,10 +27,6 @@ class Device // NOSONAR The number of fields and methods is justified, the compl { return type; } - string GetTypeString() const - { - return PbDeviceType_Name(type); - } bool IsReady() const { @@ -99,17 +95,17 @@ class Device // NOSONAR The number of fields and methods is justified, the compl return lun; } - string GetVendor() const + const string& GetVendor() const { return vendor; } void SetVendor(const string&); - string GetProduct() const + const string& GetProduct() const { return product; } void SetProduct(const string&, bool = true); - string GetRevision() const + const string& GetRevision() const { return revision; } diff --git a/cpp/base/device_factory.h b/cpp/base/device_factory.h index 56e832ef..cb2f7181 100644 --- a/cpp/base/device_factory.h +++ b/cpp/base/device_factory.h @@ -47,7 +47,7 @@ class DeviceFactory static string GetExtensionLowerCase(string_view); - inline static const unordered_map> DEVICE_MAPPING = { + inline static const unordered_map DEVICE_MAPPING = { { "daynaport", SCDP }, { "printer", SCLP }, { "services", SCHS } diff --git a/cpp/base/memory_util.cpp b/cpp/base/memory_util.cpp index c569c0a7..ad44471b 100644 --- a/cpp/base/memory_util.cpp +++ b/cpp/base/memory_util.cpp @@ -2,12 +2,36 @@ // // SCSI device emulator and SCSI tools for the Raspberry Pi // -// Copyright (C) 2022-2023 Uwe Seimet +// Copyright (C) 2022-2024 Uwe Seimet // //--------------------------------------------------------------------------- #include "memory_util.h" +template +void memory_util::SetInt16(vector &buf, int offset, int value) +{ + assert(buf.size() > static_cast(offset) + 1); + + buf[offset] = static_cast(value >> 8); + buf[offset + 1] = static_cast(value); +} +template void memory_util::SetInt16(vector&, int, int); +template void memory_util::SetInt16(vector&, int, int); + +template +void memory_util::SetInt32(vector &buf, int offset, uint32_t value) +{ + assert(buf.size() > static_cast(offset) + 3); + + buf[offset] = static_cast(value >> 24); + buf[offset + 1] = static_cast(value >> 16); + buf[offset + 2] = static_cast(value >> 8); + buf[offset + 3] = static_cast(value); +} +template void memory_util::SetInt32(vector&, int, uint32_t); +template void memory_util::SetInt32(vector&, int, uint32_t); + int memory_util::GetInt24(span buf, int offset) { assert(buf.size() > static_cast(offset) + 2); diff --git a/cpp/base/memory_util.h b/cpp/base/memory_util.h index 14f70682..3ef82057 100644 --- a/cpp/base/memory_util.h +++ b/cpp/base/memory_util.h @@ -24,25 +24,8 @@ int GetInt16(const auto &buf, int offset) return (static_cast(buf[offset]) << 8) | buf[offset + 1]; } -template -void SetInt16(vector &buf, int offset, int value) -{ - assert(buf.size() > static_cast(offset) + 1); - - buf[offset] = static_cast(value >> 8); - buf[offset + 1] = static_cast(value); -} - -template -void SetInt32(vector &buf, int offset, uint32_t value) -{ - assert(buf.size() > static_cast(offset) + 3); - - buf[offset] = static_cast(value >> 24); - buf[offset + 1] = static_cast(value >> 16); - buf[offset + 2] = static_cast(value >> 8); - buf[offset + 3] = static_cast(value); -} +template void SetInt16(vector&, int, int); +template void SetInt32(vector&, int, uint32_t); int GetInt24(span, int); uint32_t GetInt32(span, int); diff --git a/cpp/base/primary_device.cpp b/cpp/base/primary_device.cpp index d5812cba..75d6723d 100644 --- a/cpp/base/primary_device.cpp +++ b/cpp/base/primary_device.cpp @@ -53,6 +53,11 @@ bool PrimaryDevice::Init(const param_map ¶ms) return true; } +void PrimaryDevice::AddCommand(scsi_command cmd, const command &c) +{ + commands[static_cast(cmd)] = c; +} + void PrimaryDevice::Dispatch(scsi_command cmd) { if (const auto &command = commands[static_cast(cmd)]; command) { @@ -96,6 +101,23 @@ void PrimaryDevice::SetController(AbstractController *c) device_logger.SetIdAndLun(GetId(), GetLun()); } +void PrimaryDevice::StatusPhase() const +{ + controller->Status(); +} + +void PrimaryDevice::DataInPhase(int length) const +{ + controller->SetCurrentLength(length); + controller->DataIn(); +} + +void PrimaryDevice::DataOutPhase(int length) const +{ + controller->SetCurrentLength(length); + controller->DataOut(); +} + void PrimaryDevice::TestUnitReady() { CheckReady(); diff --git a/cpp/base/primary_device.h b/cpp/base/primary_device.h index 5546c601..f3b76f83 100644 --- a/cpp/base/primary_device.h +++ b/cpp/base/primary_device.h @@ -105,10 +105,7 @@ class PrimaryDevice : public ScsiPrimaryCommands, public Device { } - void AddCommand(scsi_command cmd, const command &c) - { - commands[static_cast(cmd)] = c; - } + void AddCommand(scsi_command, const command&); vector HandleInquiry(device_type, bool) const; virtual vector InquiryInternal() const = 0; @@ -121,20 +118,9 @@ class PrimaryDevice : public ScsiPrimaryCommands, public Device void ReserveUnit() override; void ReleaseUnit() override; - void StatusPhase() const - { - controller->Status(); - } - void DataInPhase(int length) const - { - controller->SetCurrentLength(length); - controller->DataIn(); - } - void DataOutPhase(int length) const - { - controller->SetCurrentLength(length); - controller->DataOut(); - } + void StatusPhase() const; + void DataInPhase(int) const; + void DataOutPhase(int) const; void LogTrace(const string &s) const { diff --git a/cpp/base/property_handler.cpp b/cpp/base/property_handler.cpp index ad881ecf..6e37f6e1 100644 --- a/cpp/base/property_handler.cpp +++ b/cpp/base/property_handler.cpp @@ -98,7 +98,7 @@ property_map PropertyHandler::GetProperties(const string &filter) const return filtered_properties; } -string PropertyHandler::GetProperty(string_view key, const string &def) const +const string& PropertyHandler::GetProperty(string_view key, const string &def) const { for (const auto& [k, v] : property_cache) { if (k == key) { diff --git a/cpp/base/property_handler.h b/cpp/base/property_handler.h index c07dc27f..ff2563f7 100644 --- a/cpp/base/property_handler.h +++ b/cpp/base/property_handler.h @@ -51,7 +51,7 @@ class PropertyHandler void Init(const string&, const property_map&, bool); property_map GetProperties(const string& = "") const; - string GetProperty(string_view, const string& = "") const; + const string& GetProperty(string_view, const string& = "") const; void AddProperty(const string &key, string_view value) { property_cache[key] = value; diff --git a/cpp/buses/bus.cpp b/cpp/buses/bus.cpp index c92f2fbe..0191ec7b 100644 --- a/cpp/buses/bus.cpp +++ b/cpp/buses/bus.cpp @@ -39,7 +39,7 @@ int Bus::CommandHandShake(vector &buf) // Timeout waiting for ACK to change if (!ack || !WaitSignal(PIN_ACK, false)) { EnableIRQ(); - return 0; + return -1; } // The ICD AdSCSI ST, AdSCSI Plus ST and AdSCSI Micro ST host adapters allow SCSI devices to be connected @@ -65,7 +65,7 @@ int Bus::CommandHandShake(vector &buf) // Timeout waiting for ACK to change if (!ack || !WaitSignal(PIN_ACK, false)) { EnableIRQ(); - return 0; + return -1; } } @@ -95,7 +95,8 @@ int Bus::CommandHandShake(vector &buf) // Timeout waiting for ACK to change if (!ack || !WaitSignal(PIN_ACK, false)) { - break; + EnableIRQ(); + return -1; } } diff --git a/cpp/buses/rpi_bus.cpp b/cpp/buses/rpi_bus.cpp index 19865aea..f50faad3 100644 --- a/cpp/buses/rpi_bus.cpp +++ b/cpp/buses/rpi_bus.cpp @@ -44,8 +44,8 @@ bool RpiBus::Init(bool target) case RpiBus::PiType::pi_5: base_addr = 0x1f00000000; - gpio_offset = GPIO_OFFSET_PI5; - pads_offset = PADS_OFFSET_PI5; + gpio_offset = GPIO_OFFSET_RP1; + pads_offset = PADS_OFFSET_RP1; break; default: diff --git a/cpp/buses/rpi_bus.h b/cpp/buses/rpi_bus.h index 6de61700..8550625b 100644 --- a/cpp/buses/rpi_bus.h +++ b/cpp/buses/rpi_bus.h @@ -178,10 +178,10 @@ class RpiBus final : public Bus constexpr static uint32_t IRPT_OFFSET = 0x0000B200; constexpr static uint32_t PADS_OFFSET = 0x00100000; - constexpr static uint32_t PADS_OFFSET_PI5 = 0x000f0000; + constexpr static uint32_t PADS_OFFSET_RP1 = 0x000f0000; constexpr static uint32_t GPIO_OFFSET = 0x00200000; - constexpr static uint32_t GPIO_OFFSET_PI5 = 0x000d0000; - constexpr static uint32_t RIO_OFFSET_PI5 = 0x000e0000; + constexpr static uint32_t GPIO_OFFSET_RP1 = 0x000d0000; + constexpr static uint32_t RIO_OFFSET_RP1 = 0x000e0000; constexpr static uint32_t QA7_OFFSET = 0x01000000; constexpr static uint32_t PI4_ARM_GICC_CTLR = 0xFF842000; diff --git a/cpp/command/command_executor.cpp b/cpp/command/command_executor.cpp index 3e2e3ce2..537cbad9 100644 --- a/cpp/command/command_executor.cpp +++ b/cpp/command/command_executor.cpp @@ -242,7 +242,7 @@ bool CommandExecutor::Attach(const CommandContext &context, const PbDeviceDefini if (!device->IsRemovable() && filename.empty()) { // GetIdentifier() cannot be used here because the device ID has not yet been set return context.ReturnLocalizedError(LocalizationKey::ERROR_DEVICE_MISSING_FILENAME, - fmt::format("{0} {1}:{2}", device->GetTypeString(), id, lun)); + fmt::format("{0} {1}:{2}", GetTypeString(*device), id, lun)); } if (!ValidateImageFile(context, *storage_device, filename)) { @@ -270,7 +270,7 @@ bool CommandExecutor::Attach(const CommandContext &context, const PbDeviceDefini if (!device->Init(params)) { return context.ReturnLocalizedError(LocalizationKey::ERROR_INITIALIZATION, - fmt::format("{0} {1}:{2}", device->GetTypeString(), id, lun)); + fmt::format("{0} {1}:{2}", GetTypeString(*device), id, lun)); } if (!controller_factory.AttachToController(bus, id, device)) { @@ -395,9 +395,10 @@ void CommandExecutor::DetachAll() const void CommandExecutor::SetUpDeviceProperties(shared_ptr device) { const string &identifier = fmt::format("device.{0}:{1}.", device->GetId(), device->GetLun()); - PropertyHandler::Instance().AddProperty(identifier + "type", device->GetTypeString()); + PropertyHandler::Instance().AddProperty(identifier + "type", GetTypeString(*device)); PropertyHandler::Instance().AddProperty(identifier + "name", device->GetVendor() + ":" + device->GetProduct() + ":" + device->GetRevision()); +#ifdef BUILD_DISK const auto disk = dynamic_pointer_cast(device); if (disk && disk->GetConfiguredSectorSize()) { PropertyHandler::Instance().AddProperty(identifier + "block_size", to_string(disk->GetConfiguredSectorSize())); @@ -409,8 +410,11 @@ void CommandExecutor::SetUpDeviceProperties(shared_ptr device) filename = filename.substr(CommandImageSupport::Instance().GetDefaultFolder().length() + 1); } PropertyHandler::Instance().AddProperty(identifier + "params", filename); + return; } - else if (!device->GetParams().empty()) { +#endif + + if (!device->GetParams().empty()) { vector p; for (const auto& [param, value] : device->GetParams()) { p.emplace_back(param + "=" + value); @@ -518,7 +522,7 @@ bool CommandExecutor::CheckForReservedFile(const CommandContext &context, const string CommandExecutor::PrintCommand(const PbCommand &command, const PbDeviceDefinition &pb_device) { - const map> params = { command.params().cbegin(), command.params().cend() }; + const map> ¶ms = { command.params().cbegin(), command.params().cend() }; ostringstream s; s << "operation=" << PbOperation_Name(command.operation()); @@ -615,7 +619,7 @@ shared_ptr CommandExecutor::CreateDevice(const CommandContext &co if (UNIQUE_DEVICE_TYPES.contains(device->GetType())) { for (const auto &d : GetAllDevices()) { if (d->GetType() == device->GetType()) { - context.ReturnLocalizedError(LocalizationKey::ERROR_UNIQUE_DEVICE_TYPE, device->GetTypeString()); + context.ReturnLocalizedError(LocalizationKey::ERROR_UNIQUE_DEVICE_TYPE, GetTypeString(*device)); return nullptr; } } @@ -648,7 +652,7 @@ bool CommandExecutor::SetSectorSize(const CommandContext &context, shared_ptrGetTypeString()); + GetTypeString(*device)); } } #endif @@ -663,25 +667,22 @@ bool CommandExecutor::ValidateOperation(const CommandContext &context, const Pri if ((operation == START || operation == STOP) && !device.IsStoppable()) { return context.ReturnLocalizedError(LocalizationKey::ERROR_OPERATION_DENIED_STOPPABLE, - PbOperation_Name(operation), - device.GetTypeString()); + PbOperation_Name(operation), GetTypeString(device)); } if ((operation == INSERT || operation == EJECT) && !device.IsRemovable()) { return context.ReturnLocalizedError(LocalizationKey::ERROR_OPERATION_DENIED_REMOVABLE, - PbOperation_Name(operation), - device.GetTypeString()); + PbOperation_Name(operation), GetTypeString(device)); } if ((operation == PROTECT || operation == UNPROTECT) && !device.IsProtectable()) { return context.ReturnLocalizedError(LocalizationKey::ERROR_OPERATION_DENIED_PROTECTABLE, - PbOperation_Name(operation), - device.GetTypeString()); + PbOperation_Name(operation), GetTypeString(device)); } if ((operation == PROTECT || operation == UNPROTECT) && !device.IsReady()) { return context.ReturnLocalizedError(LocalizationKey::ERROR_OPERATION_DENIED_READY, PbOperation_Name(operation), - device.GetTypeString()); + GetTypeString(device)); } return true; @@ -738,3 +739,13 @@ bool CommandExecutor::SetProductData(const CommandContext &context, const PbDevi return true; } + +string CommandExecutor::GetTypeString(const Device &device) +{ + return PbDeviceType_Name(device.GetType()); +} + +string CommandExecutor::GetIdentifier(const Device &device) +{ + return GetTypeString(device) + " " + to_string(device.GetId()) + ":" + to_string(device.GetLun()); +} diff --git a/cpp/command/command_executor.h b/cpp/command/command_executor.h index a08c839d..26c921ba 100644 --- a/cpp/command/command_executor.h +++ b/cpp/command/command_executor.h @@ -67,10 +67,8 @@ class CommandExecutor private: - static string GetIdentifier(const Device &device) - { - return device.GetTypeString() + " " + to_string(device.GetId()) + ":" + to_string(device.GetLun()); - } + static string GetTypeString(const Device&); + static string GetIdentifier(const Device&); static void DisplayDeviceInfo(const PrimaryDevice&); static bool CheckForReservedFile(const CommandContext&, const string&); diff --git a/cpp/command/command_image_support.cpp b/cpp/command/command_image_support.cpp index 1413ee71..c4ed9838 100644 --- a/cpp/command/command_image_support.cpp +++ b/cpp/command/command_image_support.cpp @@ -31,6 +31,11 @@ bool CommandImageSupport::CheckDepth(string_view filename) const return ranges::count(filename, '/') <= depth; } +string CommandImageSupport::GetFullName(const string &filename) const +{ + return default_folder + "/" + filename; +} + bool CommandImageSupport::CreateImageFolder(const CommandContext &context, string_view filename) const { if (const auto folder = path(filename).parent_path(); !folder.string().empty()) { @@ -86,21 +91,21 @@ string CommandImageSupport::SetDefaultFolder(string_view f) bool CommandImageSupport::CreateImage(const CommandContext &context) const { - const string filename = GetParam(context.GetCommand(), "file"); + const string &filename = GetParam(context.GetCommand(), "file"); if (filename.empty()) { return context.ReturnErrorStatus("Missing image filename"); } if (!CheckDepth(filename)) { - return context.ReturnErrorStatus(("Invalid folder hierarchy depth '" + filename + "'").c_str()); + return context.ReturnErrorStatus(("Invalid folder hierarchy depth '" + filename + "'")); } - const string full_filename = GetFullName(filename); + const string &full_filename = GetFullName(filename); if (!IsValidDstFilename(full_filename)) { return context.ReturnErrorStatus("Can't create image file: '" + full_filename + "': File already exists"); } - const string size = GetParam(context.GetCommand(), "size"); + const string &size = GetParam(context.GetCommand(), "size"); if (size.empty()) { return context.ReturnErrorStatus("Can't create image file '" + full_filename + "': Missing file size"); } @@ -151,7 +156,7 @@ bool CommandImageSupport::CreateImage(const CommandContext &context) const bool CommandImageSupport::DeleteImage(const CommandContext &context) const { - const string filename = GetParam(context.GetCommand(), "file"); + const string &filename = GetParam(context.GetCommand(), "file"); if (filename.empty()) { return context.ReturnErrorStatus("Missing image filename"); } @@ -176,8 +181,8 @@ bool CommandImageSupport::DeleteImage(const CommandContext &context) const // Delete empty subfolders size_t last_slash = filename.rfind('/'); while (last_slash != string::npos) { - const string folder = filename.substr(0, last_slash); - const auto full_folder = path(GetFullName(folder)); + const string &folder = filename.substr(0, last_slash); + const auto &full_folder = path(GetFullName(folder)); if (error_code error; !filesystem::is_empty(full_folder, error) || error) { break; @@ -260,7 +265,7 @@ bool CommandImageSupport::CopyImage(const CommandContext &context) const bool CommandImageSupport::SetImagePermissions(const CommandContext &context) const { - const string filename = GetParam(context.GetCommand(), "file"); + const string &filename = GetParam(context.GetCommand(), "file"); if (filename.empty()) { return context.ReturnErrorStatus("Missing image filename"); } @@ -269,7 +274,7 @@ bool CommandImageSupport::SetImagePermissions(const CommandContext &context) con return context.ReturnErrorStatus("Invalid folder hierarchy depth '" + filename + "'"); } - const string full_filename = GetFullName(filename); + const string &full_filename = GetFullName(filename); if (!IsValidSrcFilename(full_filename)) { return context.ReturnErrorStatus("Can't modify image file '" + full_filename + "': Invalid name or type"); } diff --git a/cpp/command/command_image_support.h b/cpp/command/command_image_support.h index 94ce4e2d..00f6c0e4 100644 --- a/cpp/command/command_image_support.h +++ b/cpp/command/command_image_support.h @@ -53,10 +53,7 @@ class CommandImageSupport CommandImageSupport operator&(const CommandImageSupport&) = delete; bool CheckDepth(string_view) const; - string GetFullName(const string &filename) const - { - return default_folder + "/" + filename; - } + string GetFullName(const string&) const; bool CreateImageFolder(const CommandContext&, string_view) const; bool ValidateParams(const CommandContext&, const string&, string&, string&) const; diff --git a/cpp/command/command_localizer.cpp b/cpp/command/command_localizer.cpp index 55996bc8..5613f6db 100644 --- a/cpp/command/command_localizer.cpp +++ b/cpp/command/command_localizer.cpp @@ -281,8 +281,7 @@ void CommandLocalizer::Add(LocalizationKey key, const string &locale, string_vie assert(!locale.empty()); assert(!value.empty()); assert(!localized_messages[locale].contains(key)); - assert((unordered_set>( { "en", "de", "sv", "fr", "es", "zh" })).contains( - locale)); + assert((unordered_set( { "en", "de", "sv", "fr", "es", "zh" })).contains(locale)); localized_messages[locale][key] = value; } diff --git a/cpp/controllers/abstract_controller.cpp b/cpp/controllers/abstract_controller.cpp index 1c2f3b94..d78038af 100644 --- a/cpp/controllers/abstract_controller.cpp +++ b/cpp/controllers/abstract_controller.cpp @@ -89,7 +89,7 @@ shutdown_mode AbstractController::ProcessOnController(int ids) { device_logger.SetIdAndLun(target_id, -1); - if (int ids_without_target = ids - (1 << target_id); ids_without_target) { + if (const int ids_without_target = ids - (1 << target_id); ids_without_target) { initiator_id = 0; while (!(ids_without_target & (1 << initiator_id))) { ++initiator_id; diff --git a/cpp/controllers/abstract_controller.h b/cpp/controllers/abstract_controller.h index 3b1b6e05..f0808dd1 100644 --- a/cpp/controllers/abstract_controller.h +++ b/cpp/controllers/abstract_controller.h @@ -25,8 +25,7 @@ class AbstractController : public PhaseHandler AbstractController(Bus&, int); ~AbstractController() override = default; - virtual void Error(sense_key, asc = asc::no_additional_sense_information, - status_code = status_code::check_condition) = 0; + virtual void Error(sense_key, asc, status_code) = 0; virtual int GetEffectiveLun() const = 0; diff --git a/cpp/controllers/controller.cpp b/cpp/controllers/controller.cpp index 1044498c..1caf0017 100644 --- a/cpp/controllers/controller.cpp +++ b/cpp/controllers/controller.cpp @@ -12,6 +12,8 @@ #include "buses/bus_factory.h" #ifdef BUILD_MODE_PAGE_DEVICE #include "devices/mode_page_device.h" +#else +#include "base/primary_device.h" #endif #include "shared/s2p_exceptions.h" @@ -104,9 +106,14 @@ void Controller::Command() auto &buf = GetBuffer(); const int actual_count = GetBus().CommandHandShake(buf); - if (!actual_count) { - LogTrace(fmt::format("Received unknown command: ${:02x}", buf[0])); - Error(sense_key::illegal_request, asc::invalid_command_operation_code); + if (actual_count <= 0) { + if (!actual_count) { + LogTrace(fmt::format("Received unknown command: ${:02x}", buf[0])); + Error(sense_key::illegal_request, asc::invalid_command_operation_code); + } + else { + Error(sense_key::aborted_command, asc::command_phase_error); + } return; } @@ -170,116 +177,111 @@ void Controller::Execute() void Controller::Status() { - if (!IsStatus()) { - LogTrace(fmt::format("Status phase, status is {0} (status code ${1:02x})", STATUS_MAPPING.at(GetStatus()), - static_cast(GetStatus()))); - - SetPhase(bus_phase::status); + if (IsStatus()) { + Send(); + return; + } - GetBus().SetMSG(false); - GetBus().SetCD(true); - GetBus().SetIO(true); + LogTrace(fmt::format("Status phase, status is {0} (status code ${1:02x})", STATUS_MAPPING.at(GetStatus()), + static_cast(GetStatus()))); - ResetOffset(); - SetCurrentLength(1); - SetTransferSize(1, 1); - GetBuffer()[0] = (uint8_t)GetStatus(); + SetPhase(bus_phase::status); - return; - } + GetBus().SetMSG(false); + GetBus().SetCD(true); + GetBus().SetIO(true); - Send(); + ResetOffset(); + SetCurrentLength(1); + SetTransferSize(1, 1); + GetBuffer()[0] = (uint8_t)GetStatus(); } void Controller::MsgIn() { - if (!IsMsgIn()) { - LogTrace("MESSAGE IN phase"); - SetPhase(bus_phase::msgin); - - GetBus().SetMSG(true); - GetBus().SetCD(true); - GetBus().SetIO(true); - - ResetOffset(); - + if (IsMsgIn()) { + Send(); return; } - Send(); + LogTrace("MESSAGE IN phase"); + SetPhase(bus_phase::msgin); + + GetBus().SetMSG(true); + GetBus().SetCD(true); + GetBus().SetIO(true); + + ResetOffset(); } void Controller::MsgOut() { - if (!IsMsgOut()) { - LogTrace("MESSAGE OUT phase"); - - // Process the IDENTIFY message - if (IsSelection()) { - atn_msg = true; - msg_bytes.clear(); - } + if (IsMsgOut()) { + Receive(); + return; + } - SetPhase(bus_phase::msgout); + LogTrace("MESSAGE OUT phase"); - GetBus().SetMSG(true); - GetBus().SetCD(true); - GetBus().SetIO(false); + // Process the IDENTIFY message + if (IsSelection()) { + atn_msg = true; + msg_bytes.clear(); + } - ResetOffset(); - SetCurrentLength(1); - SetTransferSize(1, 1); + SetPhase(bus_phase::msgout); - return; - } + GetBus().SetMSG(true); + GetBus().SetCD(true); + GetBus().SetIO(false); - Receive(); + ResetOffset(); + SetCurrentLength(1); + SetTransferSize(1, 1); } void Controller::DataIn() { - if (!IsDataIn()) { - if (!GetCurrentLength()) { - Status(); - return; - } - - LogTrace("DATA IN phase"); - SetPhase(bus_phase::datain); - - GetBus().SetMSG(false); - GetBus().SetCD(false); - GetBus().SetIO(true); - - ResetOffset(); + if (IsDataIn()) { + Send(); + return; + } + if (!GetCurrentLength()) { + Status(); return; } - Send(); + LogTrace("DATA IN phase"); + SetPhase(bus_phase::datain); + + GetBus().SetMSG(false); + GetBus().SetCD(false); + GetBus().SetIO(true); + + ResetOffset(); } void Controller::DataOut() { - if (!IsDataOut()) { - if (!GetCurrentLength()) { - Status(); - return; - } - - LogTrace("DATA OUT phase"); - SetPhase(bus_phase::dataout); - - GetBus().SetMSG(false); - GetBus().SetCD(false); - GetBus().SetIO(false); - - ResetOffset(); + if (IsDataOut()) { + Receive(); + return; + } + if (!GetCurrentLength()) { + Status(); return; } - Receive(); + LogTrace("DATA OUT phase"); + SetPhase(bus_phase::dataout); + + GetBus().SetMSG(false); + GetBus().SetCD(false); + GetBus().SetIO(false); + + ResetOffset(); } void Controller::Error(sense_key sense_key, asc asc, status_code status) @@ -419,17 +421,17 @@ void Controller::Receive() } switch (GetPhase()) { - case bus_phase::msgout: - ProcessMessage(); - break; - case bus_phase::dataout: // All data have been transferred Status(); break; + case bus_phase::msgout: + ProcessMessage(); + break; + default: - error("Unexpected bus phase: " + Bus::GetPhaseName(GetPhase())); + assert(false); break; } } @@ -438,7 +440,7 @@ void Controller::Receive() #pragma GCC diagnostic ignored "-Wunused-parameter" bool Controller::XferIn(vector &buf) { - // Limited to read commands + // Limited to read commands (DATA IN phase) switch (static_cast(GetCdbByte(0))) { case scsi_command::cmd_read6: case scsi_command::cmd_read10: @@ -462,7 +464,6 @@ bool Controller::XferIn(vector &buf) } Error(sense_key::aborted_command, asc::controller_xfer_in); - return false; } #pragma GCC diagnostic pop @@ -471,16 +472,16 @@ bool Controller::XferIn(vector &buf) #pragma GCC diagnostic ignored "-Wunused-parameter" bool Controller::XferOut(bool cont) { - auto device = GetDeviceForLun(GetEffectiveLun()); + const auto device = GetDeviceForLun(GetEffectiveLun()); - // Limited to write/verify commands + // Limited to write/verify commands (DATA OUT phase) switch (const auto opcode = static_cast(GetCdbByte(0)); opcode ) { case scsi_command::cmd_mode_select6: case scsi_command::cmd_mode_select10: { #ifdef BUILD_MODE_PAGE_DEVICE - auto mode_page_device = dynamic_pointer_cast(device); + const auto mode_page_device = dynamic_pointer_cast(device); assert(mode_page_device); if (!mode_page_device) { Error(sense_key::aborted_command, asc::controller_xfer_out); @@ -508,10 +509,8 @@ bool Controller::XferOut(bool cont) case scsi_command::cmd_write_long10: case scsi_command::cmd_write_long16: case scsi_command::cmd_execute_operation: - { try { const auto length = device->WriteData(GetBuffer(), opcode); - if (cont) { SetCurrentLength(length); ResetOffset(); @@ -521,10 +520,7 @@ bool Controller::XferOut(bool cont) Error(e.get_sense_key(), e.get_asc()); return false; } - return true; - } - break; default: assert(false); @@ -532,7 +528,6 @@ bool Controller::XferOut(bool cont) } Error(sense_key::aborted_command, asc::controller_xfer_out); - return false; } #pragma GCC diagnostic pop @@ -568,7 +563,7 @@ void Controller::ParseMessage() case 0x0c: { LogTrace("Received BUS DEVICE RESET message"); - if (auto device = GetDeviceForLun(GetEffectiveLun()); device) { + if (const auto device = GetDeviceForLun(GetEffectiveLun()); device) { device->SetReset(true); device->DiscardReservation(); } diff --git a/cpp/controllers/controller.h b/cpp/controllers/controller.h index e5e5cb7a..5826510a 100644 --- a/cpp/controllers/controller.h +++ b/cpp/controllers/controller.h @@ -16,12 +16,11 @@ class Controller : public AbstractController public: using AbstractController::AbstractController; - ~Controller() override = default; bool Process() override; - void Error(sense_key, asc asc = asc::no_additional_sense_information, - status_code status = status_code::check_condition) override; + void Error(sense_key, asc asc = asc::no_additional_sense_information, status_code status = + status_code::check_condition) override; void Reset() override; void BusFree() override; diff --git a/cpp/controllers/controller_factory.cpp b/cpp/controllers/controller_factory.cpp index 6871a714..bfe70c96 100644 --- a/cpp/controllers/controller_factory.cpp +++ b/cpp/controllers/controller_factory.cpp @@ -23,7 +23,6 @@ bool ControllerFactory::AttachToController(Bus &bus, int id, shared_ptr(bus, id); controller->AddDevice(device)) { controller->Init(); - assert(!controllers[id]); controllers[id] = controller; return true; diff --git a/cpp/controllers/phase_handler.cpp b/cpp/controllers/phase_handler.cpp index cf9b798a..fe6b61c5 100644 --- a/cpp/controllers/phase_handler.cpp +++ b/cpp/controllers/phase_handler.cpp @@ -7,19 +7,18 @@ //--------------------------------------------------------------------------- #include "phase_handler.h" -#include void PhaseHandler::Init() { - phase_executors[static_cast(bus_phase::busfree)] = [this]() {BusFree();}; - phase_executors[static_cast(bus_phase::arbitration)] = []() {throw invalid_argument("");}; - phase_executors[static_cast(bus_phase::selection)] = [this]() {Selection();}; - phase_executors[static_cast(bus_phase::reselection)] = []() {throw invalid_argument("");}; - phase_executors[static_cast(bus_phase::command)] = [this]() {Command();}; - phase_executors[static_cast(bus_phase::datain)] = [this]() {DataIn();}; - phase_executors[static_cast(bus_phase::dataout)] = [this]() {DataOut();}; - phase_executors[static_cast(bus_phase::status)] = [this]() {Status();}; - phase_executors[static_cast(bus_phase::msgin)] = [this]() {MsgIn();}; - phase_executors[static_cast(bus_phase::msgout)] = [this]() {MsgOut();}; - phase_executors[static_cast(bus_phase::reserved)] = []() {throw invalid_argument("");}; + phase_executors[static_cast(bus_phase::busfree)] = [this]() {BusFree(); return true;}; + phase_executors[static_cast(bus_phase::arbitration)] = []() {return false;}; + phase_executors[static_cast(bus_phase::selection)] = [this]() {Selection(); return true;}; + phase_executors[static_cast(bus_phase::reselection)] = []() {return false;}; + phase_executors[static_cast(bus_phase::command)] = [this]() {Command(); return true;}; + phase_executors[static_cast(bus_phase::datain)] = [this]() {DataIn(); return true;}; + phase_executors[static_cast(bus_phase::dataout)] = [this]() {DataOut(); return true;}; + phase_executors[static_cast(bus_phase::status)] = [this]() {Status(); return true;}; + phase_executors[static_cast(bus_phase::msgin)] = [this]() {MsgIn(); return true;}; + phase_executors[static_cast(bus_phase::msgout)] = [this]() {MsgOut(); return true;}; + phase_executors[static_cast(bus_phase::reserved)] = []() {return false;}; } diff --git a/cpp/controllers/phase_handler.h b/cpp/controllers/phase_handler.h index f957ac30..4474f726 100644 --- a/cpp/controllers/phase_handler.h +++ b/cpp/controllers/phase_handler.h @@ -8,8 +8,8 @@ #pragma once +#include #include -#include #include "shared/scsi.h" using namespace std; @@ -79,19 +79,13 @@ class PhaseHandler bool ProcessPhase() const { - try { - phase_executors[static_cast(phase)](); - } - catch (const invalid_argument&) { - return false; - } - - return true; + assert(phase <= bus_phase::reserved); + return phase_executors[static_cast(phase)](); } private: bus_phase phase = bus_phase::busfree; - array, 11> phase_executors; + array, static_cast(bus_phase::reserved) + 1> phase_executors; }; diff --git a/cpp/devices/disk_cache.cpp b/cpp/devices/disk_cache.cpp index 5315e99a..1d93b5fa 100644 --- a/cpp/devices/disk_cache.cpp +++ b/cpp/devices/disk_cache.cpp @@ -17,13 +17,18 @@ #include #include "disk_track.h" -bool DiskCache::Init() +DiskCache::DiskCache(const string &path, int size, uint64_t sectors) : sec_path(path), sec_blocks( + static_cast(sectors)) { - if (!sec_blocks || sec_path.empty()) { - return false; + while ((1 << sec_size) != size) { + ++sec_size; } + assert(sec_size >= 8 && sec_size <= 12); +} - return true; +bool DiskCache::Init() +{ + return sec_blocks && !sec_path.empty(); } bool DiskCache::Flush() @@ -80,7 +85,6 @@ int DiskCache::WriteSectors(span buf, uint64_t sector, uint32_t c // Track Assignment shared_ptr DiskCache::Assign(int track) { - assert(sec_size != 0); assert(track >= 0); // First, check if it is already assigned diff --git a/cpp/devices/disk_cache.h b/cpp/devices/disk_cache.h index 12c7ed1b..73b4d62e 100644 --- a/cpp/devices/disk_cache.h +++ b/cpp/devices/disk_cache.h @@ -13,27 +13,16 @@ #pragma once -#include #include "cache.h" class DiskTrack; class DiskCache : public Cache { - // Number of tracks to cache - static constexpr int CACHE_MAX = 16; public: - using cache_t = struct { - shared_ptr disktrk; - uint32_t serial; - }; - - DiskCache(const string &path, int size, uint64_t sectors) - : sec_path(path), sec_size(SHIFT_COUNTS.at(size)), sec_blocks(static_cast(sectors)) - { - } + DiskCache(const string&, int, uint64_t); ~DiskCache() override = default; bool Init() override; @@ -45,24 +34,32 @@ class DiskCache : public Cache private: + using cache_t = struct { + shared_ptr disktrk; + uint32_t serial; + }; + shared_ptr Assign(int); shared_ptr GetTrack(uint32_t); bool Load(int index, int track, shared_ptr); void UpdateSerialNumber(); - // Internal datay - array cache = { }; // Cache management - uint32_t serial = 0; // Last serial number - string sec_path; // Path - int sec_size; // Sector Size (8=256, 9=512, 10=1024, 11=2048, 12=4096) - int sec_blocks; // Blocks per sector + // Number of tracks to cache + static constexpr int CACHE_MAX = 16; + + array 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; uint64_t read_error_count = 0; uint64_t write_error_count = 0; uint64_t cache_miss_read_count = 0; uint64_t cache_miss_write_count = 0; - - static inline const unordered_map SHIFT_COUNTS = - { { 256, 8 }, { 512, 9 }, { 1024, 10 }, { 2048, 11 }, { 4096, 12 } }; }; diff --git a/cpp/devices/host_services.cpp b/cpp/devices/host_services.cpp index 93294f16..835763f1 100644 --- a/cpp/devices/host_services.cpp +++ b/cpp/devices/host_services.cpp @@ -268,24 +268,23 @@ void HostServices::SetUpModePages(map> &pages, int page, bool void HostServices::AddRealtimeClockPage(map> &pages, bool changeable) const { - pages[32] = vector(10); + pages[32] = vector(sizeof(mode_page_datetime) + 2); if (!changeable) { - const auto now = system_clock::now(); - const time_t t = system_clock::to_time_t(now); + const time_t &t = system_clock::to_time_t(system_clock::now()); tm localtime; localtime_r(&t, &localtime); mode_page_datetime datetime; datetime.major_version = 0x01; datetime.minor_version = 0x00; - datetime.year = (uint8_t)localtime.tm_year; - datetime.month = (uint8_t)localtime.tm_mon; - datetime.day = (uint8_t)localtime.tm_mday; - datetime.hour = (uint8_t)localtime.tm_hour; - datetime.minute = (uint8_t)localtime.tm_min; + datetime.year = static_cast(localtime.tm_year); + datetime.month = static_cast(localtime.tm_mon); + datetime.day = static_cast(localtime.tm_mday); + datetime.hour = static_cast(localtime.tm_hour); + datetime.minute = static_cast(localtime.tm_min); // Ignore leap second for simplicity - datetime.second = (uint8_t)(localtime.tm_sec < 60 ? localtime.tm_sec : 59); + datetime.second = static_cast(localtime.tm_sec < 60 ? localtime.tm_sec : 59); memcpy(&pages[32][2], &datetime, sizeof(datetime)); } diff --git a/cpp/devices/optical_memory.h b/cpp/devices/optical_memory.h index b00f6419..ffcb32d2 100644 --- a/cpp/devices/optical_memory.h +++ b/cpp/devices/optical_memory.h @@ -2,18 +2,12 @@ // // SCSI device emulator and SCSI tools for the Raspberry Pi // -// Copyright (C) 2001-2006 ＰＩ．(ytanaka@ipc-tokai.or.jp) -// Copyright (C) 2014-2020 GIMONS -// Copyright (C) akuker // Copyright (C) 2022-2024 Uwe Seimet // //--------------------------------------------------------------------------- #pragma once -#include -#include -#include #include "disk.h" using Geometry = pair; diff --git a/cpp/devices/printer.cpp b/cpp/devices/printer.cpp index 11bb0ccf..4b8129b0 100644 --- a/cpp/devices/printer.cpp +++ b/cpp/devices/printer.cpp @@ -47,6 +47,11 @@ bool Printer::Init(const param_map ¶ms) { PrimaryDevice::Init(params); + if (GetParam("cmd").find("%f") == string::npos) { + LogTrace("Missing filename specifier '%f'"); + return false; + } + AddCommand(scsi_command::cmd_test_unit_ready, [this] { TestUnitReady(); @@ -65,11 +70,6 @@ bool Printer::Init(const param_map ¶ms) TestUnitReady(); }); - if (GetParam("cmd").find("%f") == string::npos) { - LogTrace("Missing filename specifier %f"); - return false; - } - error_code error; file_template = temp_directory_path(error); // NOSONAR Publicly writable directory is fine here file_template += PRINTER_FILE_PATTERN; diff --git a/cpp/devices/sasi_hd.h b/cpp/devices/sasi_hd.h index 3070fd0c..d7b50081 100644 --- a/cpp/devices/sasi_hd.h +++ b/cpp/devices/sasi_hd.h @@ -8,7 +8,6 @@ #pragma once -#include #include "disk.h" class SasiHd : public Disk diff --git a/cpp/devices/scsi_hd.h b/cpp/devices/scsi_hd.h index 58d10eb0..c7062c52 100644 --- a/cpp/devices/scsi_hd.h +++ b/cpp/devices/scsi_hd.h @@ -8,9 +8,6 @@ #pragma once -#include -#include -#include #include "disk.h" class ScsiHd : public Disk diff --git a/cpp/devices/storage_device.cpp b/cpp/devices/storage_device.cpp index 881ce336..114f8727 100644 --- a/cpp/devices/storage_device.cpp +++ b/cpp/devices/storage_device.cpp @@ -30,7 +30,7 @@ void StorageDevice::CleanUp() void StorageDevice::ValidateFile() { if (!blocks) { - throw io_exception(GetTypeString() + " device has 0 blocks"); + throw io_exception("Device has 0 blocks"); } if (GetFileSize() > 2LL * 1024 * 1024 * 1024 * 1024) { diff --git a/cpp/s2p/s2p_core.cpp b/cpp/s2p/s2p_core.cpp index e4cbaf13..c09b8aac 100644 --- a/cpp/s2p/s2p_core.cpp +++ b/cpp/s2p/s2p_core.cpp @@ -340,10 +340,10 @@ void S2p::CreateDevices() SetDeviceProperties(*device, key_components[2], value); } - AttachDevices(command); + AttachInitialDevices(command); } -void S2p::AttachDevices(PbCommand &command) +void S2p::AttachInitialDevices(PbCommand &command) { if (command.devices_size()) { command.set_operation(ATTACH); @@ -356,8 +356,8 @@ void S2p::AttachDevices(PbCommand &command) #ifdef BUILD_SCHS // Ensure that all host services have a dispatcher - for (auto d : controller_factory.GetAllDevices()) { - if (auto host_services = dynamic_pointer_cast(d); host_services) { + for (auto device : controller_factory.GetAllDevices()) { + if (auto host_services = dynamic_pointer_cast(device); host_services) { host_services->SetDispatcher(dispatcher); } } diff --git a/cpp/s2p/s2p_core.h b/cpp/s2p/s2p_core.h index 33eca32f..b03bf2b7 100644 --- a/cpp/s2p/s2p_core.h +++ b/cpp/s2p/s2p_core.h @@ -33,7 +33,7 @@ class S2p string MapExtensions() const; void LogProperties() const; void CreateDevices(); - void AttachDevices(PbCommand&); + void AttachInitialDevices(PbCommand&); void ProcessScsiCommands(); bool WaitForNotBusy() const; diff --git a/cpp/s2p/s2p_parser.cpp b/cpp/s2p/s2p_parser.cpp index 4b394270..bce44ecd 100644 --- a/cpp/s2p/s2p_parser.cpp +++ b/cpp/s2p/s2p_parser.cpp @@ -232,7 +232,7 @@ property_map S2pParser::ParseArguments(span initial_args, bool &ignore_co string S2pParser::ParseBlueScsiFilename(property_map &properties, const string &d, const string &filename) { - const unordered_map> BLUE_SCSI_TO_S2P_TYPES = { + const unordered_map BLUE_SCSI_TO_S2P_TYPES = { { "CD", "sccd" }, { "FD", "schd" }, { "HD", "schd" }, diff --git a/cpp/s2p/s2p_thread.cpp b/cpp/s2p/s2p_thread.cpp index 27aa188a..3e551521 100644 --- a/cpp/s2p/s2p_thread.cpp +++ b/cpp/s2p/s2p_thread.cpp @@ -79,6 +79,11 @@ void S2pThread::Stop() } } +bool S2pThread::IsRunning() const +{ + return service_socket != -1 && service_thread.joinable(); +} + void S2pThread::Execute() const { while (service_socket != -1) { diff --git a/cpp/s2p/s2p_thread.h b/cpp/s2p/s2p_thread.h index 4ea7107b..fddf5906 100644 --- a/cpp/s2p/s2p_thread.h +++ b/cpp/s2p/s2p_thread.h @@ -25,10 +25,7 @@ class S2pThread string Init(const callback&, int); void Start(); void Stop(); - bool IsRunning() const - { - return service_socket != -1 && service_thread.joinable(); - } + bool IsRunning() const; private: diff --git a/cpp/s2pctl/s2pctl_display.cpp b/cpp/s2pctl/s2pctl_display.cpp index 374f16d5..f2469ca6 100644 --- a/cpp/s2pctl/s2pctl_display.cpp +++ b/cpp/s2pctl/s2pctl_display.cpp @@ -90,7 +90,7 @@ string S2pCtlDisplay::DisplayDeviceInfo(const PbDevice &pb_device) const string S2pCtlDisplay::DisplayVersionInfo(const PbVersionInfo &version_info) const { - string version = "Device server version: " + version_info.identifier(); + string version = "Server version: " + version_info.identifier(); if (version_info.identifier().empty() || version_info.major_version() >= 21) { if (version_info.major_version() == 21 && version_info.minor_version() < 12) { version += "RaSCSI"; diff --git a/cpp/shared/s2p_util.cpp b/cpp/shared/s2p_util.cpp index aa5b8b38..263cfe6b 100644 --- a/cpp/shared/s2p_util.cpp +++ b/cpp/shared/s2p_util.cpp @@ -120,7 +120,7 @@ string s2p_util::GetLine(const string &prompt) return ""; } - if (!line.empty() && !line.starts_with("#")) { + if (!line.empty() && line[0] != '#') { return line; } } diff --git a/cpp/shared/s2p_util.h b/cpp/shared/s2p_util.h index 5f54fc7a..8424ae47 100644 --- a/cpp/shared/s2p_util.h +++ b/cpp/shared/s2p_util.h @@ -35,8 +35,9 @@ struct StringHash } }; -string Join(const auto &collection, const string_view separator = ", ") +string Join(const auto &collection, const char *separator = ", ") { + // Using a stream (and not a string) is required in order to correctly convert the element data ostringstream s; for (const auto &element : collection) { @@ -115,10 +116,11 @@ static const unordered_map STATUS_MAPPING = { { status_code::condition_met, "CONDITION MET" }, { status_code::busy, "BUSY" }, { status_code::intermediate, "INTERMEDIATE" }, - { status_code::condition_met, "CONDITION MET" }, { status_code::intermediate_condition_met, "INTERMEDIATE-CONDITION MET" }, { status_code::reservation_conflict, "RESERVATION CONFLICT" }, { status_code::command_terminated, "COMMAND TERMINATED" }, - { status_code::queue_full, "QUEUE FULL" } + { status_code::queue_full, "QUEUE FULL" }, + { status_code::aca_active, "ACA ACTIVE" }, + { status_code::task_aborted, "TASK ABORTED" } }; } diff --git a/cpp/shared/s2p_version.cpp b/cpp/shared/s2p_version.cpp index 62ce6438..4aac0e7d 100644 --- a/cpp/shared/s2p_version.cpp +++ b/cpp/shared/s2p_version.cpp @@ -9,6 +9,6 @@ #include "s2p_version.h" const int s2p_major_version = 3; -const int s2p_minor_version = 2; +const int s2p_minor_version = 3; const int s2p_revision = 0; const std::string s2p_suffix = ""; diff --git a/cpp/shared/scsi.h b/cpp/shared/scsi.h index 4ab01430..59a189be 100644 --- a/cpp/shared/scsi.h +++ b/cpp/shared/scsi.h @@ -111,7 +111,9 @@ enum class status_code intermediate_condition_met = 0x14, reservation_conflict = 0x18, command_terminated = 0x22, - queue_full = 0x28 + queue_full = 0x28, + aca_active = 0x30, + task_aborted = 0x40 }; enum class sense_key diff --git a/cpp/test/command_context_test.cpp b/cpp/test/command_context_test.cpp index 254e2594..07290414 100644 --- a/cpp/test/command_context_test.cpp +++ b/cpp/test/command_context_test.cpp @@ -26,33 +26,33 @@ TEST(CommandContext, ReadCommand) // Invalid magic with wrong length vector data = { byte { '1' }, byte { '2' }, byte { '3' } }; - fd = open(CreateTempFileWithData(data).string().c_str(), O_RDONLY); + fd = open(CreateTempFileWithData(data).c_str(), O_RDONLY); CommandContext context2(fd); EXPECT_THROW(context2.ReadCommand(), io_exception); close(fd); // Invalid magic with right length data = { byte { '1' }, byte { '2' }, byte { '3' }, byte { '4' }, byte { '5' }, byte { '6' } }; - fd = open(CreateTempFileWithData(data).string().c_str(), O_RDONLY); + fd = open(CreateTempFileWithData(data).c_str(), O_RDONLY); CommandContext context3(fd); EXPECT_THROW(context3.ReadCommand(), io_exception); close(fd); data = { byte { 'R' }, byte { 'A' }, byte { 'S' }, byte { 'C' }, byte { 'S' }, byte { 'I' }, byte { '1' } }; // Valid magic but invalid command - fd = open(CreateTempFileWithData(data).string().c_str(), O_RDONLY); + fd = open(CreateTempFileWithData(data).c_str(), O_RDONLY); CommandContext context4(fd); EXPECT_THROW(context4.ReadCommand(), io_exception); close(fd); data = { byte { 'R' }, byte { 'A' }, byte { 'S' }, byte { 'C' }, byte { 'S' }, byte { 'I' } }; // Valid magic but missing command - fd = open(CreateTempFileWithData(data).string().c_str(), O_RDONLY); + fd = open(CreateTempFileWithData(data).c_str(), O_RDONLY); CommandContext context5(fd); EXPECT_THROW(context5.ReadCommand(), io_exception); close(fd); - const string filename = CreateTempFileWithData(data).string(); + const string &filename = CreateTempFileWithData(data); fd = open(filename.c_str(), O_RDWR | O_APPEND); PbCommand command; command.set_operation(PbOperation::SERVER_INFO); diff --git a/cpp/test/device_factory_test.cpp b/cpp/test/device_factory_test.cpp index 63a9d028..75ffbcae 100644 --- a/cpp/test/device_factory_test.cpp +++ b/cpp/test/device_factory_test.cpp @@ -8,56 +8,26 @@ #include #include "base/device_factory.h" -#ifdef BUILD_SCDP #include "devices/daynaport.h" -#endif -#ifdef BUILD_SCHS #include "devices/host_services.h" -#endif -#ifdef BUILD_SCMO #include "devices/optical_memory.h" -#endif -#ifdef BUILD_SCLP #include "devices/printer.h" -#endif -#ifdef BUILD_SAHD #include "devices/sasi_hd.h" -#endif -#ifdef BUILD_SCCD #include "devices/scsi_cd.h" -#endif -#if defined BUILD_SCHD || defined BUILD_SCRM #include "devices/scsi_hd.h" -#endif TEST(DeviceFactoryTest, CreateDevice) { const DeviceFactory &device_factory = DeviceFactory::Instance(); -#ifdef BUILD_SCHD EXPECT_NE(nullptr, dynamic_pointer_cast(device_factory.CreateDevice(SCHD, 0, ""))); -#endif -#ifdef BUILD_SCRM EXPECT_NE(nullptr, dynamic_pointer_cast(device_factory.CreateDevice(SCRM, 0, ""))); -#endif -#ifdef BUILD_SCMO EXPECT_NE(nullptr, dynamic_pointer_cast(device_factory.CreateDevice(SCMO, 0, ""))); -#endif -#ifdef BUILD_SCCD EXPECT_NE(nullptr, dynamic_pointer_cast(device_factory.CreateDevice(SCCD, 0, ""))); -#endif -#ifdef BUILD_SCDP EXPECT_NE(nullptr, dynamic_pointer_cast(device_factory.CreateDevice(SCDP, 0, ""))); -#endif -#ifdef BUILD_SCLP - EXPECT_NE(nullptr, dynamic_pointer_cast(device_factory.CreateDevice(SCHS, 0, ""))); -#endif -#ifdef BUILD_SCHS EXPECT_NE(nullptr, dynamic_pointer_cast(device_factory.CreateDevice(SCLP, 0, ""))); -#endif -#ifdef BUILD_SAHD + EXPECT_NE(nullptr, dynamic_pointer_cast(device_factory.CreateDevice(SCHS, 0, ""))); EXPECT_NE(nullptr, dynamic_pointer_cast(device_factory.CreateDevice(SAHD, 0, ""))); -#endif EXPECT_EQ(nullptr, device_factory.CreateDevice(UNDEFINED, 0, "")); } diff --git a/cpp/test/device_test.cpp b/cpp/test/device_test.cpp index 7cf6c730..20340727 100644 --- a/cpp/test/device_test.cpp +++ b/cpp/test/device_test.cpp @@ -120,33 +120,6 @@ TEST(DeviceTest, Properties) EXPECT_EQ(LUN, device.GetLun()); } -TEST(DeviceTest, GetTypeString) -{ - MockDevice schd(SCHD); - EXPECT_EQ("SCHD", schd.GetTypeString()); - - MockDevice scrm(SCRM); - EXPECT_EQ("SCRM", scrm.GetTypeString()); - - MockDevice scmo(SCMO); - EXPECT_EQ("SCMO", scmo.GetTypeString()); - - MockDevice sccd(SCCD); - EXPECT_EQ("SCCD", sccd.GetTypeString()); - - MockDevice schs(SCHS); - EXPECT_EQ("SCHS", schs.GetTypeString()); - - MockDevice scdp(SCDP); - EXPECT_EQ("SCDP", scdp.GetTypeString()); - - MockDevice sclp(SCLP); - EXPECT_EQ("SCLP", sclp.GetTypeString()); - - MockDevice sahd(SAHD); - EXPECT_EQ("SAHD", sahd.GetTypeString()); -} - TEST(DeviceTest, Vendor) { MockDevice device(0); diff --git a/cpp/test/storage_device_test.cpp b/cpp/test/storage_device_test.cpp index ea0bee0f..c419a14a 100644 --- a/cpp/test/storage_device_test.cpp +++ b/cpp/test/storage_device_test.cpp @@ -147,7 +147,7 @@ TEST(StorageDeviceTest, GetFileSize) MockStorageDevice device; const path filename = CreateTempFile(512); - device.SetFilename(filename.c_str()); + device.SetFilename(filename.string()); const off_t size = device.GetFileSize(); EXPECT_EQ(512, size); diff --git a/cpp/test/test_shared.cpp b/cpp/test/test_shared.cpp index 535fa155..b9ed75c4 100644 --- a/cpp/test/test_shared.cpp +++ b/cpp/test/test_shared.cpp @@ -126,10 +126,10 @@ pair testing::OpenTempFile() path testing::CreateTempFile(size_t size) { - return CreateTempFileWithData(vector(size)); + return path(CreateTempFileWithData(vector(size))); } -path testing::CreateTempFileWithData(const span data) +string testing::CreateTempFileWithData(const span data) { const auto& [fd, filename] = OpenTempFile(); @@ -139,7 +139,7 @@ path testing::CreateTempFileWithData(const span data) TestShared::RememberTempFile(filename); - return path(filename); + return filename; } string testing::ReadTempFileToString(const string &filename) diff --git a/cpp/test/test_shared.h b/cpp/test/test_shared.h index 9113ad12..87c985bf 100644 --- a/cpp/test/test_shared.h +++ b/cpp/test/test_shared.h @@ -30,7 +30,7 @@ vector CreateParameters(const string&); pair OpenTempFile(); path CreateTempFile(size_t); -path CreateTempFileWithData(span); +string CreateTempFileWithData(span); string ReadTempFileToString(const string&); int GetInt16(const vector&, int);