updated smells

cpp/disk_image/disk_image_handle.cpp

@@ -3,7 +3,7 @@
 //	SCSI Target Emulator RaSCSI (*^..^*)
 //	for Raspberry Pi
 //
-//	Copyright (C) 2022 akuker
+//	Copyright (C) 2022-2023 akuker
 //
 //  	Base class for interfacing with disk images.
 //
@@ -12,29 +12,25 @@
 //---------------------------------------------------------------------------
 
 #include <cassert>
+#include <string_view>
 #include "disk_image/disk_image_handle.h"
 
-DiskImageHandle::DiskImageHandle(const string &path, int size, uint32_t blocks, off_t imgoff)
+DiskImageHandle::DiskImageHandle(string_view path, int size, uint32_t blocks, off_t imgoff)
-{
+	: sec_path(path), sec_size(size), sec_blocks(blocks), imgoffset(imgoff)
-
-	serial = 0;
-	sec_path = path;
-	sec_size = size;
-	sec_blocks = blocks;
-	imgoffset = imgoff;
-}
-DiskImageHandle::~DiskImageHandle()
 {
+	assert(blocks > 0);
+	assert(imgoff >= 0);
+	assert(sec_size > 0);
 }
 
-off_t DiskImageHandle::GetSectorOffset(int block)
+off_t DiskImageHandle::GetSectorOffset(int block) const
 {
 
 	int sector_num = block & 0xff;
 	return (off_t)sector_num << sec_size;
 }
 
