Update logging and unit tests

uweseimet · Jan 2, 2025 · b2cac14 · b2cac14
1 parent a91c481
commit b2cac14
Show file tree

Hide file tree

Showing 12 changed files with 38 additions and 48 deletions.
diff --git a/cpp/command/command_response.cpp b/cpp/command/command_response.cpp
@@ -2,7 +2,7 @@
 //
 // SCSI2Pi, SCSI device emulator and SCSI tools for the Raspberry Pi
 //
-// Copyright (C) 2021-2024 Uwe Seimet
+// Copyright (C) 2021-2025 Uwe Seimet
 //
 //---------------------------------------------------------------------------
 
@@ -522,7 +522,7 @@ bool CommandResponse::ValidateImageFile(const path &path, logger &logger)
     if (is_symlink(p)) {
         p = read_symlink(p);
         if (!exists(p)) {
-            logger.warn(fmt::format("Image file symlink '{}' is broken", path.string()));
+            logger.warn("Image file symlink '{}' is broken", path.string());
             return false;
         }
     }

diff --git a/cpp/controllers/script_generator.cpp b/cpp/controllers/script_generator.cpp
@@ -37,7 +37,7 @@ void ScriptGenerator::AddCdb(int id, int lun, cdb_t cdb)
         if (i) {
             file << ':';
         }
-        file << setfill('0') << setw(2) << cdb[i];
+        file << setfill('0') << setw(2) << (cdb[i] & 0xff);
     }
 
     file << flush;

diff --git a/cpp/devices/host_services.cpp b/cpp/devices/host_services.cpp
@@ -165,7 +165,7 @@ void HostServices::ReceiveOperationResults()
 
     const auto &it = execution_results.find(GetController()->GetInitiatorId());
     if (it == execution_results.end()) {
-        throw ScsiException(SenseKey::ABORTED_COMMAND, Asc::INTERNAL_TARGET_FAILURE);
+        throw ScsiException(SenseKey::ILLEGAL_REQUEST, Asc::DATA_CURRENTLY_UNAVAILABLE);
     }
     const string &execution_result = it->second;
 
@@ -196,16 +196,10 @@ void HostServices::ReceiveOperationResults()
 
     execution_results.erase(GetController()->GetInitiatorId());
 
-    const int allocation_length = GetCdbInt16(7);
-    const int length = min(allocation_length, static_cast<int>(data.size()));
-    if (!length) {
-        StatusPhase();
-    }
-    else {
-        GetController()->CopyToBuffer(data.data(), length);
+    const int length = min(GetCdbInt16(7), static_cast<int>(data.size()));
+    GetController()->CopyToBuffer(data.data(), length);
 
-        DataInPhase(length);
-    }
+    DataInPhase(length);
 }
 
 int HostServices::ModeSense6(cdb_t cdb, data_in_t buf) const
