From f14c850e5979c57f2eae607a5b684a68387e4fb4 Mon Sep 17 00:00:00 2001 From: Uwe Seimet Date: Sat, 4 Sep 2021 15:30:36 +0200 Subject: [PATCH] Code cleanup --- src/raspberrypi/rascsi.cpp | 96 ++++++++++++++------------ src/raspberrypi/rascsi_interface.proto | 19 ++--- src/raspberrypi/rasctl.cpp | 10 ++- src/raspberrypi/rasutil.cpp | 6 +- src/raspberrypi/rasutil.h | 3 +- 5 files changed, 73 insertions(+), 61 deletions(-) diff --git a/src/raspberrypi/rascsi.cpp b/src/raspberrypi/rascsi.cpp index 2f773a3d..26db02bc 100644 --- a/src/raspberrypi/rascsi.cpp +++ b/src/raspberrypi/rascsi.cpp @@ -607,50 +607,58 @@ void GetAvailableImages(PbServerInfo& server_info) void GetDeviceInfo(const PbCommand& command, PbResult& result) { - PbDevices *pb_devices = new PbDevices(); - - for (const auto& pb_device_information : command.devices()) { - PbDevice *pb_device = pb_devices->add_devices(); - - for (size_t i = 0; i < devices.size(); i++) { - Device *device = devices[i]; + set id_sets; + if (command.devices_size() == 0) { + for (const Device *device : devices) { if (device) { - if (device->GetId() == pb_device_information.id() && device->GetLun() == pb_device_information.unit()) { - GetDevice(device, pb_device); - } - else { - delete pb_devices; - - ostringstream error; - error << "No device for ID " << pb_device_information.id() << ", unit " << pb_device_information.unit(); - result.set_msg(error.str()); - - return; - } - - break; + id_sets.insert(make_pair(device->GetId(), device->GetLun())); + } + } + } + else { + for (const auto& device : command.devices()) { + if (devices[device.id() * UnitNum + device.unit()]) { + id_sets.insert(make_pair(device.id(), device.unit())); + } + else { + ostringstream error; + error << "No device for ID " << device.id() << ", unit " << device.unit(); + result.set_status(false); + result.set_msg(error.str()); + return; } } } - result.set_status(true); + PbDevices *pb_devices = new PbDevices(); + + for (const auto& id_set : id_sets) { + Device *device = devices[id_set.first * UnitNum + id_set.second]; + PbDevice *pb_device = pb_devices->add_devices(); + GetDevice(device, pb_device); + } + result.set_allocated_device_info(pb_devices); } -void GetServerInfo(PbServerInfo& server_info) +void GetServerInfo(PbResult& result) { - server_info.set_major_version(rascsi_major_version); - server_info.set_minor_version(rascsi_minor_version); - server_info.set_patch_version(rascsi_patch_version); - GetLogLevels(server_info); - server_info.set_current_log_level(current_log_level); - server_info.set_default_image_folder(default_image_folder); - GetDeviceTypeFeatures(server_info); - GetAvailableImages(server_info); - GetDevices(server_info); + PbServerInfo *server_info = new PbServerInfo(); + + server_info->set_major_version(rascsi_major_version); + server_info->set_minor_version(rascsi_minor_version); + server_info->set_patch_version(rascsi_patch_version); + GetLogLevels(*server_info); + server_info->set_current_log_level(current_log_level); + server_info->set_default_image_folder(default_image_folder); + GetDeviceTypeFeatures(*server_info); + GetAvailableImages(*server_info); + GetDevices(*server_info); for (int id : reserved_ids) { - server_info.add_reserved_ids(id); + server_info->add_reserved_ids(id); } + + result.set_allocated_server_info(server_info); } bool SetDefaultImageFolder(const string& f) @@ -1270,7 +1278,8 @@ bool ParseArgument(int argc, char* argv[], int& port) // Display and log the device list PbServerInfo server_info; GetDevices(server_info); - const string device_list = ListDevices(server_info); + const list& devices = { server_info.devices().begin(), server_info.devices().end() }; + const string device_list = ListDevices(devices); LogDevices(device_list); cout << device_list << endl; @@ -1367,25 +1376,20 @@ static void *MonThread(void *param) } case DEVICE_INFO: { - if (command.devices().empty()) { - ReturnStatus(fd, false, "Can't get device information: Missing device IDs"); - } - else { - PbResult result; - GetDeviceInfo(command, result); - SerializeMessage(fd, result); - } + PbResult result; + result.set_status(true); + GetDeviceInfo(command, result); + SerializeMessage(fd, result); + const list& devices ={ result.device_info().devices().begin(), result.device_info().devices().end() }; + LogDevices(ListDevices(devices)); break; } case SERVER_INFO: { - PbServerInfo *server_info = new PbServerInfo(); - GetServerInfo(*server_info); PbResult result; result.set_status(true); - result.set_allocated_server_info(server_info); + GetServerInfo(result); SerializeMessage(fd, result); - LogDevices(ListDevices(*server_info)); break; } diff --git a/src/raspberrypi/rascsi_interface.proto b/src/raspberrypi/rascsi_interface.proto index 4ffec3c2..5d297df8 100644 --- a/src/raspberrypi/rascsi_interface.proto +++ b/src/raspberrypi/rascsi_interface.proto @@ -1,6 +1,7 @@ // // Each rascsi remote interface message is preceded by a little endian 32 bit header, // which contains the protobuf message size. +// The order of repeated data returned is undefined. // syntax = "proto3"; @@ -26,20 +27,20 @@ enum PbDeviceType { SCDP = 7; } -// rascsi remote operations. PbResult is returned, except for SERVER_INFO and DEVICE_INFO. +// rascsi remote operations, returns PbResult enum PbOperation { NONE = 0; // Gets the server information SERVER_INFO = 1; - // Gets information for a list of devices + // Gets information for a list of attached devices. Returns data for all attached devices if empty. DEVICE_INFO = 2; - // Set the default folder for image files, PbCommand.params contains the folder name + // Set the default folder for image files. PbCommand.params contains the folder name. DEFAULT_FOLDER = 3; - // Set server log level, PbCommand.params contains the log level + // Set server log level. PbCommand.params contains the log level. LOG_LEVEL = 4; - // Attach new device + // Attach devices ATTACH = 5; - // Detach device + // Detach devices DETACH = 6; // Detach all devices, does not require a device list DETACH_ALL = 7; @@ -51,8 +52,8 @@ enum PbOperation { PROTECT = 10; // Make medium writable (not possible for read-only media) UNPROTECT = 11; - // IDs blocked from being used, usually the ID of the initiator (computer) in the SCSI chain. - // The command params field contains the full list of IDs to reserve, or is empty in order to allow all IDs. + // IDs blocked from being used, usually the IDs of the initiators (computers) in the SCSI chain. + // PbCommand.params contains the list of IDs to reserve, or is empty in order to allow all IDs. RESERVE = 12; } @@ -149,7 +150,7 @@ message PbCommand { message PbResult { // false means that an error occurred bool status = 1; - // An optional error or information message, depending on the result status + // An optional error or information message, depending on the status. A one-line string without CR/LF. string msg = 2; // Optional additional result data oneof result { diff --git a/src/raspberrypi/rasctl.cpp b/src/raspberrypi/rasctl.cpp index 811517a1..4a043d91 100644 --- a/src/raspberrypi/rasctl.cpp +++ b/src/raspberrypi/rasctl.cpp @@ -122,8 +122,14 @@ const PbServerInfo GetServerInfo(const string& hostname, int port) void CommandList(const string& hostname, int port) { - PbServerInfo serverInfo = GetServerInfo(hostname, port); - cout << ListDevices(serverInfo) << endl; + PbCommand command; + command.set_operation(DEVICE_INFO); + + PbResult result; + SendCommand(hostname.c_str(), port, command, result); + + const list& devices = { result.device_info().devices().begin(), result.device_info().devices().end() }; + cout << ListDevices(devices) << endl; } void CommandLogLevel(const string& hostname, int port, const string& log_level) diff --git a/src/raspberrypi/rasutil.cpp b/src/raspberrypi/rasutil.cpp index 04c4767d..e1cbc09d 100644 --- a/src/raspberrypi/rasutil.cpp +++ b/src/raspberrypi/rasutil.cpp @@ -34,9 +34,9 @@ bool GetAsInt(const string& value, int& result) return true; } -string ListDevices(const PbServerInfo& server_info) +string ListDevices(const list& pb_devices) { - if (!server_info.devices_size()) { + if (pb_devices.empty()) { return "No images currently attached."; } @@ -45,7 +45,7 @@ string ListDevices(const PbServerInfo& server_info) << "| ID | UN | TYPE | DEVICE STATUS" << endl << "+----+----+------+-------------------------------------" << endl; - list devices = { server_info.devices().begin(), server_info.devices().end() }; + list devices = pb_devices; devices.sort([](const auto& a, const auto& b) { return a.id() < b.id() && a.unit() < b.unit(); }); for (const auto& device : devices) { diff --git a/src/raspberrypi/rasutil.h b/src/raspberrypi/rasutil.h index c3441e0a..4d9f2788 100644 --- a/src/raspberrypi/rasutil.h +++ b/src/raspberrypi/rasutil.h @@ -11,8 +11,9 @@ #pragma once +#include #include #include "rascsi_interface.pb.h" bool GetAsInt(const std::string&, int&); -std::string ListDevices(const rascsi_interface::PbServerInfo&); +std::string ListDevices(const std::list&);