Misc cleanup

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},
 	}
 

dhcpv4/bsdp/bsdp_option_boot_image_list.go

@@ -5,8 +5,6 @@ import (
 	"github.com/u-root/u-root/pkg/uio"
 )
 
-// Implements the BSDP option listing the boot images.
-
 // OptBootImageList contains the list of boot images presented by a netboot
 // server.
 type OptBootImageList struct {

dhcpv4/bsdp/bsdp_option_generic.go

@@ -32,9 +32,5 @@ func (o OptGeneric) ToBytes() []byte {
 
 // String returns a human-readable representation of a generic option.
 func (o OptGeneric) String() string {
-	code, ok := optionCodeToString[o.Code()]
-	if !ok {
-		code = "Unknown"
-	}
-	return fmt.Sprintf("%v -> %v", code, o.Data)
+	return fmt.Sprintf("%s -> %v", o.OptionCode, o.Data)
 }

dhcpv4/bsdp/bsdp_option_generic_test.go

@@ -50,8 +50,8 @@ func TestOptGenericString(t *testing.T) {
 
 func TestOptGenericStringUnknown(t *testing.T) {
 	o := OptGeneric{
-		OptionCode: 102, // Returend option code.
+		OptionCode: optionCode(102), // Returned option code.
 		Data:       []byte{5},
 	}
-	require.Equal(t, "Unknown -> [5]", o.String())
+	require.Equal(t, "unknown -> [5]", o.String())
 }

dhcpv4/bsdp/bsdp_option_server_identifier.go

@@ -8,7 +8,7 @@ import (
 	"github.com/u-root/u-root/pkg/uio"
 )
 
-// OptServerIdentifier represents an option encapsulating the server identifier.
+// OptServerIdentifier implements the BSDP server identifier option.
 type OptServerIdentifier struct {
 	ServerID net.IP
 }

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])
 }

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())
 }

dhcpv4/bsdp/bsdp_test.go

@@ -106,7 +106,7 @@ func TestNewInformList_ReplyPort(t *testing.T) {
 func newAck(hwAddr net.HardwareAddr, transactionID [4]byte) *dhcpv4.DHCPv4 {
 	ack, _ := dhcpv4.New()
 	ack.TransactionID = transactionID
-	ack.HWType = iana.HwTypeEthernet
+	ack.HWType = iana.HWTypeEthernet
 	ack.ClientHWAddr = hwAddr
 	ack.AddOption(&dhcpv4.OptMessageType{MessageType: dhcpv4.MessageTypeAck})
 	ack.AddOption(&dhcpv4.OptionGeneric{OptionCode: dhcpv4.OptionEnd})
@@ -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},
 					},
 				},

dhcpv4/bsdp/option_vendor_specific_information.go

@@ -48,10 +48,15 @@ func parseOption(code dhcpv4.OptionCode, data []byte) (dhcpv4.Option, error) {
 	return opt, nil
 }
 
+// codeGetter is a dhcpv4.OptionCodeGetter for BSDP optionCodes.
+func codeGetter(c uint8) dhcpv4.OptionCode {
+	return optionCode(c)
+}
+
 // ParseOptVendorSpecificInformation constructs an OptVendorSpecificInformation struct from a sequence of
 // bytes and returns it, or an error.
 func ParseOptVendorSpecificInformation(data []byte) (*OptVendorSpecificInformation, error) {
-	options, err := dhcpv4.OptionsFromBytesWithParser(data, parseOption, false /* don't check for OptionEnd tag */)
+	options, err := dhcpv4.OptionsFromBytesWithParser(data, codeGetter, parseOption, false /* don't check for OptionEnd tag */)
 	if err != nil {
 		return nil, err
 	}

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,33 +181,43 @@ func TestOptVendorSpecificInformationGetOptions(t *testing.T) {
 	o = &OptVendorSpecificInformation{
 		[]dhcpv4.Option{
 			&OptMessageType{MessageTypeList},
-			&OptVersion{Version1_1},
+			Version1_1,
 		},
 	}
 	foundOpts = o.GetOption(OptionMessageType)
 	require.Equal(t, 1, len(foundOpts), "should only get one option")
 	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) {
 	// No option
 	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))
 }

dhcpv4/bsdp/types.go

@@ -1,34 +1,48 @@
 package bsdp
 
-import "github.com/insomniacslk/dhcp/dhcpv4"
-
 // DefaultMacOSVendorClassIdentifier is a default vendor class identifier used
 // on non-darwin hosts where the vendor class identifier cannot be determined.
 // It should mostly be used for debugging if testing BSDP on a non-darwin
 // system.
 const DefaultMacOSVendorClassIdentifier = AppleVendorID + "/i386/MacMini6,1"
 
+// optionCode are BSDP option codes.
+//
+// optionCode implements the dhcpv4.OptionCode interface.
+type optionCode uint8
+
+func (o optionCode) Code() uint8 {
+	return uint8(o)
+}
+
+func (o optionCode) String() string {
+	if s, ok := optionCodeToString[o]; ok {
+		return s
+	}
+	return "unknown"
+}
+
 // Options (occur as sub-options of DHCP option 43).
 const (
-	OptionMessageType                   dhcpv4.OptionCode = 1
-	OptionVersion                       dhcpv4.OptionCode = 2
-	OptionServerIdentifier              dhcpv4.OptionCode = 3
-	OptionServerPriority                dhcpv4.OptionCode = 4
-	OptionReplyPort                     dhcpv4.OptionCode = 5
-	OptionBootImageListPath             dhcpv4.OptionCode = 6 // Not used
-	OptionDefaultBootImageID            dhcpv4.OptionCode = 7
-	OptionSelectedBootImageID           dhcpv4.OptionCode = 8
-	OptionBootImageList                 dhcpv4.OptionCode = 9
-	OptionNetboot1_0Firmware            dhcpv4.OptionCode = 10
-	OptionBootImageAttributesFilterList dhcpv4.OptionCode = 11
-	OptionShadowMountPath               dhcpv4.OptionCode = 128
-	OptionShadowFilePath                dhcpv4.OptionCode = 129
-	OptionMachineName                   dhcpv4.OptionCode = 130
+	OptionMessageType                   optionCode = 1
+	OptionVersion                       optionCode = 2
+	OptionServerIdentifier              optionCode = 3
+	OptionServerPriority                optionCode = 4
+	OptionReplyPort                     optionCode = 5
+	OptionBootImageListPath             optionCode = 6 // Not used
+	OptionDefaultBootImageID            optionCode = 7
+	OptionSelectedBootImageID           optionCode = 8
+	OptionBootImageList                 optionCode = 9
+	OptionNetboot1_0Firmware            optionCode = 10
+	OptionBootImageAttributesFilterList optionCode = 11
+	OptionShadowMountPath               optionCode = 128
+	OptionShadowFilePath                optionCode = 129
+	OptionMachineName                   optionCode = 130
 )
 
 // optionCodeToString maps BSDP OptionCodes to human-readable strings
 // describing what they are.
-var optionCodeToString = map[dhcpv4.OptionCode]string{
+var optionCodeToString = map[optionCode]string{
 	OptionMessageType:                   "BSDP Message Type",
 	OptionVersion:                       "BSDP Version",
 	OptionServerIdentifier:              "BSDP Server Identifier",