diff --git a/cpp/devices/ctapdriver.cpp b/cpp/devices/ctapdriver.cpp index c010c93c..6f394ae6 100644 --- a/cpp/devices/ctapdriver.cpp +++ b/cpp/devices/ctapdriver.cpp @@ -213,6 +213,14 @@ void CTapDriver::CleanUp() const } } +param_map CTapDriver::GetDefaultParams() const +{ + return { + { "interface", Join(GetNetworkInterfaces(), ",") }, + { "inet", DEFAULT_IP } + }; +} + pair CTapDriver::ExtractAddressAndMask(const string& s) { string address = s; diff --git a/cpp/devices/ctapdriver.h b/cpp/devices/ctapdriver.h index f8b3bdfd..1e0eff68 100644 --- a/cpp/devices/ctapdriver.h +++ b/cpp/devices/ctapdriver.h @@ -31,6 +31,8 @@ class CTapDriver { static const string BRIDGE_NAME; + const inline static string DEFAULT_IP = "10.10.20.1/24"; //NOSONAR This hardcoded IP address is safe + public: CTapDriver() = default; @@ -41,6 +43,8 @@ public: bool Init(const param_map&); void CleanUp() const; + param_map GetDefaultParams() const; + void GetMacAddr(uint8_t *) const; int Receive(uint8_t *) const; int Send(const uint8_t *, int) const; diff --git a/cpp/devices/device.cpp b/cpp/devices/device.cpp index 58b071f8..6c764ec3 100644 --- a/cpp/devices/device.cpp +++ b/cpp/devices/device.cpp @@ -89,7 +89,7 @@ string Device::GetParam(const string& key) const void Device::SetParams(const param_map& set_params) { - params = default_params; + params = GetDefaultParams(); // Devices with image file support implicitly support the "file" parameter if (SupportsFile()) { diff --git a/cpp/devices/device.h b/cpp/devices/device.h index 80f1a272..edff1d13 100644 --- a/cpp/devices/device.h +++ b/cpp/devices/device.h @@ -68,9 +68,6 @@ class Device //NOSONAR The number of fields and methods is justified, the comple // The parameters the device was created with param_map params; - // The default parameters - param_map default_params; - // Sense Key and ASC // MSB Reserved (0x00) // Sense Key @@ -139,7 +136,7 @@ public: void SupportsParams(bool b) { supports_params = b; } void SupportsFile(bool b) { supports_file = b; } auto GetParams() const { return params; } - void SetDefaultParams(const param_map& p) { default_params = p; } + virtual param_map GetDefaultParams() const { return {}; } void SetStatusCode(int s) { status_code = s; } diff --git a/cpp/devices/device_factory.cpp b/cpp/devices/device_factory.cpp index da69fa98..f951d645 100644 --- a/cpp/devices/device_factory.cpp +++ b/cpp/devices/device_factory.cpp @@ -29,14 +29,6 @@ DeviceFactory::DeviceFactory() sector_sizes[SCMO] = { 512, 1024, 2048, 4096 }; sector_sizes[SCCD] = { 512, 2048}; - const string interfaces = Join(GetNetworkInterfaces(), ","); - - default_params[SCBR]["interface"] = interfaces; - default_params[SCBR]["inet"] = DEFAULT_IP; - default_params[SCDP]["interface"] = interfaces; - default_params[SCDP]["inet"] = DEFAULT_IP; - default_params[SCLP]["cmd"] = "lp -oraw %f"; - extension_mapping["hd1"] = SCHD; extension_mapping["hds"] = SCHD; extension_mapping["hda"] = SCHD; @@ -115,7 +107,6 @@ shared_ptr DeviceFactory::CreateDevice(PbDeviceType type, int lun device = make_shared(lun); // Since this is an emulation for a specific driver the product name has to be set accordingly device->SetProduct("RASCSI BRIDGE"); - device->SetDefaultParams(default_params.find(type)->second); break; case SCDP: @@ -124,7 +115,6 @@ shared_ptr DeviceFactory::CreateDevice(PbDeviceType type, int lun device->SetVendor("Dayna"); device->SetProduct("SCSI/Link"); device->SetRevision("1.4a"); - device->SetDefaultParams(default_params.find(type)->second); break; case SCHS: @@ -137,7 +127,6 @@ shared_ptr DeviceFactory::CreateDevice(PbDeviceType type, int lun case SCLP: device = make_shared(lun); device->SetProduct("SCSI PRINTER"); - device->SetDefaultParams(default_params.find(type)->second); break; default: @@ -147,14 +136,14 @@ shared_ptr DeviceFactory::CreateDevice(PbDeviceType type, int lun return device; } -const unordered_set& DeviceFactory::GetSectorSizes(PbDeviceType type) const +// TODO Move to respective device, which may require changes in the SCSI_HD/SCSIHD_NEC inheritance hierarchy +unordered_set DeviceFactory::GetSectorSizes(PbDeviceType type) const { const auto& it = sector_sizes.find(type); - return it != sector_sizes.end() ? it->second : EMPTY_SET; -} - -const param_map& DeviceFactory::GetDefaultParams(PbDeviceType type) const -{ - const auto& it = default_params.find(type); - return it != default_params.end() ? it->second : EMPTY_PARAM_MAP; + if (it != sector_sizes.end()) { + return it->second; + } + else { + return {}; + } } diff --git a/cpp/devices/device_factory.h b/cpp/devices/device_factory.h index cdce0ba2..fef1c17d 100644 --- a/cpp/devices/device_factory.h +++ b/cpp/devices/device_factory.h @@ -12,7 +12,6 @@ #pragma once #include "shared/piscsi_util.h" -#include "devices/device.h" #include #include #include @@ -25,7 +24,6 @@ class PrimaryDevice; class DeviceFactory { - const inline static string DEFAULT_IP = "10.10.20.1/24"; //NOSONAR This hardcoded IP address is safe public: @@ -34,20 +32,14 @@ public: shared_ptr CreateDevice(PbDeviceType, int, const string&) const; PbDeviceType GetTypeForFile(const string&) const; - const unordered_set& GetSectorSizes(PbDeviceType type) const; - const param_map& GetDefaultParams(PbDeviceType type) const; + unordered_set GetSectorSizes(PbDeviceType type) const; const auto& GetExtensionMapping() const { return extension_mapping; } private: unordered_map> sector_sizes; - unordered_map default_params; - unordered_map> extension_mapping; unordered_map> device_mapping; - - inline static const unordered_set EMPTY_SET; - inline static const param_map EMPTY_PARAM_MAP; }; diff --git a/cpp/devices/scsi_daynaport.cpp b/cpp/devices/scsi_daynaport.cpp index e710ab33..db46ea16 100644 --- a/cpp/devices/scsi_daynaport.cpp +++ b/cpp/devices/scsi_daynaport.cpp @@ -54,8 +54,8 @@ bool SCSIDaynaPort::Init(const param_map& params) // In the MacOS driver, it looks like the driver is doing two "READ" system calls. SetSendDelay(DAYNAPORT_READ_HEADER_SZ); - m_bTapEnable = m_tap.Init(GetParams()); - if (!m_bTapEnable) { + tap_enabled = tap.Init(GetParams()); + if (!tap_enabled) { // Not terminating on regular Linux PCs is helpful for testing #if !defined(__x86_64__) && !defined(__X86__) return false; @@ -73,7 +73,7 @@ bool SCSIDaynaPort::Init(const param_map& params) void SCSIDaynaPort::CleanUp() { - m_tap.CleanUp(); + tap.CleanUp(); } vector SCSIDaynaPort::InquiryInternal() const @@ -145,7 +145,7 @@ int SCSIDaynaPort::Read(cdb_t cdb, vector& buf, uint64_t) const // The first 2 bytes are reserved for the length of the packet // The next 4 bytes are reserved for a flag field //rx_packet_size = m_tap.Rx(response->data); - rx_packet_size = m_tap.Receive(&buf[DAYNAPORT_READ_HEADER_SZ]); + rx_packet_size = tap.Receive(&buf[DAYNAPORT_READ_HEADER_SZ]); // If we didn't receive anything, return size of 0 if (rx_packet_size <= 0) { @@ -194,7 +194,7 @@ int SCSIDaynaPort::Read(cdb_t cdb, vector& buf, uint64_t) const // If there are pending packets to be processed, we'll tell the host that the read // length was 0. - if (!m_tap.HasPendingPackets()) { + if (!tap.HasPendingPackets()) { response->length = 0; response->flags = read_data_flags_t::e_no_more_data; return DAYNAPORT_READ_HEADER_SZ; @@ -219,7 +219,7 @@ int SCSIDaynaPort::Read(cdb_t cdb, vector& buf, uint64_t) const size = 64; } SetInt16(buf, 0, size); - SetInt32(buf, 2, m_tap.HasPendingPackets() ? 0x10 : 0x00); + SetInt32(buf, 2, tap.HasPendingPackets() ? 0x10 : 0x00); // Return the packet size + 2 for the length + 4 for the flag field // The CRC was already appended by the ctapdriver @@ -256,13 +256,13 @@ bool SCSIDaynaPort::Write(cdb_t cdb, span buf) const { if (const int data_format = cdb[5]; data_format == 0x00) { const int data_length = GetInt16(cdb, 3); - m_tap.Send(buf.data(), data_length); + tap.Send(buf.data(), data_length); LogTrace("Transmitted " + to_string(data_length) + " byte(s) (00 format)"); } else if (data_format == 0x80) { // The data length is specified in the first 2 bytes of the payload const int data_length = buf[1] + ((static_cast(buf[0]) & 0xff) << 8); - m_tap.Send(&(buf.data()[4]), data_length); + tap.Send(&(buf.data()[4]), data_length); LogTrace("Transmitted " + to_string(data_length) + "byte(s) (80 format)"); } else { @@ -455,18 +455,18 @@ void SCSIDaynaPort::SetMcastAddr() const void SCSIDaynaPort::EnableInterface() const { if (GetController()->GetCmdByte(5) & 0x80) { - if (const string error = m_tap.IpLink(true); !error.empty()) { + if (const string error = tap.IpLink(true); !error.empty()) { LogWarn("Unable to enable the DaynaPort Interface: " + error); throw scsi_exception(sense_key::aborted_command); } - m_tap.Flush(); + tap.Flush(); LogInfo("The DaynaPort interface has been ENABLED"); } else { - if (const string error = m_tap.IpLink(false); !error.empty()) { + if (const string error = tap.IpLink(false); !error.empty()) { LogWarn("Unable to disable the DaynaPort Interface: " + error); throw scsi_exception(sense_key::aborted_command); diff --git a/cpp/devices/scsi_daynaport.h b/cpp/devices/scsi_daynaport.h index 45d0ea50..dc2ede6a 100644 --- a/cpp/devices/scsi_daynaport.h +++ b/cpp/devices/scsi_daynaport.h @@ -52,6 +52,8 @@ public: bool Init(const param_map&) override; void CleanUp() override; + param_map GetDefaultParams() const override { return tap.GetDefaultParams(); } + // Commands vector InquiryInternal() const override; int Read(cdb_t, vector&, uint64_t) const; @@ -116,8 +118,7 @@ private: .frames_lost = 0, }; - CTapDriver m_tap; + CTapDriver tap; - // TAP valid flag - bool m_bTapEnable = false; + bool tap_enabled = false; }; diff --git a/cpp/devices/scsi_host_bridge.cpp b/cpp/devices/scsi_host_bridge.cpp index 93ef1c0d..394441c1 100644 --- a/cpp/devices/scsi_host_bridge.cpp +++ b/cpp/devices/scsi_host_bridge.cpp @@ -42,14 +42,14 @@ bool SCSIBR::Init(const param_map& params) AddCommand(scsi_command::eCmdSendMessage10, [this] { SendMessage10(); }); #ifdef __linux__ - m_bTapEnable = tap.Init(GetParams()); - if (!m_bTapEnable){ + tap_enabled = tap.Init(GetParams()); + if (!tap_enabled){ return false; } #endif // Generate MAC Address - if (m_bTapEnable) { + if (tap_enabled) { tap.GetMacAddr(mac_addr.data()); mac_addr[5]++; } @@ -57,13 +57,13 @@ bool SCSIBR::Init(const param_map& params) // Packet reception flag OFF packet_enable = false; - SetReady(m_bTapEnable); + SetReady(tap_enabled); // Not terminating on regular Linux PCs is helpful for testing #if defined(__x86_64__) || defined(__X86__) return true; #else - return m_bTapEnable; + return tap_enabled; #endif } @@ -85,7 +85,7 @@ vector SCSIBR::InquiryInternal() const buf[36] = '0'; // TAP Enable - if (m_bTapEnable) { + if (tap_enabled) { buf[37] = '1'; } @@ -115,7 +115,7 @@ int SCSIBR::GetMessage10(cdb_t cdb, vector& buf) switch (type) { case 1: // Ethernet // Do not process if TAP is invalid - if (!m_bTapEnable) { + if (!tap_enabled) { return 0; } @@ -206,7 +206,7 @@ bool SCSIBR::ReadWrite(cdb_t cdb, vector& buf) switch (type) { case 1: // Ethernet // Do not process if TAP is invalid - if (!m_bTapEnable) { + if (!tap_enabled) { return false; } diff --git a/cpp/devices/scsi_host_bridge.h b/cpp/devices/scsi_host_bridge.h index f3a80096..b2cdf981 100644 --- a/cpp/devices/scsi_host_bridge.h +++ b/cpp/devices/scsi_host_bridge.h @@ -38,6 +38,8 @@ public: bool Init(const param_map&) override; void CleanUp() override; + param_map GetDefaultParams() const override { return tap.GetDefaultParams(); } + // Commands vector InquiryInternal() const override; int GetMessage10(cdb_t, vector&); @@ -55,7 +57,7 @@ private: void SendPacket(span, int) const; // Send a packet CTapDriver tap; // TAP driver - bool m_bTapEnable = false; // TAP valid flag + bool tap_enabled = false; // TAP valid flag array mac_addr = {}; // MAC Address int packet_len = 0; // Receive packet size array packet_buf; // Receive packet buffer diff --git a/cpp/devices/scsi_printer.cpp b/cpp/devices/scsi_printer.cpp index da4bff40..f4399985 100644 --- a/cpp/devices/scsi_printer.cpp +++ b/cpp/devices/scsi_printer.cpp @@ -73,6 +73,27 @@ bool SCSIPrinter::Init(const param_map& params) return true; } +void SCSIPrinter::CleanUp() +{ + PrimaryDevice::CleanUp(); + + if (out.is_open()) { + out.close(); + + error_code error; + remove(path(filename), error); + + filename = ""; + } +} + +param_map SCSIPrinter::GetDefaultParams() const +{ + return { + { "cmd", "lp -oraw %f" } + }; +} + void SCSIPrinter::TestUnitReady() { // The printer is always ready @@ -164,17 +185,3 @@ bool SCSIPrinter::WriteByteSequence(span buf) return !out.fail(); } - -void SCSIPrinter::CleanUp() -{ - PrimaryDevice::CleanUp(); - - if (out.is_open()) { - out.close(); - - error_code error; - remove(path(filename), error); - - filename = ""; - } -} diff --git a/cpp/devices/scsi_printer.h b/cpp/devices/scsi_printer.h index 6e626e54..e9f6c92d 100644 --- a/cpp/devices/scsi_printer.h +++ b/cpp/devices/scsi_printer.h @@ -33,6 +33,8 @@ public: bool Init(const param_map&) override; void CleanUp() override; + param_map GetDefaultParams() const override; + vector InquiryInternal() const override; bool WriteByteSequence(span) override; diff --git a/cpp/devices/scsihd_nec.h b/cpp/devices/scsihd_nec.h index 0bf7db36..eec3c1be 100644 --- a/cpp/devices/scsihd_nec.h +++ b/cpp/devices/scsihd_nec.h @@ -33,7 +33,7 @@ class SCSIHD_NEC : public SCSIHD //NOSONAR The inheritance hierarchy depth is ac { public: - explicit SCSIHD_NEC(int lun) : SCSIHD(lun, sector_sizes, false) {} + explicit SCSIHD_NEC(int lun) : SCSIHD(lun, { 512 }, false) {} ~SCSIHD_NEC() override = default; void Open() override; @@ -52,8 +52,6 @@ private: static int GetInt16LittleEndian(const uint8_t *); static int GetInt32LittleEndian(const uint8_t *); - static inline const unordered_set sector_sizes = { 512 }; - // Image file offset off_t image_offset = 0; diff --git a/cpp/piscsi/piscsi_response.cpp b/cpp/piscsi/piscsi_response.cpp index 6987f78f..16ac25b5 100644 --- a/cpp/piscsi/piscsi_response.cpp +++ b/cpp/piscsi/piscsi_response.cpp @@ -36,7 +36,7 @@ void PiscsiResponse::GetDeviceProperties(const Device& device, PbDevicePropertie properties.set_supports_params(device.SupportsParams()); if (device.SupportsParams()) { - for (const auto& [key, value] : device_factory.GetDefaultParams(device.GetType())) { + for (const auto& [key, value] : device.GetDefaultParams()) { auto& map = *properties.mutable_default_params(); map[key] = value; } diff --git a/cpp/test/device_factory_test.cpp b/cpp/test/device_factory_test.cpp index b37c9bc3..0c51c194 100644 --- a/cpp/test/device_factory_test.cpp +++ b/cpp/test/device_factory_test.cpp @@ -92,35 +92,6 @@ TEST(DeviceFactoryTest, GetExtensionMapping) EXPECT_EQ(SCCD, mapping["is1"]); } -TEST(DeviceFactoryTest, GetDefaultParams) -{ - DeviceFactory device_factory; - - param_map params = device_factory.GetDefaultParams(SCHD); - EXPECT_TRUE(params.empty()); - - params = device_factory.GetDefaultParams(SCRM); - EXPECT_TRUE(params.empty()); - - params = device_factory.GetDefaultParams(SCMO); - EXPECT_TRUE(params.empty()); - - params = device_factory.GetDefaultParams(SCCD); - EXPECT_TRUE(params.empty()); - - params = device_factory.GetDefaultParams(SCHS); - EXPECT_TRUE(params.empty()); - - params = device_factory.GetDefaultParams(SCBR); - EXPECT_EQ(2, params.size()); - - params = device_factory.GetDefaultParams(SCDP); - EXPECT_EQ(2, params.size()); - - params = device_factory.GetDefaultParams(SCLP); - EXPECT_EQ(1, params.size()); -} - TEST(DeviceFactoryTest, UnknownDeviceType) { DeviceFactory device_factory; diff --git a/cpp/test/device_test.cpp b/cpp/test/device_test.cpp index 0662b67e..60ad3b36 100644 --- a/cpp/test/device_test.cpp +++ b/cpp/test/device_test.cpp @@ -10,6 +10,14 @@ #include "mocks.h" #include "devices/device.h" +TEST(DeviceTest, GetDefaultParams) +{ + MockDevice device(0); + + const auto params = device.GetDefaultParams(); + EXPECT_TRUE(params.empty()); +} + TEST(DeviceTest, Properties) { const int LUN = 5; @@ -192,26 +200,6 @@ TEST(DeviceTest, GetPaddedName) EXPECT_EQ("V P R ", device.GetPaddedName()); } -TEST(DeviceTest, Params) -{ - MockDevice device(0); - param_map params; - params["key"] = "value"; - - EXPECT_EQ("", device.GetParam("key")); - - device.SetParams(params); - EXPECT_EQ("", device.GetParam("key")); - - param_map default_params; - default_params["key"] = "value"; - device.SetDefaultParams(default_params); - EXPECT_EQ("", device.GetParam("key")); - - device.SetParams(params); - EXPECT_EQ("value", device.GetParam("key")); -} - TEST(DeviceTest, StatusCode) { MockDevice device(0); diff --git a/cpp/test/piscsi_response_test.cpp b/cpp/test/piscsi_response_test.cpp index 19ebb49c..278880d7 100644 --- a/cpp/test/piscsi_response_test.cpp +++ b/cpp/test/piscsi_response_test.cpp @@ -53,7 +53,6 @@ void TestNonDiskDevice(PbDeviceType type, int default_param_count) EXPECT_EQ(0, device.block_size()); EXPECT_EQ(0, device.block_count()); EXPECT_EQ(default_param_count, device.properties().default_params().size()); - EXPECT_EQ(default_param_count, device.params().size()); EXPECT_FALSE(device.properties().supports_file()); if (default_param_count) { EXPECT_TRUE(device.properties().supports_params()); diff --git a/cpp/test/scsi_daynaport_test.cpp b/cpp/test/scsi_daynaport_test.cpp index 7b54a3f3..712b3b38 100644 --- a/cpp/test/scsi_daynaport_test.cpp +++ b/cpp/test/scsi_daynaport_test.cpp @@ -11,6 +11,13 @@ #include "shared/piscsi_exceptions.h" #include "devices/scsi_daynaport.h" +TEST(ScsiDaynaportTest, GetDefaultParams) +{ + const auto [controller, daynaport] = CreateDevice(SCDP); + const auto params = daynaport->GetDefaultParams(); + EXPECT_EQ(2, params.size()); +} + TEST(ScsiDaynaportTest, Inquiry) { TestInquiry::Inquiry(SCDP, device_type::processor, scsi_level::scsi_2, "Dayna SCSI/Link 1.4a", 0x20, false); diff --git a/cpp/test/scsi_host_bridge_test.cpp b/cpp/test/scsi_host_bridge_test.cpp index f0b2843f..117ab519 100644 --- a/cpp/test/scsi_host_bridge_test.cpp +++ b/cpp/test/scsi_host_bridge_test.cpp @@ -9,6 +9,13 @@ #include "mocks.h" +TEST(ScsiHostBridgeTest, GetDefaultParams) +{ + const auto [controller, bridge] = CreateDevice(SCBR); + const auto params = bridge->GetDefaultParams(); + EXPECT_EQ(2, params.size()); +} + TEST(ScsiHostBridgeTest, Inquiry) { TestInquiry::Inquiry(SCBR, device_type::communications, scsi_level::scsi_2, "PiSCSI RASCSI BRIDGE ", 0x27, false); diff --git a/cpp/test/scsi_printer_test.cpp b/cpp/test/scsi_printer_test.cpp index 7f29f4cf..36bfcd79 100644 --- a/cpp/test/scsi_printer_test.cpp +++ b/cpp/test/scsi_printer_test.cpp @@ -13,6 +13,13 @@ using namespace std; +TEST(ScsiPrinterTest, GetDefaultParams) +{ + const auto [controller, printer] = CreateDevice(SCLP); + const auto params = printer->GetDefaultParams(); + EXPECT_EQ(1, params.size()); +} + TEST(ScsiPrinterTest, Init) { auto [controller, printer] = CreateDevice(SCLP);