Skip to content

Commit

Permalink
Re-thread errors through sector marshaling code
Browse files Browse the repository at this point in the history
I'm still not sure what underlying errors might need to be reported
up, and calling panic in a library was definitely a mistake. (Added a
TODO to supermon.go to remove panics there too.)
  • Loading branch information
zellyn committed Nov 13, 2016
1 parent 68ee8a4 commit 844ea47
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 19 deletions.
13 changes: 9 additions & 4 deletions lib/disk/marshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ package disk
// SectorSource is the interface for types that can marshal to sectors.
type SectorSource interface {
// ToSector marshals the sector struct to exactly 256 bytes.
ToSector() []byte
ToSector() ([]byte, error)
// GetTrack returns the track that a sector struct was loaded from.
GetTrack() byte
// GetSector returns the sector that a sector struct was loaded from.
Expand All @@ -19,7 +19,7 @@ type SectorSource interface {
type SectorSink interface {
// FromSector unmarshals the sector struct from bytes. Input is
// expected to be exactly 256 bytes.
FromSector(data []byte)
FromSector(data []byte) error
// SetTrack sets the track that a sector struct was loaded from.
SetTrack(track byte)
// SetSector sets the sector that a sector struct was loaded from.
Expand All @@ -33,7 +33,9 @@ func UnmarshalLogicalSector(d LogicalSectorDisk, ss SectorSink, track, sector by
if err != nil {
return err
}
ss.FromSector(bytes)
if err := ss.FromSector(bytes); err != nil {
return err
}
ss.SetTrack(track)
ss.SetSector(sector)
return nil
Expand All @@ -44,6 +46,9 @@ func UnmarshalLogicalSector(d LogicalSectorDisk, ss SectorSink, track, sector by
func MarshalLogicalSector(d LogicalSectorDisk, ss SectorSource) error {
track := ss.GetTrack()
sector := ss.GetSector()
bytes := ss.ToSector()
bytes, err := ss.ToSector()
if err != nil {
return err
}
return d.WriteLogicalSector(track, sector, bytes)
}
27 changes: 15 additions & 12 deletions lib/dos33/dos33.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func (v *VTOC) Validate() error {
}

// ToSector marshals the VTOC sector to bytes.
func (v VTOC) ToSector() []byte {
func (v VTOC) ToSector() ([]byte, error) {
buf := make([]byte, 256)
buf[0x00] = v.Unused1
buf[0x01] = v.CatalogTrack
Expand All @@ -144,7 +144,7 @@ func (v VTOC) ToSector() []byte {
for i, m := range v.FreeSectors {
copyBytes(buf[0x38+4*i:0x38+4*i+4], m[:])
}
return buf
return buf, nil
}

// copyBytes is just like the builtin copy, but just for byte slices,
Expand All @@ -158,9 +158,9 @@ func copyBytes(dst, src []byte) int {

// FromSector unmarshals the VTOC sector from bytes. Input is
// expected to be exactly 256 bytes.
func (v *VTOC) FromSector(data []byte) {
func (v *VTOC) FromSector(data []byte) error {
if len(data) != 256 {
panic(fmt.Sprintf("VTOC.FromSector expects exactly 256 bytes; got %d", len(data)))
return fmt.Errorf("VTOC.FromSector expects exactly 256 bytes; got %d", len(data))
}

v.Unused1 = data[0x00]
Expand All @@ -181,6 +181,7 @@ func (v *VTOC) FromSector(data []byte) {
for i := range v.FreeSectors {
copyBytes(v.FreeSectors[i][:], data[0x38+4*i:0x38+4*i+4])
}
return nil
}

func DefaultVTOC() VTOC {
Expand Down Expand Up @@ -217,7 +218,7 @@ type CatalogSector struct {
}

// ToSector marshals the CatalogSector to bytes.
func (cs CatalogSector) ToSector() []byte {
func (cs CatalogSector) ToSector() ([]byte, error) {
buf := make([]byte, 256)
buf[0x00] = cs.Unused1
buf[0x01] = cs.NextTrack
Expand All @@ -227,14 +228,14 @@ func (cs CatalogSector) ToSector() []byte {
fdBytes := fd.ToBytes()
copyBytes(buf[0x0b+35*i:0x0b+35*(i+1)], fdBytes)
}
return buf
return buf, nil
}

// FromSector unmarshals the CatalogSector from bytes. Input is
// expected to be exactly 256 bytes.
func (cs *CatalogSector) FromSector(data []byte) {
func (cs *CatalogSector) FromSector(data []byte) error {
if len(data) != 256 {
panic(fmt.Sprintf("CatalogSector.FromSector expects exactly 256 bytes; got %d", len(data)))
return fmt.Errorf("CatalogSector.FromSector expects exactly 256 bytes; got %d", len(data))
}

cs.Unused1 = data[0x00]
Expand All @@ -245,6 +246,7 @@ func (cs *CatalogSector) FromSector(data []byte) {
for i := range cs.FileDescs {
cs.FileDescs[i].FromBytes(data[0x0b+35*i : 0x0b+35*(i+1)])
}
return nil
}

type Filetype byte
Expand Down Expand Up @@ -360,7 +362,7 @@ type TrackSectorList struct {
}

// ToSector marshals the TrackSectorList to bytes.
func (tsl TrackSectorList) ToSector() []byte {
func (tsl TrackSectorList) ToSector() ([]byte, error) {
buf := make([]byte, 256)
buf[0x00] = tsl.Unused1
buf[0x01] = tsl.NextTrack
Expand All @@ -373,14 +375,14 @@ func (tsl TrackSectorList) ToSector() []byte {
buf[0x0C+i*2] = ts.Track
buf[0x0D+i*2] = ts.Sector
}
return buf
return buf, nil
}

// FromSector unmarshals the TrackSectorList from bytes. Input is
// expected to be exactly 256 bytes.
func (tsl *TrackSectorList) FromSector(data []byte) {
func (tsl *TrackSectorList) FromSector(data []byte) error {
if len(data) != 256 {
panic(fmt.Sprintf("TrackSectorList.FromSector expects exactly 256 bytes; got %d", len(data)))
return fmt.Errorf("TrackSectorList.FromSector expects exactly 256 bytes; got %d", len(data))
}

tsl.Unused1 = data[0x00]
Expand All @@ -394,6 +396,7 @@ func (tsl *TrackSectorList) FromSector(data []byte) {
tsl.TrackSectors[i].Track = data[0x0C+i*2]
tsl.TrackSectors[i].Sector = data[0x0D+i*2]
}
return nil
}

// readCatalogSectors reads the raw CatalogSector structs from a DOS
Expand Down
15 changes: 12 additions & 3 deletions lib/dos33/dos33_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@ func TestVTOCMarshalRoundtrip(t *testing.T) {
copy(buf1, buf)
vtoc1 := &VTOC{}
vtoc1.FromSector(buf1)
buf2 := vtoc1.ToSector()
buf2, err := vtoc1.ToSector()
if err != nil {
t.Fatal(err)
}
if !reflect.DeepEqual(buf, buf2) {
t.Errorf("Buffers differ: %v != %v", buf, buf2)
}
Expand All @@ -35,7 +38,10 @@ func TestCatalogSectorMarshalRoundtrip(t *testing.T) {
copy(buf1, buf)
cs1 := &CatalogSector{}
cs1.FromSector(buf1)
buf2 := cs1.ToSector()
buf2, err := cs1.ToSector()
if err != nil {
t.Fatal(err)
}
if !reflect.DeepEqual(buf, buf2) {
t.Errorf("Buffers differ: %v != %v", buf, buf2)
}
Expand All @@ -54,7 +60,10 @@ func TestTrackSectorListMarshalRoundtrip(t *testing.T) {
copy(buf1, buf)
cs1 := &TrackSectorList{}
cs1.FromSector(buf1)
buf2 := cs1.ToSector()
buf2, err := cs1.ToSector()
if err != nil {
t.Fatal(err)
}
if !reflect.DeepEqual(buf, buf2) {
t.Errorf("Buffers differ: %v != %v", buf, buf2)
}
Expand Down
2 changes: 2 additions & 0 deletions lib/supermon/supermon.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
// structures of NakedOS/Super-Mon disks.
package supermon

// TODO(zellyn): remove panics.

import (
"fmt"

Expand Down

0 comments on commit 844ea47

Please sign in to comment.