@@ -278,7 +272,7 @@ void HostServices::AddRealtimeClockPage(map<int, vector<byte>> &pages, bool chan
 int HostServices::WriteData(cdb_t cdb, data_out_t buf, int, int l)
 {
     if (static_cast<ScsiCommand>(cdb[0]) != ScsiCommand::EXECUTE_OPERATION) {
-        throw ScsiException(SenseKey::ABORTED_COMMAND);
+        throw ScsiException(SenseKey::ABORTED_COMMAND, Asc::INTERNAL_TARGET_FAILURE);
     }
 
     const auto length = GetCdbInt16(7);
@@ -291,38 +285,35 @@ int HostServices::WriteData(cdb_t cdb, data_out_t buf, int, int l)
     switch (input_format) {
     case ProtobufFormat::BINARY:
         if (!cmd.ParseFromArray(buf.data(), length)) {
-            LogTrace("Failed to deserialize protobuf binary data");
-            throw ScsiException(SenseKey::ABORTED_COMMAND);
+            throw ScsiException(SenseKey::ABORTED_COMMAND, Asc::INTERNAL_TARGET_FAILURE);
         }
         break;
 
     case ProtobufFormat::JSON: {
         if (string c((const char*)buf.data(), length); !JsonStringToMessage(c, &cmd).ok()) {
-            LogTrace("Failed to deserialize protobuf JSON data");
-            throw ScsiException(SenseKey::ABORTED_COMMAND);
+            throw ScsiException(SenseKey::ABORTED_COMMAND, Asc::INTERNAL_TARGET_FAILURE);
         }
         break;
     }
 
     case ProtobufFormat::TEXT: {
         if (string c((const char*)buf.data(), length); !TextFormat::ParseFromString(c, &cmd)) {
-            LogTrace("Failed to deserialize protobuf text format data");
-            throw ScsiException(SenseKey::ABORTED_COMMAND);
+            throw ScsiException(SenseKey::ABORTED_COMMAND, Asc::INTERNAL_TARGET_FAILURE);
         }
         break;
     }
 
     default:
         assert(false);
-        throw ScsiException(SenseKey::ABORTED_COMMAND);
+        throw ScsiException(SenseKey::ILLEGAL_REQUEST, Asc::INVALID_FIELD_IN_CDB);
     }
 
     PbResult result;
     CommandContext context(cmd, GetLogger());
     context.SetLocale(protobuf_util::GetParam(cmd, "locale"));
     if (!dispatcher->DispatchCommand(context, result)) {
         LogTrace("Failed to execute " + PbOperation_Name(cmd.operation()) + " operation");
-        throw ScsiException(SenseKey::ABORTED_COMMAND);
+        throw ScsiException(SenseKey::ABORTED_COMMAND, Asc::INTERNAL_TARGET_FAILURE);
     }
 
     execution_results[GetController()->GetInitiatorId()] = result.SerializeAsString();
@@ -332,18 +323,15 @@ int HostServices::WriteData(cdb_t cdb, data_out_t buf, int, int l)
 
 ProtobufFormat HostServices::ConvertFormat() const
 {
-    switch (GetCdbByte(1) & 0b00000111) {
+    switch (GetCdbByte(1) & 0b00011111) {
     case 0x001:
         return ProtobufFormat::BINARY;
-        break;
 
     case 0b010:
         return ProtobufFormat::JSON;
-        break;
 
     case 0b100:
         return ProtobufFormat::TEXT;
-        break;
 
     default:
         throw ScsiException(SenseKey::ILLEGAL_REQUEST, Asc::INVALID_FIELD_IN_CDB);

diff --git a/cpp/devices/optical_memory.cpp b/cpp/devices/optical_memory.cpp
@@ -4,8 +4,7 @@
 //
 // Copyright (C) 2001-2006 ＰＩ．([email protected])
 // Copyright (C) 2014-2020 GIMONS
-// Coypright (C) akuker
-// Copyright (C) 2022-2024 Uwe Seimet
+// Copyright (C) 2022-2025 Uwe Seimet
 //
 //---------------------------------------------------------------------------
 

diff --git a/cpp/devices/printer.cpp b/cpp/devices/printer.cpp
@@ -101,9 +101,8 @@ void Printer::Print()
     LogTrace(fmt::format("Expecting to receive {} byte(s) for printing", length));
 
     if (length > GetController()->GetBuffer().size()) {
-        LogError(
-            fmt::format("Transfer buffer overflow: Buffer size is {0} bytes, {1} byte(s) expected",
-                GetController()->GetBuffer().size(), length));
+        LogError(fmt::format("Transfer buffer overflow: Buffer size is {0} bytes, {1} byte(s) expected",
+            GetController()->GetBuffer().size(), length));
 
         ++print_error_count;
 
@@ -134,9 +133,8 @@ void Printer::SynchronizeBuffer()
     cmd.replace(file_position, 2, filename);
 
     error_code error;
-    LogTrace(fmt::format("Printing file '{0}' with {1} byte(s)", filename, file_size(path(filename), error)));
-
-    LogDebug(fmt::format("Executing print command '{}'", cmd));
+    LogTrace(fmt::format("Printing file '{0}' with {1} byte(s) using print command '{2}'", filename,
+        file_size(path(filename), error), cmd));
 
     if (system(cmd.c_str())) {
         LogError(fmt::format("Printing file '{}' failed, the Pi's printing system might not be configured", filename));
@@ -155,10 +153,9 @@ void Printer::SynchronizeBuffer()
 
 int Printer::WriteData(cdb_t cdb, data_out_t buf, int, int l)
 {
-    const auto command = static_cast<ScsiCommand>(cdb[0]);
-    assert(command == ScsiCommand::PRINT);
-    if (command != ScsiCommand::PRINT) {
-        throw ScsiException(SenseKey::ABORTED_COMMAND);
+    if (cdb[0] != static_cast<int>(ScsiCommand::PRINT)) {
+        assert(false);
+        throw ScsiException(SenseKey::ABORTED_COMMAND, Asc::INTERNAL_TARGET_FAILURE);
     }
 
     const auto length = GetCdbInt24(2);

diff --git a/cpp/devices/sasi_hd.cpp b/cpp/devices/sasi_hd.cpp
@@ -32,7 +32,7 @@ void SasiHd::Inquiry()
 {
     // Byte 0 = 0: Direct access device
 
-    const array<uint8_t, 2> buf = { };
+    const array<const uint8_t, 2> buf = { };
     GetController()->CopyToBuffer(buf.data(), buf.size());
 
     DataInPhase(buf.size());
@@ -53,7 +53,7 @@ void SasiHd::RequestSense()
     }
 
     // Non-extended format
-    const array<uint8_t, 4> buf = { static_cast<uint8_t>(GetSenseKey()), static_cast<uint8_t>(GetLun() << 5) };
+    const array<const uint8_t, 4> buf = { static_cast<uint8_t>(GetSenseKey()), static_cast<uint8_t>(GetLun() << 5) };
     GetController()->CopyToBuffer(buf.data(), allocation_length);
 
     DataInPhase(buf.size());

diff --git a/cpp/initiator/initiator_executor.cpp b/cpp/initiator/initiator_executor.cpp
@@ -2,7 +2,7 @@
 //
 // SCSI2Pi, SCSI device emulator and SCSI tools for the Raspberry Pi
 //
-// Copyright (C) 2023-2024 Uwe Seimet
+// Copyright (C) 2023-2025 Uwe Seimet
 //
 //---------------------------------------------------------------------------
 
@@ -244,7 +244,7 @@ void InitiatorExecutor::DataOut(data_out_t buf, int &length)
         throw PhaseException("No more data for DATA OUT phase");
     }
 
-    initiator_logger.debug(fmt::format("Sending {0} byte(s):\n{1}", length, formatter.FormatBytes(buf, length)));
+    initiator_logger.debug("Sending {0} byte(s):\n{1}", length, formatter.FormatBytes(buf, length));
 
     byte_count = bus.SendHandShake(buf.data(), length);
     if (byte_count != length) {

diff --git a/cpp/shared/s2p_util.cpp b/cpp/shared/s2p_util.cpp
@@ -2,7 +2,7 @@
 //
 // SCSI2Pi, SCSI device emulator and SCSI tools for the Raspberry Pi
 //
-// Copyright (C) 2021-2024 Uwe Seimet
+// Copyright (C) 2021-2025 Uwe Seimet
 //
 //---------------------------------------------------------------------------
 
@@ -203,7 +203,7 @@ string s2p_util::Banner(string_view app)
         << "Version " << GetVersionString() << "\n"
         << "Copyright (C) 2016-2020 GIMONS\n"
         << "Copyright (C) 2020-2023 Contributors to the PiSCSI project\n"
-        << "Copyright (C) 2021-2024 Uwe Seimet\n";
+        << "Copyright (C) 2021-2025 Uwe Seimet\n";
 
     return s.str();
 }

diff --git a/cpp/shared/s2p_util.h b/cpp/shared/s2p_util.h
@@ -122,7 +122,8 @@ static const unordered_map<Asc, const char*> ASC_MAPPING = {
     { Asc::INTERNAL_TARGET_FAILURE, "INTERNAL TARGET FAILURE" },
     { Asc::COMMAND_PHASE_ERROR, "COMMAND PHASE ERROR" },
     { Asc::DATA_PHASE_ERROR, "DATA PHASE ERROR" },
-    { Asc::MEDIUM_LOAD_OR_EJECT_FAILED, "MEDIA LOAD OR EJECT FAILED" }
+    { Asc::MEDIUM_LOAD_OR_EJECT_FAILED, "MEDIA LOAD OR EJECT FAILED" },
+    { Asc::DATA_CURRENTLY_UNAVAILABLE, "DATA CURRENTLY UNAVAILALBLE" }
 };
 
 static const unordered_map<StatusCode, const char*> STATUS_MAPPING = {

diff --git a/cpp/shared/scsi.h b/cpp/shared/scsi.h
@@ -221,6 +221,7 @@ enum class Asc
     COMMAND_PHASE_ERROR = 0x4a,
     DATA_PHASE_ERROR = 0x4b,
     MEDIUM_LOAD_OR_EJECT_FAILED = 0x53,
+    DATA_CURRENTLY_UNAVAILABLE = 0x55
 };
 
 enum class Ascq

diff --git a/cpp/test/disk_test.cpp b/cpp/test/disk_test.cpp
@@ -661,7 +661,7 @@ TEST(DiskTest, WriteData)
 {
     MockDisk disk;
 
-    EXPECT_THAT([&] {disk.WriteData( vector {static_cast<int>(ScsiCommand::WRITE_6)}, {}, 0, 0);}, Throws<ScsiException>(AllOf(
+    EXPECT_THAT([&] {disk.WriteData( {}, {}, 0, 0);}, Throws<ScsiException>(AllOf(
                 Property(&ScsiException::get_sense_key, SenseKey::NOT_READY),
                 Property(&ScsiException::get_asc, Asc::MEDIUM_NOT_PRESENT)))) << "Disk is not ready";
 }

diff --git a/cpp/test/host_services_test.cpp b/cpp/test/host_services_test.cpp
@@ -117,9 +117,13 @@ TEST(HostServicesTest, ReceiveOperationResults)
     Dispatch(services, ScsiCommand::RECEIVE_OPERATION_RESULTS, SenseKey::ILLEGAL_REQUEST, Asc::INVALID_FIELD_IN_CDB,
         "Illegal format");
 
+    controller->SetCdbByte(1, 0b11000);
+    Dispatch(services, ScsiCommand::RECEIVE_OPERATION_RESULTS, SenseKey::ILLEGAL_REQUEST, Asc::INVALID_FIELD_IN_CDB,
+        "Illegal format");
+
     controller->SetCdbByte(1, 0b010);
-    Dispatch(services, ScsiCommand::RECEIVE_OPERATION_RESULTS, SenseKey::ABORTED_COMMAND,
-        Asc::INTERNAL_TARGET_FAILURE, "No matching initiator ID");
+    Dispatch(services, ScsiCommand::RECEIVE_OPERATION_RESULTS, SenseKey::ILLEGAL_REQUEST,
+        Asc::DATA_CURRENTLY_UNAVAILABLE, "No matching initiator ID");
 }
 
 TEST(HostServicesTest, ModeSense6)