Merge pull request #237 from skx/236-cache

Update to avoid modifying the FCB when file open/made

Author: skx

cpm/cpm.go

@@ -130,12 +130,12 @@ type CPM struct {
 	// ccp contains the name of the CCP we should load
 	ccp string
 
-	// files is the cache we use for File handles.
+	// files is the cache we use for File handles - to avoid having to open
+	// close files on the host-side during every operation.
 	//
-	// NOTE: The key is the address of the FCB which we assume is static, but
-	// I'm increasingly of the opinion this is wrong - we should cache on the
-	// filepath.
-	files map[uint16]FileCache
+	// The key is the name of the CP/M file, inside the guest.  (i.e. "FOO.BAR"
+	// rather than A/FOO.BAR which might be the ultimate path on the host.)
+	files map[string]FileCache
 
 	// virtual contains a reference to a static filesystem which
 	// is embedded within our binary, if any.
@@ -623,7 +623,7 @@ func New(options ...Option) (*CPM, error) {
 		ccp:          DefaultCCP,
 		dma:          DefaultDMAAddress,
 		drives:       make(map[string]string),
-		files:        make(map[uint16]FileCache),
+		files:        make(map[string]FileCache),
 		input:        iDriver, // default
 		output:       oDriver, // default
 		prnPath:      DefaultPrinterPath,
@@ -894,13 +894,13 @@ func (cpm *CPM) Execute(args []string) error {
 	// Reset any cached filehandles.
 	//
 	// This is only required when running the CCP, as there we're persistent.
-	for fcb, obj := range cpm.files {
+	for name, obj := range cpm.files {
 		slog.Debug("Closing handle in FileCache",
-			slog.String("path", obj.name),
-			slog.Int("fcb", int(fcb)))
+			slog.String("host", obj.name),
+			slog.String("guest", name))
 		obj.handle.Close()
 	}
-	cpm.files = make(map[uint16]FileCache)
+	cpm.files = make(map[string]FileCache)
 
 	// Create the CPU, pointing to our memory, and setting the initial program counter
 	// to point to our expected entry-point.

cpm/cpm_bdos.go

@@ -394,7 +394,7 @@ func BdosSysCallFileOpen(cpm *CPM) error {
 
 		// Yes we can!
 		// Save the file handle in our cache.
-		cpm.files[ptr] = FileCache{name: fileName, handle: nil}
+		cpm.files[fcbPtr.GetCacheKey()] = FileCache{name: fileName, handle: nil}
 
 		// Get file size, in blocks
 		fLen := uint8(len(virt) / blkSize)
@@ -405,10 +405,6 @@ func BdosSysCallFileOpen(cpm *CPM) error {
 			fcbPtr.RC = fLen
 		}
 
-		// Write our cache-key in the FCB
-		fcbPtr.Al[0] = uint8(ptr & 0xFF)
-		fcbPtr.Al[1] = uint8(ptr >> 8)
-
 		// Update the FCB in memory.
 		cpm.Memory.SetRange(ptr, fcbPtr.AsBytes()...)
 
@@ -443,7 +439,7 @@ func BdosSysCallFileOpen(cpm *CPM) error {
 	}
 
 	// Save the file handle in our cache.
-	cpm.files[ptr] = FileCache{name: fileName, handle: file}
+	cpm.files[fcbPtr.GetCacheKey()] = FileCache{name: fileName, handle: file}
 
 	// Get file size, in bytes
 	fi, err := file.Stat()
@@ -469,10 +465,6 @@ func BdosSysCallFileOpen(cpm *CPM) error {
 		slog.Int("record_count", int(fcbPtr.RC)),
 		slog.Int64("file_size", fileSize))
 
-	// Write our cache-key in the FCB
-	fcbPtr.Al[0] = uint8(ptr & 0xFF)
-	fcbPtr.Al[1] = uint8(uint16(ptr >> 8))
-
 	// Update the FCB in memory.
 	cpm.Memory.SetRange(ptr, fcbPtr.AsBytes()...)
 
@@ -498,14 +490,11 @@ func BdosSysCallFileClose(cpm *CPM) error {
 	// Create a structure with the contents
 	fcbPtr := fcb.FromBytes(xxx)
 
-	// Get our cache-key from the FCB
-	key := uint16(uint16(fcbPtr.Al[1])<<8 + uint16(fcbPtr.Al[0]))
-
 	// Get the file handle from our cache.
-	obj, ok := cpm.files[key]
+	obj, ok := cpm.files[fcbPtr.GetCacheKey()]
 	if !ok {
 		slog.Debug("SysCallFileClose tried to close a file that wasn't open",
-			slog.Int("fcb", int(ptr)))
+			slog.String("guest", fcbPtr.GetCacheKey()))
 		cpm.CPU.States.HL.SetU16(0x00FF)
 		return nil
 	}
@@ -545,11 +534,8 @@ func BdosSysCallFileClose(cpm *CPM) error {
 	}
 
 	// delete the entry from the cache.
-	delete(cpm.files, key)
+	delete(cpm.files, fcbPtr.GetCacheKey())
 
-	// Update the FCB in RAM
-	fcbPtr.Al[0] = 0x00
-	fcbPtr.Al[1] = 0x00
 	cpm.Memory.SetRange(ptr, fcbPtr.AsBytes()...)
 
 	// Record success
@@ -768,11 +754,8 @@ func BdosSysCallRead(cpm *CPM) error {
 	// Create a structure with the contents
 	fcbPtr := fcb.FromBytes(xxx)
 
-	// Get our cache-key from the FCB
-	key := uint16(uint16(fcbPtr.Al[1])<<8 + uint16(fcbPtr.Al[0]))
-
 	// Get the file handle in our cache.
-	obj, ok := cpm.files[key]
+	obj, ok := cpm.files[fcbPtr.GetCacheKey()]
 	if !ok {
 		slog.Error("SysCallRead: Attempting to read from a file that isn't open",
 			slog.String("filename", fcbPtr.GetFileName()))
@@ -883,11 +866,8 @@ func BdosSysCallWrite(cpm *CPM) error {
 	// Create a structure with the contents
 	fcbPtr := fcb.FromBytes(xxx)
 
-	// Get our cache-key from the FCB
-	key := uint16(uint16(fcbPtr.Al[1])<<8 + uint16(fcbPtr.Al[0]))
-
 	// Get the file handle in our cache.
-	obj, ok := cpm.files[key]
+	obj, ok := cpm.files[fcbPtr.GetCacheKey()]
 	if !ok {
 		slog.Error("SysCallWrite: Attempting to write to a file that isn't open")
 		cpm.CPU.States.HL.SetU16(0x00FF)
@@ -1028,12 +1008,8 @@ func BdosSysCallMakeFile(cpm *CPM) error {
 		fcbPtr.RC = fLen
 	}
 
-	// Write our cache-key in the FCB
-	fcbPtr.Al[0] = uint8(ptr & 0xFF)
-	fcbPtr.Al[1] = uint8(ptr >> 8)
-
 	// Save the file-handle
-	cpm.files[ptr] = FileCache{name: fileName, handle: file}
+	cpm.files[fcbPtr.GetCacheKey()] = FileCache{name: fileName, handle: file}
 
 	l.Debug("result:OK",
 		slog.Int("fcb", int(ptr)),
@@ -1264,11 +1240,8 @@ func BdosSysCallReadRand(cpm *CPM) error {
 	// Create a structure with the contents
 	fcbPtr := fcb.FromBytes(xxx)
 
-	// Get our cache-key from the FCB
-	key := uint16(uint16(fcbPtr.Al[1])<<8 + uint16(fcbPtr.Al[0]))
-
 	// Get the file handle in our cache.
-	obj, ok := cpm.files[key]
+	obj, ok := cpm.files[fcbPtr.GetCacheKey()]
 	if !ok {
 		slog.Error("SysCallReadRand: Attempting to read from a file that isn't open",
 			slog.String("filename", fcbPtr.GetFileName()))
@@ -1361,11 +1334,8 @@ func BdosSysCallWriteRand(cpm *CPM) error {
 	// Create a structure with the contents
 	fcbPtr := fcb.FromBytes(xxx)
 
-	// Get our cache-key from the FCB
-	key := uint16(uint16(fcbPtr.Al[1])<<8 + uint16(fcbPtr.Al[0]))
-
 	// Get the file handle in our cache.
-	obj, ok := cpm.files[key]
+	obj, ok := cpm.files[fcbPtr.GetCacheKey()]
 	if !ok {
 		slog.Error("SysCallWriteRand: Attempting to write to a file that isn't open")
 		cpm.CPU.States.HL.SetU16(0x00FF)

fcb/fcb.go

@@ -115,6 +115,33 @@ func (f *FCB) GetFileName() string {
 	return strings.TrimSpace(name)
 }
 
+// GetCacheKey returns a string which can be used for caching this
+// object in some way - it's the name of the file, as seen by the
+// CP/M system.
+func (f *FCB) GetCacheKey() string {
+	t := ""
+
+	// Name
+	for _, c := range f.Name {
+		if unicode.IsPrint(rune(c)) {
+			t += string(c)
+		} else {
+			t += " "
+		}
+	}
+
+	// Suffix
+	for _, c := range f.Type {
+		if unicode.IsPrint(rune(c)) {
+			t += string(c)
+		} else {
+			t += " "
+		}
+	}
+	return t
+
+}
+
 // AsBytes returns the entry of the FCB in a format suitable
 // for copying to RAM.
 func (f *FCB) AsBytes() []uint8 {

fcb/fcb_test.go

@@ -78,6 +78,9 @@ func TestFCBFromString(t *testing.T) {
 	if f.GetType() != "   " {
 		t.Fatalf("unexpected suffix '%v'", f.GetType())
 	}
+	if f.GetCacheKey() != "FOO        " {
+		t.Fatalf("name wrong, got '%v'", f.GetCacheKey())
+	}
 
 	// Try a long name, to confirm it is truncated
 	f = FromString("c:this-is-a-long-name")
@@ -105,6 +108,9 @@ func TestFCBFromString(t *testing.T) {
 	if f.GetFileName() != "THIS-IS-.LON" {
 		t.Fatalf("wrong name returned, got %v", f.GetFileName())
 	}
+	if f.GetCacheKey() != "THIS-IS-LON" {
+		t.Fatalf("wrong cache returned, got %v", f.GetCacheKey())
+	}
 
 	// wildcard
 	f = FromString("c:steve*.*")
@@ -129,6 +135,15 @@ func TestFCBFromString(t *testing.T) {
 		t.Fatalf("name wrong, got '%v'", f.GetName())
 	}
 
+	f = FromString("")
+	f.Name[0] = 0x00
+	f.Name[1] = 0x01
+	f.Type[0] = 0x00
+	f.Type[1] = 0x01
+	if f.GetCacheKey() != "           " {
+		t.Fatalf("wrong cache returned, got %v", f.GetCacheKey())
+	}
+
 }
 
 func TestDoesMatch(t *testing.T) {