From 844ea47435e2d8a31ca6650fc778a41197b9b24a Mon Sep 17 00:00:00 2001 From: Zellyn Hunter Date: Sat, 12 Nov 2016 22:25:42 -0500 Subject: [PATCH] Re-thread errors through sector marshaling code 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.) --- lib/disk/marshal.go | 13 +++++++++---- lib/dos33/dos33.go | 27 +++++++++++++++------------ lib/dos33/dos33_test.go | 15 ++++++++++++--- lib/supermon/supermon.go | 2 ++ 4 files changed, 38 insertions(+), 19 deletions(-) diff --git a/lib/disk/marshal.go b/lib/disk/marshal.go index 2b3a1ab..e6204d1 100644 --- a/lib/disk/marshal.go +++ b/lib/disk/marshal.go @@ -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. @@ -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. @@ -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 @@ -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) } diff --git a/lib/dos33/dos33.go b/lib/dos33/dos33.go index c84763b..370cff6 100644 --- a/lib/dos33/dos33.go +++ b/lib/dos33/dos33.go @@ -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 @@ -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, @@ -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] @@ -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 { @@ -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 @@ -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] @@ -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 @@ -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 @@ -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] @@ -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 diff --git a/lib/dos33/dos33_test.go b/lib/dos33/dos33_test.go index 0c57aac..5bf6ff9 100644 --- a/lib/dos33/dos33_test.go +++ b/lib/dos33/dos33_test.go @@ -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) } @@ -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) } @@ -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) } diff --git a/lib/supermon/supermon.go b/lib/supermon/supermon.go index 6f456e4..4548c3a 100644 --- a/lib/supermon/supermon.go +++ b/lib/supermon/supermon.go @@ -4,6 +4,8 @@ // structures of NakedOS/Super-Mon disks. package supermon +// TODO(zellyn): remove panics. + import ( "fmt"