Refactored towards a better design for sector maps

- Loading a .dsk (or any other file type) should yield a SectorMap
  interface, which reads and writes *physical*
  sectors. (Auto-detection of sector mapping will come later.)
- The various filesystem packages (dos33, supermon, etc.) are
  responsible for logical sector mapping.
- The weird part about this is that .dsk images are already mapped to
  dos33 logical sector order, so they have to be un-mapped, then
  re-mapped by the dos33 package. It's still cleaner that way.

lib/disk/disk.go

@@ -30,12 +30,90 @@ type TrackSector struct {
 }
 
 type SectorDisk interface {
+	// ReadPhysicalSector reads a single physical sector from the disk. It
+	// always returns 256 byes.
+	ReadPhysicalSector(track byte, sector byte) ([]byte, error)
+	// WritePhysicalSector writes a single physical sector to a disk. It
+	// expects exactly 256 bytes.
+	WritePhysicalSector(track byte, sector byte, data []byte) error
+	// Sectors returns the number of sectors on the SectorDisk
+	Sectors() byte
+	// Tracks returns the number of tracks on the SectorDisk
+	Tracks() byte
+}
+
+type LogicalSectorDisk interface {
 	// ReadLogicalSector reads a single logical sector from the disk. It
 	// always returns 256 byes.
 	ReadLogicalSector(track byte, sector byte) ([]byte, error)
 	// WriteLogicalSector writes a single logical sector to a disk. It
 	// expects exactly 256 bytes.
 	WriteLogicalSector(track byte, sector byte, data []byte) error
+	// Sectors returns the number of sectors on the SectorDisk
+	Sectors() byte
+	// Tracks returns the number of tracks on the SectorDisk
+	Tracks() byte
+}
+
+// MappedDisk wraps a SectorDisk as a LogicalSectorDisk, handling the
+// logical-to-physical sector mapping.
+type MappedDisk struct {
+	sectorDisk        SectorDisk // The underlying physical sector disk.
+	logicalToPhysical []byte     // The mapping of logical to physical sectors.
+}
+
+var _ LogicalSectorDisk = MappedDisk{}
+
+func NewMappedDisk(sd SectorDisk, logicalToPhysical []byte) (MappedDisk, error) {
+	if logicalToPhysical != nil && len(logicalToPhysical) != int(sd.Sectors()) {
+		return MappedDisk{}, fmt.Errorf("NewMappedDisk called on a disk image with %d sectors per track, but a mapping of length %d", sd.Sectors(), len(logicalToPhysical))
+	}
+	if logicalToPhysical == nil {
+		logicalToPhysical = make([]byte, int(sd.Sectors()))
+		for i := range logicalToPhysical {
+			logicalToPhysical[i] = byte(i)
+		}
+	}
+	return MappedDisk{
+		sectorDisk:        sd,
+		logicalToPhysical: logicalToPhysical,
+	}, nil
+}
+
+// ReadLogicalSector reads a single logical sector from the disk. It
+// always returns 256 byes.
+func (md MappedDisk) ReadLogicalSector(track byte, sector byte) ([]byte, error) {
+	if track >= md.sectorDisk.Tracks() {
+		return nil, fmt.Errorf("ReadLogicalSector expected track between 0 and %d; got %d", md.sectorDisk.Tracks()-1, track)
+	}
+	if sector >= md.sectorDisk.Sectors() {
+		return nil, fmt.Errorf("ReadLogicalSector expected sector between 0 and %d; got %d", md.sectorDisk.Sectors()-1, sector)
+	}
+	physicalSector := md.logicalToPhysical[int(sector)]
+	return md.sectorDisk.ReadPhysicalSector(track, physicalSector)
+}
+
+// WriteLogicalSector writes a single logical sector to a disk. It
+// expects exactly 256 bytes.
+func (md MappedDisk) WriteLogicalSector(track byte, sector byte, data []byte) error {
+	if track >= md.sectorDisk.Tracks() {
+		return fmt.Errorf("WriteLogicalSector expected track between 0 and %d; got %d", md.sectorDisk.Tracks()-1, track)
+	}
+	if sector >= md.sectorDisk.Sectors() {
+		return fmt.Errorf("WriteLogicalSector expected sector between 0 and %d; got %d", md.sectorDisk.Sectors()-1, sector)
+	}
+	physicalSector := md.logicalToPhysical[int(sector)]
+	return md.sectorDisk.WritePhysicalSector(track, physicalSector, data)
+}
+
+// Sectors returns the number of sectors on the DSK image.
+func (md MappedDisk) Sectors() byte {
+	return md.sectorDisk.Sectors()
+}
+
+// Tracks returns the number of tracks on the DSK image.
+func (md MappedDisk) Tracks() byte {
+	return md.sectorDisk.Tracks()
 }
 
 const (
@@ -48,55 +126,76 @@ const (
 
 // DSK represents a .dsk disk image.
 type DSK struct {
-	data [DOS33DiskBytes]byte
+	data             []byte // The actual data in the file
+	sectors          byte   // Number of sectors per track
+	physicalToStored []byte // Map of physical on-disk sector numbers to sectors in the disk image
+	bytesPerTrack    int    // Number of bytes per track
+	tracks           byte   // Number of tracks
 }
 
 var _ SectorDisk = (*DSK)(nil)
 
 // LoadDSK loads a .dsk image from a file.
 func LoadDSK(filename string) (DSK, error) {
-	d := DSK{}
 	bb, err := ioutil.ReadFile(filename)
 	if err != nil {
-		return d, err
+		return DSK{}, err
 	}
+	// TODO(zellyn): handle 13-sector disks.
 	if len(bb) != DOS33DiskBytes {
-		return d, fmt.Errorf("Expected file %q to contain %d bytes, but got %d.", filename, DOS33DiskBytes, len(bb))
+		return DSK{}, fmt.Errorf("Expected file %q to contain %d bytes, but got %d.", filename, DOS33DiskBytes, len(bb))
 	}
-	copy(d.data[:], bb)
-	return d, nil
+	return DSK{
+		data:             bb,
+		sectors:          16,
+		physicalToStored: Dos33PhysicalToLogicalSectorMap,
+		bytesPerTrack:    16 * 256,
+		tracks:           DOS33Tracks,
+	}, nil
 }
 
-// ReadLogicalSector reads a single logical sector from the disk. It
+// ReadPhysicalSector reads a single physical sector from the disk. It
 // always returns 256 byes.
-func (d DSK) ReadLogicalSector(track byte, sector byte) ([]byte, error) {
-	if track >= DOS33Tracks {
-		return nil, fmt.Errorf("Expected track between 0 and %d; got %d", DOS33Tracks-1, track)
+func (d DSK) ReadPhysicalSector(track byte, sector byte) ([]byte, error) {
+	if track >= d.tracks {
+		return nil, fmt.Errorf("ReadPhysicalSector expected track between 0 and %d; got %d", d.tracks-1, track)
 	}
-	if sector >= DOS33Sectors {
-		return nil, fmt.Errorf("Expected sector between 0 and %d; got %d", DOS33Sectors-1, sector)
+	if sector >= d.sectors {
+		return nil, fmt.Errorf("ReadPhysicalSector expected sector between 0 and %d; got %d", d.sectors-1, sector)
 	}
 
-	start := int(track)*DOS33TrackBytes + 256*int(sector)
+	storedSector := d.physicalToStored[int(sector)]
+	start := int(track)*d.bytesPerTrack + 256*int(storedSector)
 	buf := make([]byte, 256)
 	copy(buf, d.data[start:start+256])
 	return buf, nil
 }
 
-// WriteLogicalSector writes a single logical sector to a disk. It
+// WritePhysicalSector writes a single physical sector to a disk. It
 // expects exactly 256 bytes.
-func (d DSK) WriteLogicalSector(track byte, sector byte, data []byte) error {
-	if track >= DOS33Tracks {
-		return fmt.Errorf("Expected track between 0 and %d; got %d", DOS33Tracks-1, track)
+func (d DSK) WritePhysicalSector(track byte, sector byte, data []byte) error {
+	if track >= d.tracks {
+		return fmt.Errorf("WritePhysicalSector expected track between 0 and %d; got %d", d.tracks-1, track)
 	}
-	if sector >= DOS33Sectors {
-		return fmt.Errorf("Expected sector between 0 and %d; got %d", DOS33Sectors-1, sector)
+	if sector >= d.sectors {
+		return fmt.Errorf("WritePhysicalSector expected sector between 0 and %d; got %d", d.sectors-1, sector)
 	}
 	if len(data) != 256 {
-		return fmt.Errorf("WriteLogicalSector expects data of length 256; got %d", len(data))
+		return fmt.Errorf("WritePhysicalSector expects data of length 256; got %d", len(data))
 	}
 
-	start := int(track)*DOS33TrackBytes + 256*int(sector)
+	storedSector := d.physicalToStored[int(sector)]
+	start := int(track)*d.bytesPerTrack + 256*int(storedSector)
 	copy(d.data[start:start+256], data)
 	return nil
 }
+
+// Sectors returns the number of sectors on the DSK image.
+func (d DSK) Sectors() byte {
+	return d.sectors
+}
+
+// Tracks returns the number of tracks on the DSK image.
+func (d DSK) Tracks() byte {
+	return d.tracks
+}

lib/disk/marshal.go

@@ -28,7 +28,7 @@ type SectorSink interface {
 
 // UnmarshalLogicalSector reads a sector from a SectorDisk, and
 // unmarshals it into a SectorSink, setting its track and sector.
-func UnmarshalLogicalSector(d SectorDisk, ss SectorSink, track, sector byte) error {
+func UnmarshalLogicalSector(d LogicalSectorDisk, ss SectorSink, track, sector byte) error {
 	bytes, err := d.ReadLogicalSector(track, sector)
 	if err != nil {
 		return err
@@ -41,7 +41,7 @@ func UnmarshalLogicalSector(d SectorDisk, ss SectorSink, track, sector byte) err
 
 // MarshalLogicalSector marshals a SectorSource to its sector on a
 // SectorDisk.
-func MarshalLogicalSector(d SectorDisk, ss SectorSource) error {
+func MarshalLogicalSector(d LogicalSectorDisk, ss SectorSource) error {
 	track := ss.GetTrack()
 	sector := ss.GetSector()
 	bytes := ss.ToSector()

lib/dos33/dos33.go

@@ -398,7 +398,7 @@ func (tsl *TrackSectorList) FromSector(data []byte) {
 
 // readCatalogSectors reads the raw CatalogSector structs from a DOS
 // 3.3 disk.
-func readCatalogSectors(d disk.SectorDisk) ([]CatalogSector, error) {
+func readCatalogSectors(d disk.LogicalSectorDisk) ([]CatalogSector, error) {
 	v := &VTOC{}
 	err := disk.UnmarshalLogicalSector(d, v, VTOCTrack, VTOCSector)
 	if err != nil {
@@ -431,7 +431,7 @@ func readCatalogSectors(d disk.SectorDisk) ([]CatalogSector, error) {
 }
 
 // ReadCatalog reads the catalog of a DOS 3.3 disk.
-func ReadCatalog(d disk.SectorDisk) (files, deleted []FileDesc, err error) {
+func ReadCatalog(d disk.LogicalSectorDisk) (files, deleted []FileDesc, err error) {
 	css, err := readCatalogSectors(d)
 	if err != nil {
 		return nil, nil, err

lib/dos33/dos33_test.go

@@ -67,7 +67,11 @@ func TestTrackSectorListMarshalRoundtrip(t *testing.T) {
 
 // TestReadCatalog tests the reading of the catalog of a test disk.
 func TestReadCatalog(t *testing.T) {
-	dsk, err := disk.LoadDSK("testdata/dos33test.dsk")
+	sd, err := disk.LoadDSK("testdata/dos33test.dsk")
+	if err != nil {
+		t.Fatal(err)
+	}
+	dsk, err := disk.NewMappedDisk(sd, disk.Dos33LogicalToPhysicalSectorMap)
 	if err != nil {
 		t.Fatal(err)
 	}

lib/supermon/supermon.go

@@ -19,33 +19,6 @@ const (
 	FileReserved = 0xfe
 )
 
-// SectorDiskShim is a shim to undo DOS 3.3 sector mapping:
-// NakedOS/Super-Mon disks are typically represented as DOS 3.3 .dsk
-// images, but NakedOS does no sector mapping.
-type SectorDiskShim struct {
-	Dos33 disk.SectorDisk
-}
-
-// ReadLogicalSector reads a single logical sector from the disk. It
-// always returns 256 byes.
-func (s SectorDiskShim) ReadLogicalSector(track byte, sector byte) ([]byte, error) {
-	if sector >= 16 {
-		return nil, fmt.Errorf("Expected sector between 0 and 15; got %d", sector)
-	}
-	sector = disk.Dos33PhysicalToLogicalSectorMap[int(sector)]
-	return s.Dos33.ReadLogicalSector(track, sector)
-}
-
-// WriteLogicalSector writes a single logical sector to a disk. It
-// expects exactly 256 bytes.
-func (s SectorDiskShim) WriteLogicalSector(track byte, sector byte, data []byte) error {
-	if sector >= 16 {
-		return fmt.Errorf("Expected sector between 0 and 15; got %d", sector)
-	}
-	sector = disk.Dos33PhysicalToLogicalSectorMap[int(sector)]
-	return s.Dos33.WriteLogicalSector(track, sector, data)
-}
-
 // SectorMap is the list of sectors by file. It's always 560 bytes
 // long (35 tracks * 16 sectors).
 type SectorMap []byte
@@ -53,15 +26,15 @@ type SectorMap []byte
 // LoadSectorMap loads a NakedOS sector map.
 func LoadSectorMap(sd disk.SectorDisk) (SectorMap, error) {
 	sm := SectorMap(make([]byte, 560))
-	sector09, err := sd.ReadLogicalSector(0, 9)
+	sector09, err := sd.ReadPhysicalSector(0, 9)
 	if err != nil {
 		return sm, err
 	}
-	sector0A, err := sd.ReadLogicalSector(0, 0xA)
+	sector0A, err := sd.ReadPhysicalSector(0, 0xA)
 	if err != nil {
 		return sm, err
 	}
-	sector0B, err := sd.ReadLogicalSector(0, 0xB)
+	sector0B, err := sd.ReadPhysicalSector(0, 0xB)
 	if err != nil {
 		return sm, err
 	}
@@ -71,6 +44,26 @@ func LoadSectorMap(sd disk.SectorDisk) (SectorMap, error) {
 	return sm, nil
 }
 
+// Verify checks that we actually have a NakedOS disk.
+func (sm SectorMap) Verify() error {
+	for sector := byte(0); sector <= 0xB; sector++ {
+		if file := sm.FileForSector(0, sector); file != FileReserved {
+			return fmt.Errorf("Expected track 0, sectors 0-C to be reserved (0xFE), but got 0x%02X in sector %X", file, sector)
+		}
+	}
+
+	for track := byte(0); track < 35; track++ {
+		for sector := byte(0); sector < 16; sector++ {
+			file := sm.FileForSector(track, sector)
+			if file == FileIllegal {
+				return fmt.Errorf("Found illegal sector map value (%02X), in track %X sector %X", FileIllegal, track, sector)
+			}
+		}
+	}
+
+	return nil
+}
+
 // FileForSector returns the file that owns the given track/sector.
 func (sm SectorMap) FileForSector(track, sector byte) byte {
 	if track >= 35 {
@@ -96,25 +89,6 @@ func (sm SectorMap) SectorsForFile(file byte) []disk.TrackSector {
 	return result
 }
 
-func (sm SectorMap) Verify() error {
-	for sector := byte(0); sector <= 0xB; sector++ {
-		if file := sm.FileForSector(0, sector); file != FileReserved {
-			return fmt.Errorf("Expected track 0, sectors 0-C to be reserved (0xFE), but got 0x%02X in sector %X", file, sector)
-		}
-	}
-
-	for track := byte(0); track < 35; track++ {
-		for sector := byte(0); sector < 16; sector++ {
-			file := sm.FileForSector(track, sector)
-			if file == FileIllegal {
-				return fmt.Errorf("Found illegal sector map value (%02X), in track %X sector %X", FileIllegal, track, sector)
-			}
-		}
-	}
-
-	return nil
-}
-
 // SectorsByFile returns a map of file number to slice of sectors.
 func (sm SectorMap) SectorsByFile() map[byte][]disk.TrackSector {
 	result := map[byte][]disk.TrackSector{}
@@ -126,3 +100,112 @@ func (sm SectorMap) SectorsByFile() map[byte][]disk.TrackSector {
 	}
 	return result
 }
+
+// ReadFile reads the contents of a file.
+func (sm SectorMap) ReadFile(sd disk.SectorDisk, file byte) []byte {
+	var result []byte
+	for _, ts := range sm.SectorsForFile(file) {
+		bytes, err := sd.ReadPhysicalSector(ts.Track, ts.Sector)
+		if err != nil {
+			panic(err.Error())
+		}
+		result = append(result, bytes...)
+	}
+	return result
+}
+
+// Symbol represents a single Super-Mon symbol.
+type Symbol struct {
+	// Address is the memory address the symbol points to, or 0 for an
+	// empty symbol table entry.
+	Address uint16
+	// Name is the name of the symbol.
+	Name string
+	// Link is the index of the next symbol in the symbol chain for this
+	// hash key, or -1 if none.
+	Link int
+}
+
+// decodeSymbol decodes a Super-Mon encoded symbol table entry,
+// returning the string representation.
+func decodeSymbol(five []byte, extra byte) string {
+	result := ""
+	value := uint64(five[0]) + uint64(five[1])<<8 + uint64(five[2])<<16 + uint64(five[3])<<24 + uint64(five[4])<<32 + uint64(extra)<<40
+	for value&0x1f > 0 {
+		if value&0x1f < 27 {
+			result = result + string(value&0x1f+'@')
+			value >>= 5
+			continue
+		}
+		if value&0x20 == 0 {
+			result = result + string((value&0x1f)-0x1b+'0')
+		} else {
+			result = result + string((value&0x1f)-0x1b+'5')
+		}
+		value >>= 6
+	}
+	return result
+}
+
+// SymbolTable represents an entire Super-Mon symbol table. It'll
+// always be 819 entries long, because it includes blanks.
+type SymbolTable []Symbol
+
+// ReadSymbolTable reads the symbol table from a disk. If there are
+// problems with the symbol table (like it doesn't exist, or the link
+// pointers are problematic), it'll return nil and an error.
+func (sm SectorMap) ReadSymbolTable(sd disk.SectorDisk) (SymbolTable, error) {
+	table := make(SymbolTable, 0, 819)
+	symtbl1 := sm.ReadFile(sd, 3)
+	if len(symtbl1) != 0x1000 {
+		return nil, fmt.Errorf("expected file FSYMTBL1(0x3) to be 0x1000 bytes long; got 0x%04X", len(symtbl1))
+	}
+	symtbl2 := sm.ReadFile(sd, 4)
+	if len(symtbl2) != 0x1000 {
+		return nil, fmt.Errorf("expected file FSYMTBL1(0x4) to be 0x1000 bytes long; got 0x%04X", len(symtbl2))
+	}
+
+	five := []byte{0, 0, 0, 0, 0}
+	for i := 0; i < 0x0fff; i += 5 {
+		address := uint16(symtbl1[i]) + uint16(symtbl1[i+1])<<8
+		if address == 0 {
+			table = append(table, Symbol{})
+			continue
+		}
+		linkAddr := uint16(symtbl1[i+2]) + uint16(symtbl1[i+3])<<8
+		link := -1
+		if linkAddr != 0 {
+			if linkAddr < 0xD000 || linkAddr >= 0xDFFF {
+				return nil, fmt.Errorf("Expected symbol table link address between 0xD000 and 0xDFFE; got 0x%04X", linkAddr)
+			}
+			if (linkAddr-0xD000)%5 != 0 {
+				return nil, fmt.Errorf("Expected symbol table link address to 0xD000+5x; got 0x%04X", linkAddr)
+			}
+			link = (int(linkAddr) - 0xD000) % 5
+		}
+		extra := symtbl1[i+4]
+		copy(five, symtbl2[i:i+5])
+		name := decodeSymbol(five, extra)
+		symbol := Symbol{
+			Address: address,
+			Name:    name,
+			Link:    link,
+		}
+		table = append(table, symbol)
+	}
+
+	// TODO(zellyn): check link addresses.
+
+	return table, nil
+}
+
+// SymbolsByAddress returns a map of addresses to slices of symbols.
+func (st SymbolTable) SymbolsByAddress() map[uint16][]Symbol {
+	result := map[uint16][]Symbol{}
+	for _, symbol := range st {
+		if symbol.Address != 0 {
+			result[symbol.Address] = append(result[symbol.Address], symbol)
+		}
+	}
+	return result
+}

lib/supermon/supermon_test.go

@@ -9,24 +9,23 @@ import (
 )
 
 // loadSectorMap loads a sector map for the disk image contained in
-// filename.
-func loadSectorMap(filename string) (SectorMap, error) {
-	dsk, err := disk.LoadDSK(filename)
+// filename. It returns the sector map and a sector disk.
+func loadSectorMap(filename string) (SectorMap, disk.SectorDisk, error) {
+	sd, err := disk.LoadDSK(filename)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
-	sd := SectorDiskShim{Dos33: dsk}
 	sm, err := LoadSectorMap(sd)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
-	return sm, nil
+	return sm, sd, nil
 }
 
 // TestReadSectorMap tests the reading of the sector map of a test
 // disk.
 func TestReadSectorMap(t *testing.T) {
-	sm, err := loadSectorMap("testdata/chacha20.dsk")
+	sm, _, err := loadSectorMap("testdata/chacha20.dsk")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -58,3 +57,49 @@ func TestReadSectorMap(t *testing.T) {
 		}
 	}
 }
+
+// TestReadSymbolTable tests the reading of the symbol table of a test
+// disk.
+func TestReadSymbolTable(t *testing.T) {
+	sm, sd, err := loadSectorMap("testdata/chacha20.dsk")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := sm.Verify(); err != nil {
+		t.Fatal(err)
+	}
+
+	st, err := sm.ReadSymbolTable(sd)
+	if err != nil {
+		t.Fatal(err)
+	}
+	symbols := st.SymbolsByAddress()
+
+	testData := []struct {
+		file uint16
+		name string
+	}{
+		{1, "FHELLO"},
+		{2, "FSUPERMON"},
+		{3, "FSYMTBL1"},
+		{4, "FSYMTBL2"},
+		{5, "FMONHELP"},
+		{6, "FSHORTSUP"},
+		{7, "FSHRTHELP"},
+		{8, "FSHORT"},
+		{9, "FCHACHA"},
+	}
+
+	for _, tt := range testData {
+		fileAddr := uint16(0xDF00) + tt.file
+		syms := symbols[fileAddr]
+		if len(syms) != 1 {
+			t.Errorf("Expected one symbol for address %04X (file %q), but got %d.", fileAddr, tt.file, len(syms))
+			continue
+		}
+		if syms[0].Name != tt.name {
+			t.Errorf("Expected symbol name for address %04X to be %q, but got %q.", fileAddr, tt.name, syms[0].Name)
+			continue
+		}
+	}
+}