Code cleanup

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();
+	set<id_set> id_sets;
-
+	if (command.devices_size() == 0) {
-	for (const auto& pb_device_information : command.devices()) {
+		for (const Device *device : devices) {
-		PbDevice *pb_device = pb_devices->add_devices();
-
-		for (size_t i = 0; i < devices.size(); i++) {
-			Device *device = devices[i];
 			if (device) {
-				if (device->GetId() == pb_device_information.id() && device->GetLun() == pb_device_information.unit()) {
+				id_sets.insert(make_pair(device->GetId(), device->GetLun()));
-					GetDevice(device, pb_device);
+			}
-				}
+		}
-				else {
+	}
-					delete pb_devices;
+	else {
-
+		for (const auto& device : command.devices()) {
-					ostringstream error;
+			if (devices[device.id() * UnitNum + device.unit()]) {
-					error << "No device for ID " << pb_device_information.id() << ", unit " << pb_device_information.unit();
+				id_sets.insert(make_pair(device.id(), device.unit()));
-					result.set_msg(error.str());
+			}
-
+			else {
-					return;
+				ostringstream error;
-				}
+				error << "No device for ID " << device.id() << ", unit " << device.unit();
-
+				result.set_status(false);
-				break;
+				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);
+	PbServerInfo *server_info = new PbServerInfo();
-	server_info.set_minor_version(rascsi_minor_version);
+
-	server_info.set_patch_version(rascsi_patch_version);
+	server_info->set_major_version(rascsi_major_version);
-	GetLogLevels(server_info);
+	server_info->set_minor_version(rascsi_minor_version);
-	server_info.set_current_log_level(current_log_level);
+	server_info->set_patch_version(rascsi_patch_version);
-	server_info.set_default_image_folder(default_image_folder);
+	GetLogLevels(*server_info);
-	GetDeviceTypeFeatures(server_info);
+	server_info->set_current_log_level(current_log_level);
-	GetAvailableImages(server_info);
+	server_info->set_default_image_folder(default_image_folder);
-	GetDevices(server_info);
+	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<PbDevice>& 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()) {
+					PbResult result;
-						ReturnStatus(fd, false, "Can't get device information: Missing device IDs");
+					result.set_status(true);
-					}
+					GetDeviceInfo(command, result);
-					else {
+					SerializeMessage(fd, result);
-						PbResult result;
+					const list<PbDevice>& devices ={ result.device_info().devices().begin(), result.device_info().devices().end() };
-						GetDeviceInfo(command, result);
+					LogDevices(ListDevices(devices));
-						SerializeMessage(fd, result);
-					}
 					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;
 				}
 

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.
+    // IDs blocked from being used, usually the IDs of the initiators (computers) 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.
+    // 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 {

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);
+	PbCommand command;
-	cout << ListDevices(serverInfo) << endl;
+	command.set_operation(DEVICE_INFO);
+
+	PbResult result;
+	SendCommand(hostname.c_str(), port, command, result);
+
+	const list<PbDevice>& 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)

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<PbDevice>& 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<PbDevice> devices = { server_info.devices().begin(), server_info.devices().end() };
+	list<PbDevice> 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) {

src/raspberrypi/rasutil.h

@@ -11,8 +11,9 @@
 
 #pragma once
 
+#include <list>
 #include <string>
 #include "rascsi_interface.pb.h"
 
 bool GetAsInt(const std::string&, int&);
-std::string ListDevices(const rascsi_interface::PbServerInfo&);
+std::string ListDevices(const std::list<rascsi_interface::PbDevice>&);