bsdp: simplify version type.

5 files changed, 46 insertions, 40 deletions

diff --git a/dhcpv4/bsdp/bsdp.go b/dhcpv4/bsdp/bsdp.go
index 12f3c06..43adcc9 100644
--- a/dhcpv4/bsdp/bsdp.go
+++ b/dhcpv4/bsdp/bsdp.go
@@ -109,7 +109,7 @@ func NewInformList(hwaddr net.HardwareAddr, localIP net.IP, replyPort uint16) (*
 	// These are vendor-specific options used to pass along BSDP information.
 	vendorOpts := []dhcpv4.Option{
 		&OptMessageType{MessageTypeList},
-		&OptVersion{Version1_1},
+		Version1_1,
 	}
 	if needsReplyPort(replyPort) {
 		vendorOpts = append(vendorOpts, &OptReplyPort{replyPort})
@@ -156,7 +156,7 @@ func InformSelectForAck(ack dhcpv4.DHCPv4, replyPort uint16, selectedImage BootI
 	// Data for OptionSelectedBootImageID
 	vendorOpts := []dhcpv4.Option{
 		&OptMessageType{MessageTypeSelect},
-		&OptVersion{Version1_1},
+		Version1_1,
 		&OptSelectedBootImageID{selectedImage.ID},
 	}
 
diff --git a/dhcpv4/bsdp/bsdp_option_version.go b/dhcpv4/bsdp/bsdp_option_version.go
index 041f3d1..d6b78c8 100644
--- a/dhcpv4/bsdp/bsdp_option_version.go
+++ b/dhcpv4/bsdp/bsdp_option_version.go
@@ -7,37 +7,35 @@ import (
 	"github.com/u-root/u-root/pkg/uio"
 )
 
+// Version is the BSDP protocol version. Can be one of 1.0 or 1.1.
+type Version [2]byte
+
 // Specific versions.
 var (
-	Version1_0 = []byte{1, 0}
-	Version1_1 = []byte{1, 1}
+	Version1_0 = Version{1, 0}
+	Version1_1 = Version{1, 1}
 )
 
-// OptVersion represents a BSDP protocol version.
-//
-// Implements the BSDP option version. Can be one of 1.0 or 1.1
-type OptVersion struct {
-	Version []byte
-}
-
 // ParseOptVersion constructs an OptVersion struct from a sequence of
 // bytes and returns it, or an error.
-func ParseOptVersion(data []byte) (*OptVersion, error) {
+func ParseOptVersion(data []byte) (Version, error) {
 	buf := uio.NewBigEndianBuffer(data)
-	return &OptVersion{buf.CopyN(2)}, buf.FinError()
+	var v Version
+	buf.ReadBytes(v[:])
+	return v, buf.FinError()
 }
 
 // Code returns the option code.
-func (o *OptVersion) Code() dhcpv4.OptionCode {
+func (o Version) Code() dhcpv4.OptionCode {
 	return OptionVersion
 }
 
 // ToBytes returns a serialized stream of bytes for this option.
-func (o *OptVersion) ToBytes() []byte {
-	return o.Version
+func (o Version) ToBytes() []byte {
+	return o[:]
 }
 
 // String returns a human-readable string for this option.
-func (o *OptVersion) String() string {
-	return fmt.Sprintf("BSDP Version -> %v.%v", o.Version[0], o.Version[1])
+func (o Version) String() string {
+	return fmt.Sprintf("BSDP Version -> %d.%d", o[0], o[1])
 }
diff --git a/dhcpv4/bsdp/bsdp_option_version_test.go b/dhcpv4/bsdp/bsdp_option_version_test.go
index 8b592a8..69d4c86 100644
--- a/dhcpv4/bsdp/bsdp_option_version_test.go
+++ b/dhcpv4/bsdp/bsdp_option_version_test.go
@@ -7,7 +7,7 @@ import (
 )
 
 func TestOptVersionInterfaceMethods(t *testing.T) {
-	o := OptVersion{Version1_1}
+	o := Version1_1
 	require.Equal(t, OptionVersion, o.Code(), "Code")
 	require.Equal(t, []byte{1, 1}, o.ToBytes(), "ToBytes")
 }
@@ -16,7 +16,7 @@ func TestParseOptVersion(t *testing.T) {
 	data := []byte{1, 1}
 	o, err := ParseOptVersion(data)
 	require.NoError(t, err)
-	require.Equal(t, &OptVersion{Version1_1}, o)
+	require.Equal(t, Version1_1, o)
 
 	// Short byte stream
 	data = []byte{2}
@@ -25,7 +25,5 @@ func TestParseOptVersion(t *testing.T) {
 }
 
 func TestOptVersionString(t *testing.T) {
-	// known
-	o := OptVersion{Version1_1}
-	require.Equal(t, "BSDP Version -> 1.1", o.String())
+	require.Equal(t, "BSDP Version -> 1.1", Version1_1.String())
 }
diff --git a/dhcpv4/bsdp/bsdp_test.go b/dhcpv4/bsdp/bsdp_test.go
index ce09324..cb1a456 100644
--- a/dhcpv4/bsdp/bsdp_test.go
+++ b/dhcpv4/bsdp/bsdp_test.go
@@ -401,7 +401,7 @@ func TestMessageTypeForPacket(t *testing.T) {
 				&dhcpv4.OptHostName{HostName: "foobar1234"},
 				&OptVendorSpecificInformation{
 					Options: []dhcpv4.Option{
-						&OptVersion{Version: Version1_1},
+						Version1_1,
 					},
 				},
 			},
@@ -412,7 +412,7 @@ func TestMessageTypeForPacket(t *testing.T) {
 				&dhcpv4.OptHostName{HostName: "foobar1234"},
 				&OptVendorSpecificInformation{
 					Options: []dhcpv4.Option{
-						&OptVersion{Version: Version1_1},
+						Version1_1,
 						&OptMessageType{Type: MessageTypeList},
 					},
 				},
diff --git a/dhcpv4/bsdp/option_vendor_specific_information_test.go b/dhcpv4/bsdp/option_vendor_specific_information_test.go
index 108a95b..64977c4 100644
--- a/dhcpv4/bsdp/option_vendor_specific_information_test.go
+++ b/dhcpv4/bsdp/option_vendor_specific_information_test.go
@@ -9,7 +9,7 @@ import (
 
 func TestOptVendorSpecificInformationInterfaceMethods(t *testing.T) {
 	messageTypeOpt := &OptMessageType{MessageTypeList}
-	versionOpt := &OptVersion{Version1_1}
+	versionOpt := Version1_1
 	o := &OptVendorSpecificInformation{[]dhcpv4.Option{messageTypeOpt, versionOpt}}
 	require.Equal(t, dhcpv4.OptionVendorSpecificInformation, o.Code(), "Code")
 
@@ -20,7 +20,7 @@ func TestOptVendorSpecificInformationInterfaceMethods(t *testing.T) {
 	o = &OptVendorSpecificInformation{
 		[]dhcpv4.Option{
 			&OptMessageType{MessageTypeList},
-			&OptVersion{Version1_1},
+			Version1_1,
 		},
 	}
 	require.Equal(t, expectedBytes, o.ToBytes(), "ToBytes")
@@ -44,7 +44,7 @@ func TestParseOptVendorSpecificInformation(t *testing.T) {
 	expected := &OptVendorSpecificInformation{
 		[]dhcpv4.Option{
 			&OptMessageType{MessageTypeList},
-			&OptVersion{Version1_1},
+			Version1_1,
 		},
 	}
 	require.Equal(t, 2, len(o.Options), "number of parsed suboptions")
@@ -126,7 +126,7 @@ func TestOptVendorSpecificInformationString(t *testing.T) {
 	o := &OptVendorSpecificInformation{
 		[]dhcpv4.Option{
 			&OptMessageType{MessageTypeList},
-			&OptVersion{Version1_1},
+			Version1_1,
 		},
 	}
 	expectedString := "Vendor Specific Information ->\n  BSDP Message Type -> LIST\n  BSDP Version -> 1.1"
@@ -171,7 +171,7 @@ func TestOptVendorSpecificInformationGetOptions(t *testing.T) {
 	o := &OptVendorSpecificInformation{
 		[]dhcpv4.Option{
 			&OptMessageType{MessageTypeList},
-			&OptVersion{Version1_1},
+			Version1_1,
 		},
 	}
 	foundOpts := o.GetOption(OptionBootImageList)
@@ -181,7 +181,7 @@ func TestOptVendorSpecificInformationGetOptions(t *testing.T) {
 	o = &OptVendorSpecificInformation{
 		[]dhcpv4.Option{
 			&OptMessageType{MessageTypeList},
-			&OptVersion{Version1_1},
+			Version1_1,
 		},
 	}
 	foundOpts = o.GetOption(OptionMessageType)
@@ -189,17 +189,27 @@ func TestOptVendorSpecificInformationGetOptions(t *testing.T) {
 	require.Equal(t, MessageTypeList, foundOpts[0].(*OptMessageType).Type)
 
 	// Multiple options
+	//
+	// TODO: Remove this test when RFC 3396 is properly implemented. This
+	// isn't a valid packet. RFC 2131, Section 4.1: "Options may appear
+	// only once." RFC 3396 clarifies this to say that options will be
+	// concatenated. I.e., in this case, the bytes would be:
+	//
+	// <versioncode> 4 1 1 0 1
+	//
+	// Which would obviously not be parsed by the Version parser, as it
+	// only accepts two bytes.
 	o = &OptVendorSpecificInformation{
 		[]dhcpv4.Option{
 			&OptMessageType{MessageTypeList},
-			&OptVersion{Version1_1},
-			&OptVersion{Version1_0},
+			Version1_1,
+			Version1_0,
 		},
 	}
 	foundOpts = o.GetOption(OptionVersion)
 	require.Equal(t, 2, len(foundOpts), "should get two options")
-	require.Equal(t, Version1_1, foundOpts[0].(*OptVersion).Version)
-	require.Equal(t, Version1_0, foundOpts[1].(*OptVersion).Version)
+	require.Equal(t, Version1_1, foundOpts[0].(Version))
+	require.Equal(t, Version1_0, foundOpts[1].(Version))
 }
 
 func TestOptVendorSpecificInformationGetOneOption(t *testing.T) {
@@ -207,7 +217,7 @@ func TestOptVendorSpecificInformationGetOneOption(t *testing.T) {
 	o := &OptVendorSpecificInformation{
 		[]dhcpv4.Option{
 			&OptMessageType{MessageTypeList},
-			&OptVersion{Version1_1},
+			Version1_1,
 		},
 	}
 	foundOpt := o.GetOneOption(OptionBootImageList)
@@ -217,7 +227,7 @@ func TestOptVendorSpecificInformationGetOneOption(t *testing.T) {
 	o = &OptVendorSpecificInformation{
 		[]dhcpv4.Option{
 			&OptMessageType{MessageTypeList},
-			&OptVersion{Version1_1},
+			Version1_1,
 		},
 	}
 	foundOpt = o.GetOneOption(OptionMessageType)
@@ -227,10 +237,10 @@ func TestOptVendorSpecificInformationGetOneOption(t *testing.T) {
 	o = &OptVendorSpecificInformation{
 		[]dhcpv4.Option{
 			&OptMessageType{MessageTypeList},
-			&OptVersion{Version1_1},
-			&OptVersion{Version1_0},
+			Version1_1,
+			Version1_0,
 		},
 	}
 	foundOpt = o.GetOneOption(OptionVersion)
-	require.Equal(t, Version1_1, foundOpt.(*OptVersion).Version)
+	require.Equal(t, Version1_1, foundOpt.(Version))
 }