Do not abort but only warn on unexpected CD-ROM image size (#1084)

* Do not abort but only warn on unexpected CD-ROM image size * Unit test update * Updated exception handling * Cleanup * Fixed typo
PiSCSI · Jan 28, 2023 · b6941c9 · b6941c9
1 parent 139a6ec
commit b6941c9
Show file tree

Hide file tree

Showing 4 changed files with 9 additions and 30 deletions.
diff --git a/cpp/devices/scsicd.cpp b/cpp/devices/scsicd.cpp
@@ -10,8 +10,6 @@
 //	Licensed under the BSD 3-Clause License.
 //	See LICENSE file in the project root folder.
 //
-//	[ SCSI CD-ROM ]
-//
 //---------------------------------------------------------------------------
 
 #include "shared/piscsi_exceptions.h"
@@ -79,7 +77,6 @@ void SCSICD::Open()
 	SetReadOnly(true);
 	SetProtectable(false);
 
-	// Attention if ready
 	if (IsReady()) {
 		SetAttn(true);
 	}
@@ -113,13 +110,12 @@ void SCSICD::OpenIso()
 			throw io_exception("Illegal raw ISO CD-ROM file header");
 		}
 
-		// Set to RAW file
 		rawfile = true;
 	}
 
 	if (rawfile) {
 		if (size % 2536) {
-			throw io_exception("Raw ISO CD-ROM file size must be a multiple of 2536 bytes but is "
+			GetLogger().Warn("Raw ISO CD-ROM file size is not a multiple of 2536 bytes but is "
 					+ to_string(size) + " bytes");
 		}
 
@@ -134,7 +130,6 @@ void SCSICD::OpenIso()
 // TODO This code is only executed if the filename starts with a `\`, but fails to open files starting with `\`.
 void SCSICD::OpenPhysical()
 {
-	// Get size
 	off_t size = GetFileSize();
 	if (size < 2048) {
 		throw io_exception("CD-ROM file size must be at least 2048 bytes");
@@ -176,12 +171,10 @@ void SCSICD::SetUpModePages(map<int, vector<byte>>& pages, int page, bool change
 {
 	Disk::SetUpModePages(pages, page, changeable);
 
-	// Page code 13
 	if (page == 0x0d || page == 0x3f) {
 		AddCDROMPage(pages, changeable);
 	}
 
-	// Page code 14
 	if (page == 0x0e || page == 0x3f) {
 		AddCDDAPage(pages, changeable);
 	}
@@ -226,10 +219,7 @@ int SCSICD::Read(const vector<int>& cdb, vector<uint8_t>& buf, uint64_t block)
 {
 	CheckReady();
 
-	// Search for the track
 	const int index = SearchTrack(static_cast<int>(block));
-
-	// If invalid, out of range
 	if (index < 0) {
 		throw scsi_exception(sense_key::ILLEGAL_REQUEST, asc::LBA_OUT_OF_RANGE);
 	}
@@ -249,7 +239,6 @@ int SCSICD::Read(const vector<int>& cdb, vector<uint8_t>& buf, uint64_t block)
 		dataindex = index;
 	}
 
-	// Base class
 	assert(dataindex >= 0);
 	return Disk::Read(cdb, buf, block);
 }
@@ -321,7 +310,6 @@ int SCSICD::ReadTocInternal(const vector<int>& cdb, vector<uint8_t>& buf)
 
 	int offset = 4;
 
-	// Loop....
 	for (int i = 0; i < loop; i++) {
 		// ADR and Control
 		if (tracks[index]->IsAudio()) {
@@ -351,11 +339,6 @@ int SCSICD::ReadTocInternal(const vector<int>& cdb, vector<uint8_t>& buf)
 	return length;
 }
 
-//---------------------------------------------------------------------------
-//
-//	LBA→MSF Conversion
-//
-//---------------------------------------------------------------------------
 void SCSICD::LBAtoMSF(uint32_t lba, uint8_t *msf) const
 {
 	// 75 and 75*60 get the remainder
@@ -390,12 +373,6 @@ void SCSICD::ClearTrack()
 	audioindex = -1;
 }
 
-//---------------------------------------------------------------------------
-//
-//	Track Search
-//	* Returns -1 if not found
-//
-//---------------------------------------------------------------------------
 int SCSICD::SearchTrack(uint32_t lba) const
 {
 	// Track loop

diff --git a/cpp/piscsi/piscsi_executor.cpp b/cpp/piscsi/piscsi_executor.cpp
@@ -599,7 +599,12 @@ bool PiscsiExecutor::ValidateImageFile(const CommandContext& context, StorageDev
 		storage_device.SetProtectable(true);
 	}
 
-	storage_device.Open();
+	try {
+		storage_device.Open();
+	}
+	catch(const io_exception&) {
+		return context.ReturnLocalizedError(LocalizationKey::ERROR_FILE_OPEN, effective_filename);
+	}
 
 	full_path = effective_filename;
 

diff --git a/cpp/test/piscsi_executor_test.cpp b/cpp/test/piscsi_executor_test.cpp
@@ -278,7 +278,7 @@ TEST_F(PiscsiExecutorTest, Attach)
 
 	path filename = CreateTempFile(1);
 	SetParam(definition, "file", filename.c_str());
-	EXPECT_THROW(executor.Attach(context, definition, false), io_exception)	<< "Too small image file not rejected";
+	EXPECT_FALSE(executor.Attach(context, definition, false)) << "Too small image file not rejected";
 	remove(filename);
 
 	filename = CreateTempFile(512);
@@ -359,8 +359,7 @@ TEST_F(PiscsiExecutorTest, Insert)
 
 	path filename = CreateTempFile(1);
 	SetParam(definition, "file", filename.c_str());
-	EXPECT_THROW(executor.Insert(context, definition, device, false), io_exception)
-		<< "Too small image file not rejected";
+	EXPECT_FALSE(executor.Insert(context, definition, device, false)) << "Too small image file not rejected";
 	remove(filename);
 
 	filename = CreateTempFile(512);

diff --git a/cpp/test/scsicd_test.cpp b/cpp/test/scsicd_test.cpp
@@ -95,9 +95,7 @@ TEST(ScsiCdTest, Open)
 	out.open(filename);
 	out.write(header.data(), header.size());
 	out.close();
-	resize_file(filename, 2 * 2535);
 	cd_raw.SetFilename(string(filename));
-	EXPECT_THROW(cd_raw.Open(), io_exception) << "Raw ISO CD-ROM image file size must be a multiple of 2536";
 	resize_file(filename, 2 * 2536);
 	cd_raw.Open();
 	EXPECT_EQ(2, cd_raw.GetBlockCount());