-off_t DiskImageHandle::GetTrackOffset(int block)
+off_t DiskImageHandle::GetTrackOffset(int block) const
 {
 
 	// Assuming that all tracks hold 256 sectors

cpp/disk_image/disk_image_handle.h

@@ -3,7 +3,7 @@
 //	SCSI Target Emulator RaSCSI (*^..^*)
 //	for Raspberry Pi
 //
-//	Copyright (C) 2022 akuker
+//	Copyright (C) 2022-2023 akuker
 //
 //  	Base class for interfacing with disk images.
 //
@@ -13,20 +13,21 @@
 
 #pragma once
 
-// #include "filepath.h"
 #include <string>
 #include <cstdint>
 #include <vector>
+#include <string_view>
 
 using namespace std;
 
 class DiskImageHandle
 {
 public:
-	DiskImageHandle(const string &path, int size, uint32_t blocks, off_t imgoff = 0);
+	DiskImageHandle(string_view path, int size, uint32_t blocks, off_t imgoff = 0);
-	virtual ~DiskImageHandle();
+	virtual ~DiskImageHandle() = default;
 
 	void SetRawMode(bool raw) { cd_raw = raw; }; // CD-ROM raw mode setting
+	bool GetRawMode() const {return cd_raw;}
 
 	// Access
 	virtual bool Save() = 0;											   // Save and release all
@@ -35,14 +36,19 @@ public:
 	virtual bool GetCache(int index, int &track, uint32_t &serial) const = 0; // Get cache information
 
 protected:
+	int GetSectorSize() const {return sec_size;}
+	int GetBlocksPerSector() const { return sec_blocks;}
+	string GetPath() const {return sec_path;}
+	off_t GetImgOffset() const {return imgoffset;}
+
+	off_t GetTrackOffset(int block) const;
+	off_t GetSectorOffset(int block) const;
+
+private:
 	bool cd_raw = false;
-	uint32_t serial;	   // Last serial number
 	string sec_path; // Path
 
 	int sec_size;	 // Sector Size (8=256, 9=512, 10=1024, 11=2048, 12=4096)
 	int sec_blocks;	 // Blocks per sector
 	off_t imgoffset; // Offset to actual data
-
-	off_t GetTrackOffset(int block);
-	off_t GetSectorOffset(int block);
 };

cpp/disk_image/disk_image_handle_factory.cpp

@@ -3,7 +3,7 @@
 //	SCSI Target Emulator RaSCSI (*^..^*)
 //	for Raspberry Pi
 //
-//	Copyright (C) 2022 akuker
+//	Copyright (C) 2022-2023 akuker
 //
 //  	Factory class for creating DiskImageHandles
 //
@@ -42,7 +42,7 @@ unique_ptr<DiskImageHandle> DiskImageHandleFactory::CreateDiskImageHandle(const
 
     if (result == nullptr)
     {
-        LOGWARN("%s Unable to create the File Access", __PRETTY_FUNCTION__);
+        LOGWARN("%s Unable to create the File Access", __PRETTY_FUNCTION__)
     }
     return result;
 }

cpp/disk_image/disk_image_handle_factory.h

@@ -16,7 +16,7 @@
 #include "disk_image/disk_image_handle.h"
 #include <memory>
 
-enum DiskImageHandleType
+enum class DiskImageHandleType
 {
 	eRamCache,
 	eMmapFile,

cpp/disk_image/disk_track_cache.cpp

@@ -4,6 +4,7 @@
 //
 //	Copyright (C) 2001-2006 ＰＩ．(ytanaka@ipc-tokai.or.jp)
 //	Copyright (C) 2014-2020 GIMONS
+//  Copyright (C) 2022-2023 akuker
 //
 //	XM6i
 //	Copyright (C) 2010-2015 isaki@NetBSD.org
@@ -280,7 +281,7 @@ bool DiskTrack::ReadSector(vector<uint8_t>& buf, int sec) const
 {
 	assert((sec >= 0) & (sec < 0x100));
 
-	LOGTRACE("%s reading sector: %d", __PRETTY_FUNCTION__,sec);
+	LOGTRACE("%s reading sector: %d", __PRETTY_FUNCTION__,sec)
 	// Error if not initialized
 	if (!dt.init) {
 		return false;
@@ -368,7 +369,7 @@ bool DiskCache::Save()
 		// Is it a valid track?
 		if (cache[i].disktrk) {
 			// Save
-			if (!cache[i].disktrk->Save(sec_path)) {
+			if (!cache[i].disktrk->Save(GetPath())) {
 				return false;
 			}
 		}
@@ -411,8 +412,6 @@ void DiskCache::Clear()
 
 bool DiskCache::ReadSector(vector<uint8_t>& buf, int block)
 {
-	assert(sec_size != 0);
-
 	// Update first
 	UpdateSerialNumber();
 
@@ -431,8 +430,6 @@ bool DiskCache::ReadSector(vector<uint8_t>& buf, int block)
 
 bool DiskCache::WriteSector(const vector<uint8_t>& buf, int block)
 {
-	assert(sec_size != 0);
-
 	// Update first
 	UpdateSerialNumber();
 
@@ -456,7 +453,6 @@ bool DiskCache::WriteSector(const vector<uint8_t>& buf, int block)
 //---------------------------------------------------------------------------
 DiskTrack* DiskCache::Assign(int track)
 {
-	assert(sec_size != 0);
 	assert(track >= 0);
 
 	// First, check if it is already assigned
@@ -503,7 +499,7 @@ DiskTrack* DiskCache::Assign(int track)
 	}
 
 	// Save this track
-	if (!cache[c].disktrk->Save(sec_path)) {
+	if (!cache[c].disktrk->Save(GetPath())) {
 		return NULL;
 	}
 
@@ -533,7 +529,7 @@ bool DiskCache::Load(int index, int track, DiskTrack *disktrk)
 	assert(!cache[index].disktrk);
 
 	// Get the number of sectors on this track
-	int sectors = sec_blocks - (track << 8);
+	int sectors = GetBlocksPerSector() - (track << 8);
 	assert(sectors > 0);
 	if (sectors > 0x100) {
 		sectors = 0x100;
@@ -545,10 +541,10 @@ bool DiskCache::Load(int index, int track, DiskTrack *disktrk)
 	}
 
 	// Initialize disk track
-	disktrk->Init(track, sec_size, sectors, cd_raw, imgoffset);
+	disktrk->Init(track, GetSectorSize(), GetBlocksPerSector(), GetRawMode(), GetImgOffset());
 
 	// Try loading
-	if (!disktrk->Load(sec_path)) {
+	if (!disktrk->Load(GetPath())) {
 		// Failure
 		delete disktrk;
 		return false;

cpp/disk_image/disk_track_cache.h

@@ -4,6 +4,7 @@
 //
 //	Copyright (C) 2001-2006 ＰＩ．(ytanaka@ipc-tokai.or.jp)
 //	Copyright (C) 2014-2020 GIMONS
+//  Copyright (C) 2022-2023 akuker
 //
 //	XM6i
 //	Copyright (C) 2010-2015 isaki@NetBSD.org
@@ -17,76 +18,80 @@
 
 #pragma once
 
-#include <string>
 #include "disk_image/disk_image_handle.h"
+#include <string>
 
 using namespace std;
 
-// Number of tracks to cache
-#define CacheMax 16
-
 class DiskTrack
 {
-private:
+  private:
-	 struct {
+    struct {
-		int track;							// Track Number
+        int track;       // Track Number
-		int size;							// Sector Size (8=256, 9=512, 10=1024, 11=2048, 12=4096)
+        int size;        // Sector Size (8=256, 9=512, 10=1024, 11=2048, 12=4096)
-		int sectors;							// Number of sectors(<0x100)
+        int sectors;     // Number of sectors(<0x100)
-		uint32_t length;							// Data buffer length
+        uint32_t length; // Data buffer length
-		uint8_t *buffer;							// Data buffer
+        uint8_t* buffer; // Data buffer
-		bool init;							// Is it initilized?
+        bool init;       // Is it initilized?
-		bool changed;							// Changed flag
+        bool changed;    // Changed flag
-		uint32_t maplen;							// Changed map length
+        uint32_t maplen; // Changed map length
-		bool *changemap;						// Changed map
+        bool* changemap; // Changed map
-		bool raw;							// RAW mode flag
+        bool raw;        // RAW mode flag
-		off_t imgoffset;						// Offset to actual data
+        off_t imgoffset; // Offset to actual data
-	} dt;
+    } dt;
 
-public:
+  public:
-	DiskTrack();
+    DiskTrack();
-	~DiskTrack();
+    ~DiskTrack();
 
-private:
+  private:
-	friend class DiskCache;
+    friend class DiskCache;
 
-	void Init(int track, int size, int sectors, bool raw = false, off_t imgoff = 0) ;
+    void Init(int track, int size, int sectors, bool raw = false, off_t imgoff = 0);
-	bool Load(const string& path) ;
+    bool Load(const string& path);
-	bool Save(const string& path) ;
+    bool Save(const string& path);
 
-	// Read / Write
+    // Read / Write
-	bool ReadSector(vector<uint8_t>& buf, int sec) const;				// Sector Read
+    bool ReadSector(vector<uint8_t>& buf, int sec) const;  // Sector Read
-	bool WriteSector(const vector<uint8_t>& buf, int sec);				// Sector Write
+    bool WriteSector(const vector<uint8_t>& buf, int sec); // Sector Write
 
-	int GetTrack() const		{ return dt.track; }		// Get track
+    // Get track
+    int GetTrack() const
+    {
+        return dt.track;
+    }
 };
 
 class DiskCache : public DiskImageHandle
 {
-public:
+  public:
-	// Internal data definition
+    // Internal data definition
-	typedef struct {
+    typedef struct {
-		DiskTrack *disktrk;						// Disk Track
+        DiskTrack* disktrk; // Disk Track
-		uint32_t serial;							// Serial
+        uint32_t serial;    // Serial
-	} cache_t;
+    } cache_t;
 
-public:
+    DiskCache(const string& path, int size, uint32_t blocks, off_t imgoff = 0);
-	DiskCache(const string& path, int size, uint32_t blocks, off_t imgoff = 0);
+    ~DiskCache();
-	~DiskCache();
 
-	// Access
+    // Access
-	bool Save() override;							// Save and release all
+    bool Save() override;                                                  // Save and release all
-	bool ReadSector(vector<uint8_t>& buf, int block) override;				// Sector Read
+    bool ReadSector(vector<uint8_t>& buf, int block) override;             // Sector Read
-	bool WriteSector(const vector<uint8_t>& buf, int block) override;			// Sector Write
+    bool WriteSector(const vector<uint8_t>& buf, int block) override;      // Sector Write
-	bool GetCache(int index, int& track, uint32_t& serial) const override;	// Get cache information
+    bool GetCache(int index, int& track, uint32_t& serial) const override; // Get cache information
 
-private:
+  private:
-	// Internal Management
+    // Number of tracks to cache
-	void Clear();							// Clear all tracks
+    static const int CacheMax = 16;
-	DiskTrack* Assign(int track);					// Load track
-	bool Load(int index, int track, DiskTrack *disktrk = NULL);	// Load track
-	void UpdateSerialNumber();							// Update serial number
 
-	// Internal data
+    // Internal Management
-	cache_t cache[CacheMax];						// Cache management
+    void Clear();                                                  // Clear all tracks
-};
+    DiskTrack* Assign(int track);                                  // Load track
+    bool Load(int index, int track, DiskTrack* disktrk = nullptr); // Load track
+    void UpdateSerialNumber();                                     // Update serial number
 
+    // Internal data
+    array<cache_t, CacheMax> cache; // Cache management
+
+    uint32_t serial; // Last serial number
+};

cpp/disk_image/mmap_file_handle.cpp

@@ -3,7 +3,7 @@
 //	SCSI Target Emulator RaSCSI (*^..^*)
 //	for Raspberry Pi
 //
-//	Copyright (C) 2022 akuker
+//	Copyright (C) 2022-2023 akuker
 //
 //  This method of file access will use the mmap() capabilities to 'map' the
 //  file into memory.
@@ -73,11 +73,10 @@ MmapFileHandle::~MmapFileHandle()
 
 bool MmapFileHandle::ReadSector(vector<uint8_t>& buf, int block)
 {
-	assert(sec_size != 0);
+	assert(block < GetBlocksPerSector());
-	assert(block < sec_blocks);
 	assert(memory_block);
 
-	int sector_size_bytes = (off_t)1 << sec_size;
+	int sector_size_bytes = (off_t)1 << GetSectorSize();
 
 	// Calculate offset into the image file
 	off_t offset = GetTrackOffset(block);
@@ -90,12 +89,12 @@ bool MmapFileHandle::ReadSector(vector<uint8_t>& buf, int block)
 
 bool MmapFileHandle::WriteSector(const vector<uint8_t>& buf, int block)
 {
-	assert(block < sec_blocks);
+	assert(block < GetBlocksPerSector());
 	assert(memory_block);
 
-	assert((block * sec_size) <= (sb.st_size + sec_size));
+	assert((block * GetSectorSize()) <= (sb.st_size + GetSectorSize()));
 
-	memcpy((void *)&memory_block[(block * sec_size)], buf.data(), sec_size);
+	memcpy((void *)&memory_block[(block * GetSectorSize())], buf.data(), GetSectorSize());
 
 	return true;
 }

cpp/disk_image/mmap_file_handle.h

@@ -3,7 +3,7 @@
 //	SCSI Target Emulator RaSCSI (*^..^*)
 //	for Raspberry Pi
 //
-//	Copyright (C) 2022 akuker
+//	Copyright (C) 2022-2023 akuker
 //
 //  This method of file access will use the mmap() capabilities to 'map' the
 //  file into memory.
@@ -33,13 +33,13 @@ class MmapFileHandle : public DiskImageHandle
 
 public:
 	MmapFileHandle(const string &path, int size, uint32_t blocks, off_t imgoff = 0);
-	~MmapFileHandle();
+	~MmapFileHandle() override;
 
 	// Access
-	bool Save() { return true; };												// Save and release all
+	bool Save()  override { return true; };												// Save and release all
-	bool ReadSector(vector<uint8_t>& buf, int block);										// Sector Read
+	bool ReadSector(vector<uint8_t>& buf, int block) override;										// Sector Read
-	bool WriteSector(const vector<uint8_t>& buf, int block);								// Sector Write
+	bool WriteSector(const vector<uint8_t>& buf, int block) override;								// Sector Write
-	bool GetCache(int index, int &track, uint32_t &serial) const { (void)index; (void)track; (void)serial; return true; }; // Get cache information
+	bool GetCache(int index, int &track, uint32_t &serial) const override { (void)index; (void)track; (void)serial; return true; }; // Get cache information
 
 private:
 	const char *memory_block;

cpp/disk_image/posix_file_handle.cpp

@@ -3,7 +3,7 @@
 //	SCSI Target Emulator RaSCSI (*^..^*)
 //	for Raspberry Pi
 //
-//	Copyright (C) 2022 akuker
+//	Copyright (C) 2022-2023 akuker
 //
 //	[ PosixFileHandle ]
 //
@@ -34,11 +34,11 @@ PosixFileHandle::PosixFileHandle(const string &path, int size, uint32_t blocks,
 	struct stat sb;
 	if (fstat(fd, &sb) < 0)
 	{
-		LOGWARN("Unable to run fstat. Errno:%d", errno);
+		LOGWARN("Unable to run fstat. Errno:%d", errno)
 		return;
 	}
 
-	LOGWARN("%s opened file of size: %d", __PRETTY_FUNCTION__, (unsigned int)sb.st_size);
+	LOGWARN("%s opened file of size: %d", __PRETTY_FUNCTION__, (unsigned int)sb.st_size)
 
 	initialized = true;
 }
@@ -58,20 +58,19 @@ bool PosixFileHandle::ReadSector(vector<uint8_t>& buf, int block)
 		return false;
 	}
 
-	assert(sec_size != 0);
+	assert(block < GetBlocksPerSector());
-	assert(block < sec_blocks);
 
-	size_t sector_size_bytes = (size_t)1 << sec_size;
+	size_t sector_size_bytes = (size_t)1 << GetSectorSize();
 
 	// Calculate offset into the image file
 	off_t offset = GetTrackOffset(block);
 	offset += GetSectorOffset(block);
 
 	lseek(fd, offset, SEEK_SET);
-	size_t result = read(fd, buf.data(), sector_size_bytes);
+	size_t result = read(fd, buf.data(), sector_size_bytes); 
 	if (result != sector_size_bytes)
 	{
-		LOGWARN("%s only read %d bytes but wanted %d", __PRETTY_FUNCTION__, (unsigned int)result, (unsigned int)sector_size_bytes);
+		LOGWARN("%s only read %d bytes but wanted %d", __PRETTY_FUNCTION__, (unsigned int)result, (unsigned int)sector_size_bytes)
 	}
 
 	return true;
@@ -84,9 +83,9 @@ bool PosixFileHandle::WriteSector(const vector<uint8_t>& buf, int block)
 		return false;
 	}
 
-	assert(block < sec_blocks);
+	assert(block < GetBlocksPerSector());
 
-	size_t sector_size_bytes = (size_t)1 << sec_size;
+	size_t sector_size_bytes = (size_t)1 << GetSectorSize();
 
 	off_t offset = GetTrackOffset(block);
 	offset += GetSectorOffset(block);

cpp/disk_image/posix_file_handle.h

@@ -3,7 +3,7 @@
 //	SCSI Target Emulator RaSCSI (*^..^*)
 //	for Raspberry Pi
 //
-//	Copyright (C) 2022 akuker
+//	Copyright (C) 2022-2023 akuker
 //
 //	[ PosixFileHandle ]
 //
@@ -11,7 +11,6 @@
 
 #pragma once
 
-// #include "filepath.h"
 #include "disk_image/disk_image_handle.h"
 
 class PosixFileHandle : public DiskImageHandle
@@ -19,13 +18,13 @@ class PosixFileHandle : public DiskImageHandle
 
 public:
 	PosixFileHandle(const string &path, int size, uint32_t blocks, off_t imgoff = 0);
-	~PosixFileHandle();
+	~PosixFileHandle() override;
 
 	// Access
-	bool Save() { return true; };												// Save and release all
+	bool Save() override  { return true; };												// Save and release all
-	bool ReadSector(vector<uint8_t>& buf, int block);										// Sector Read
+	bool ReadSector(vector<uint8_t>& buf, int block) override ;										// Sector Read
-	bool WriteSector(const vector<uint8_t>& buf, int block);								// Sector Write
+	bool WriteSector(const vector<uint8_t>& buf, int block) override ;								// Sector Write
-	bool GetCache(int index, int &track, uint32_t &serial) const { (void)index; (void)track; (void)serial; return true; }; // Get cache information
+	bool GetCache(int index, int &track, uint32_t &serial) const override { (void)index; (void)track; (void)serial; return true; }; // Get cache information
 
 private:
 	int fd;