From a1a860f4e7162317d0b41a69b0fa710faee6f600 Mon Sep 17 00:00:00 2001
From: Uwe Seimet <Uwe.Seimet@seimet.de>
Date: Wed, 15 Nov 2023 23:45:34 +0100
Subject: [PATCH] Refactoring

---
 cpp/scsiexec/phase_executor.h  |  1 +
 cpp/scsiexec/scsi_executor.cpp | 71 +++++++++++-----------------------
 cpp/scsiexec/scsi_executor.h   |  3 +-
 cpp/scsiexec/scsiexec_core.cpp | 25 +++++++-----
 4 files changed, 42 insertions(+), 58 deletions(-)

diff --git a/cpp/scsiexec/phase_executor.h b/cpp/scsiexec/phase_executor.h
index 58f4b0d6..24902493 100644
--- a/cpp/scsiexec/phase_executor.h
+++ b/cpp/scsiexec/phase_executor.h
@@ -10,6 +10,7 @@
 #pragma once
 
 #include "hal/bus.h"
+#include "generated/piscsi_interface.pb.h"
 #include <ctime>
 #include <stdexcept>
 #include <span>
diff --git a/cpp/scsiexec/scsi_executor.cpp b/cpp/scsiexec/scsi_executor.cpp
index d3eca133..f72769d9 100644
--- a/cpp/scsiexec/scsi_executor.cpp
+++ b/cpp/scsiexec/scsi_executor.cpp
@@ -9,28 +9,39 @@
 
 #include "shared/scsi.h"
 #include "scsiexec/scsi_executor.h"
-#include "generated/piscsi_interface.pb.h"
 #include <google/protobuf/util/json_util.h>
 #include <spdlog/spdlog.h>
 #include <array>
 #include <fstream>
-#include <iostream>
 #include <filesystem>
 
 using namespace std;
 using namespace filesystem;
+using namespace google::protobuf::util;
 using namespace spdlog;
 using namespace scsi_defs;
 using namespace piscsi_interface;
 
-bool ScsiExecutor::Execute(const string& input_filename, const string& output_filename, bool binary, string& result)
+bool ScsiExecutor::Execute(const string& filename, bool binary, PbResult& result, string& error)
 {
     int input_length = 0;
 
-    if (!binary) {
-        ifstream in(input_filename);
+    if (binary) {
+        ifstream in(filename, ios::binary);
         if (in.fail()) {
-            result = "Can't open JSON input file '" + input_filename + "': " + strerror(errno);
+            error = "Can't open binary input file '" + filename + "': " + strerror(errno);
+            return false;
+        }
+
+        input_length = file_size(filename);
+        vector<char> b(input_length);
+        in.read(b.data(), input_length);
+        memcpy(buffer.data(), b.data(), input_length);
+    }
+    else {
+        ifstream in(filename);
+        if (in.fail()) {
+            error = "Can't open JSON input file '" + filename + "': " + strerror(errno);
             return false;
         }
 
@@ -40,18 +51,6 @@ bool ScsiExecutor::Execute(const string& input_filename, const string& output_fi
         input_length = json.size();
         memcpy(buffer.data(), json.data(), input_length);
     }
-    else {
-        ifstream in(input_filename, ios::binary);
-        if (in.fail()) {
-            result = "Can't open binary input file '" + input_filename + "': " + strerror(errno);
-            return false;
-        }
-
-        input_length = file_size(input_filename);
-        vector<char> b(input_length);
-        in.read(b.data(), input_length);
-        memcpy(buffer.data(), b.data(), input_length);
-    }
 
     array<uint8_t, 10> cdb = { };
     cdb[1] = binary ? 0x0a : 0x05;
@@ -65,41 +64,17 @@ bool ScsiExecutor::Execute(const string& input_filename, const string& output_fi
     const int length = phase_executor->GetByteCount();
 
     if (binary) {
-        PbResult r;
-        if (!r.ParseFromArray(buffer.data(), length)) {
-            result = "Can't parse received binary protobuf data";
+        if (!result.ParseFromArray(buffer.data(), length)) {
+            error = "Can't parse received binary protobuf data";
             return false;
         }
-
-        if (output_filename.empty()) {
-            google::protobuf::util::MessageToJsonString(r, &result);
-        }
-        else {
-            if (binary) {
-                 ofstream out(output_filename, ios::binary);
-                 if (out.fail()) {
-                     result = "Can't open binary output file '" + output_filename + "'";
-                     return false;
-                 }
-            }
-            else {
-                ofstream out(output_filename);
-                if (out.fail()) {
-                    result = "Can't open JSON output file '" + output_filename + "'";
-                    return false;
-                }
-            }
-        }
     }
     else {
         const string json((const char*) buffer.data(), length);
-
-        if (output_filename.empty()) {
-            result = json;
-        }
-        else {
-
-        }
+         if (!JsonStringToMessage(json, &result).ok()) {
+             error = "Can't parse received JSON protobuf data";
+             return false;
+         }
     }
 
     return true;
diff --git a/cpp/scsiexec/scsi_executor.h b/cpp/scsiexec/scsi_executor.h
index 006ddd6b..006fbbf1 100644
--- a/cpp/scsiexec/scsi_executor.h
+++ b/cpp/scsiexec/scsi_executor.h
@@ -15,6 +15,7 @@
 #include <string>
 
 using namespace std;
+using namespace piscsi_interface;
 
 class ScsiExecutor
 {
@@ -29,7 +30,7 @@ public:
     }
     ~ScsiExecutor() = default;
 
-    bool Execute(const string&, const string&, bool, string&);
+    bool Execute(const string&, bool, PbResult&, string&);
     bool ShutDown();
 
     void SetTarget(int id, int lun)
diff --git a/cpp/scsiexec/scsiexec_core.cpp b/cpp/scsiexec/scsiexec_core.cpp
index dc7df6b2..fee28765 100644
--- a/cpp/scsiexec/scsiexec_core.cpp
+++ b/cpp/scsiexec/scsiexec_core.cpp
@@ -189,18 +189,25 @@ int ScsiExec::run(span<char*> args, bool in_process)
     bool status = false;
     if (shut_down) {
         status = scsi_executor->ShutDown();
+
+        CleanUp();
+
+        return status ? EXIT_SUCCESS : EXIT_FAILURE;
     }
-    else {
-        string result;
-        status = scsi_executor->Execute(input_filename, output_filename, binary, result);
-        if (status) {
-            cout << result << '\n' << flush;
-        }
-        else {
-            cerr << "Error: " << result << endl;
-        }
+
+    PbResult result;
+    string error;
+    status = scsi_executor->Execute(input_filename, binary, result, error);
+    if (!status) {
+        cerr << "Error: " << error << endl;
+
+        CleanUp();
+
+        return status ? EXIT_SUCCESS : EXIT_FAILURE;
     }
 
+    // TODO File output
+
     CleanUp();
 
     return status ? EXIT_SUCCESS : EXIT_FAILURE;