fixes according to github review

Signed-off-by: Christopher Meis <christopher.meis@9elements.com>

pkg/ipmi/constants.go

@@ -1,4 +1,4 @@
-// Copyright 2019 the u-root Authors. All rights reserved
+// Copyright 2019-2021 the u-root Authors. All rights reserved
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 

pkg/ipmi/ipmi_dev.go → pkg/ipmi/dev.go

@@ -1,4 +1,4 @@
-// Copyright 2019 the u-root Authors. All rights reserved
+// Copyright 2019-2021 the u-root Authors. All rights reserved
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
@@ -18,41 +18,36 @@ import (
 
 type Dev struct {
 	*os.File
-}
-
-var (
 	// unixSyscall gives the option to overwrite the actual sysCall with a dummy function when writing tests.
-	unixSyscall = unix.Syscall
+	unixSyscall func(uintptr, uintptr, uintptr, uintptr) (uintptr, uintptr, unix.Errno)
 
 	// fileSyscallConn gives the option to overwrite the actual function call with a dummy function when writing tests.
-	fileSyscallConn = func(f *os.File) (syscall.RawConn, error) {
-		return f.SyscallConn()
-	}
+	fileSyscallConn func(f *os.File) (syscall.RawConn, error)
 
 	// fileSetReadDeadline gives the option to overwrite the actual function call with a dummy function when writing tests.
-	fileSetReadDeadline = func(f *os.File, t time.Duration) error {
-		return f.SetReadDeadline(time.Now().Add(t))
-	}
+	fileSetReadDeadline func(f *os.File, t time.Duration) error
+
 	// connRead gives the option to overwrite the actual function call with a dummy function when writing tests.
-	connRead = func(f func(fd uintptr) bool, conn syscall.RawConn) error {
-		return conn.Read(f)
-	}
-)
+	connRead func(f func(fd uintptr) bool, conn syscall.RawConn) error
+}
 
+// SendRequest uses unix.Syscall IOCTL to send a request to the BMC.
 func (d *Dev) SendRequest(req *request) error {
-	_, _, err := unixSyscall(unix.SYS_IOCTL, d.File.Fd(), _IPMICTL_SEND_COMMAND, uintptr(unsafe.Pointer(req)))
+	_, _, err := d.unixSyscall(unix.SYS_IOCTL, d.File.Fd(), _IPMICTL_SEND_COMMAND, uintptr(unsafe.Pointer(req)))
 	runtime.KeepAlive(req)
 	if err != 0 {
 		return fmt.Errorf("syscall failed with: %v", err.Error())
 	}
 	return nil
 }
 
+// ReceiveResponse uses syscall Rawconn to read a response via unix.Syscall IOCTL from the BMC.
+// It takes the message ID of the request and awaits the response with the same message ID.
 func (d *Dev) ReceiveResponse(msgID int64, resp *response, buf []byte) ([]byte, error) {
 	var result []byte
 	var rerr error
 	readMsg := func(fd uintptr) bool {
-		_, _, err := unixSyscall(unix.SYS_IOCTL, d.File.Fd(), _IPMICTL_RECEIVE_MSG_TRUNC, uintptr(unsafe.Pointer(resp)))
+		_, _, err := d.unixSyscall(unix.SYS_IOCTL, d.File.Fd(), _IPMICTL_RECEIVE_MSG_TRUNC, uintptr(unsafe.Pointer(resp)))
 		runtime.KeepAlive(resp)
 		if err != 0 {
 			rerr = fmt.Errorf("ioctlGetRecv failed with %v", err)
@@ -76,24 +71,38 @@ func (d *Dev) ReceiveResponse(msgID int64, resp *response, buf []byte) ([]byte,
 	}
 
 	// Read response.
-	conn, err := fileSyscallConn(d.GetFile())
+	conn, err := d.fileSyscallConn(d.GetFile())
 	if err != nil {
 		return nil, fmt.Errorf("failed to get file rawconn: %v", err)
 	}
-	if err := fileSetReadDeadline(d.GetFile(), timeout); err != nil {
+	if err := d.fileSetReadDeadline(d.GetFile(), timeout); err != nil {
 		return nil, fmt.Errorf("failed to set read deadline: %v", err)
 	}
-	if err := connRead(readMsg, conn); err != nil {
+	if err := d.connRead(readMsg, conn); err != nil {
 		return nil, fmt.Errorf("failed to read rawconn: %v", err)
 	}
 
 	return result, rerr
 }
 
+// GetFile returns the file of Dev
 func (d *Dev) GetFile() *os.File {
 	return d.File
 }
 
+// GetDev takes a file and returns a new Dev
 func GetDev(f *os.File) *Dev {
-	return &Dev{File: f}
+	return &Dev{
+		File:        f,
+		unixSyscall: unix.Syscall,
+		fileSyscallConn: func(f *os.File) (syscall.RawConn, error) {
+			return f.SyscallConn()
+		},
+		fileSetReadDeadline: func(f *os.File, t time.Duration) error {
+			return f.SetReadDeadline(time.Now().Add(t))
+		},
+		connRead: func(f func(fd uintptr) bool, conn syscall.RawConn) error {
+			return conn.Read(f)
+		},
+	}
 }

pkg/ipmi/ipmi_dev_test.go → pkg/ipmi/dev_test.go

@@ -24,7 +24,7 @@ func TestSendRequestSuccess(t *testing.T) {
 	}
 	req := &request{}
 
-	unixSyscall = func(trap, a1, a2, a3 uintptr) (uintptr, uintptr, syscall.Errno) {
+	d.unixSyscall = func(trap, a1, a2, a3 uintptr) (uintptr, uintptr, syscall.Errno) {
 		return 0, 0, 0
 	}
 	if err := d.SendRequest(req); err != nil {
@@ -44,7 +44,7 @@ func TestSendRequestFailWithSyscall(t *testing.T) {
 	}
 	req := &request{}
 
-	unixSyscall = func(trap, a1, a2, a3 uintptr) (uintptr, uintptr, syscall.Errno) {
+	d.unixSyscall = func(trap, a1, a2, a3 uintptr) (uintptr, uintptr, syscall.Errno) {
 		return 0, 0, 1
 	}
 	if err := d.SendRequest(req); err == nil {
@@ -59,17 +59,15 @@ func TestReceiveResponseSuccess(t *testing.T) {
 	}
 	defer os.RemoveAll(df.Name())
 	// We need a dummy dev struct
-	d := Dev{
-		File: df,
-	}
+	d := GetDev(df)
 
 	resp := &response{}
 	buf := make([]byte, 1)
-	unixSyscall = func(trap, a1, a2, a3 uintptr) (uintptr, uintptr, syscall.Errno) {
+	d.unixSyscall = func(trap, a1, a2, a3 uintptr) (uintptr, uintptr, syscall.Errno) {
 		return 0, 0, 0
 	}
 
-	fileSetReadDeadline = func(f *os.File, t time.Duration) error {
+	d.fileSetReadDeadline = func(f *os.File, t time.Duration) error {
 		return nil
 	}
 
@@ -86,20 +84,18 @@ func TestReceiveResponseFailWithMsgID(t *testing.T) {
 	}
 	defer os.RemoveAll(df.Name())
 	// We need a dummy dev struct
-	d := Dev{
-		File: df,
-	}
+	d := GetDev(df)
 
 	// Message ID is set to 1, but will be expected to be zero later.
 	resp := &response{
 		msgid: 1,
 	}
 	buf := make([]byte, 1)
-	unixSyscall = func(trap, a1, a2, a3 uintptr) (uintptr, uintptr, syscall.Errno) {
+	d.unixSyscall = func(trap, a1, a2, a3 uintptr) (uintptr, uintptr, syscall.Errno) {
 		return 0, 0, 0
 	}
 
-	fileSetReadDeadline = func(f *os.File, t time.Duration) error {
+	d.fileSetReadDeadline = func(f *os.File, t time.Duration) error {
 		return nil
 	}
 
@@ -117,19 +113,17 @@ func TestReceiveResponseFailWithSyscall(t *testing.T) {
 	}
 	defer os.RemoveAll(df.Name())
 	// We need a dummy dev struct
-	d := Dev{
-		File: df,
-	}
+	d := GetDev(df)
 
 	resp := &response{
 		msgid: 0,
 	}
 	buf := make([]byte, 1)
-	unixSyscall = func(trap, a1, a2, a3 uintptr) (uintptr, uintptr, syscall.Errno) {
+	d.unixSyscall = func(trap, a1, a2, a3 uintptr) (uintptr, uintptr, syscall.Errno) {
 		return 0, 0, 1
 	}
 
-	fileSetReadDeadline = func(f *os.File, t time.Duration) error {
+	d.fileSetReadDeadline = func(f *os.File, t time.Duration) error {
 		return nil
 	}
 

pkg/ipmi/ipmi_handler.go → pkg/ipmi/handler.go

@@ -1,4 +1,4 @@
-// Copyright 2019 the u-root Authors. All rights reserved
+// Copyright 2019-2021 the u-root Authors. All rights reserved
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
@@ -8,7 +8,7 @@ import (
 	"os"
 )
 
-type IPMIHandler interface {
+type Handler interface {
 	SendRequest(*request) error
 	ReceiveResponse(int64, *response, []byte) ([]byte, error)
 	GetFile() *os.File

pkg/ipmi/ipmi.go

@@ -1,4 +1,4 @@
-// Copyright 2019 the u-root Authors. All rights reserved
+// Copyright 2019-2021 the u-root Authors. All rights reserved
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
@@ -30,7 +30,7 @@ var (
 
 // IPMI represents access to the IPMI interface.
 type IPMI struct {
-	IPMIHandler
+	Handler
 }
 
 // SendRecv sends the IPMI message, receives the response, and returns the
@@ -66,7 +66,7 @@ func (i *IPMI) RawSendRecv(msg Msg) ([]byte, error) {
 
 	// Send request.
 	for {
-		switch err := i.IPMIHandler.SendRequest(req); {
+		switch err := i.Handler.SendRequest(req); {
 		case err == syscall.EINTR:
 			continue
 		case err != nil:
@@ -86,7 +86,7 @@ func (i *IPMI) RawSendRecv(msg Msg) ([]byte, error) {
 		msg:     recvMsg,
 	}
 
-	data, err := i.IPMIHandler.ReceiveResponse(req.msgid, recv, buf)
+	data, err := i.Handler.ReceiveResponse(req.msgid, recv, buf)
 	if err != nil {
 		return nil, err
 	}
@@ -336,5 +336,5 @@ func (i *IPMI) RawCmd(param []byte) ([]byte, error) {
 }
 
 func (i *IPMI) Close() error {
-	return i.IPMIHandler.GetFile().Close()
+	return i.Handler.GetFile().Close()
 }

pkg/ipmi/ipmi_test.go

@@ -17,7 +17,7 @@ func TestWatchdogRunning(t *testing.T) {
 
 }
 
-func TestShutiffWatchdog(t *testing.T) {
+func TestShutoffWatchdog(t *testing.T) {
 	i := GetMockIPMI()
 	if err := i.ShutoffWatchdog(); err != nil {
 		t.Error(err)

pkg/ipmi/ipmi_linux.go → pkg/ipmi/linux.go

@@ -1,7 +1,9 @@
-// Copyright 2019 the u-root Authors. All rights reserved
+// Copyright 2019-2021 the u-root Authors. All rights reserved
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
+//go:build linux
+
 package ipmi
 
 import (
@@ -18,5 +20,5 @@ func Open(devnum int) (*IPMI, error) {
 		return nil, err
 	}
 
-	return &IPMI{IPMIHandler: GetDev(f)}, nil
+	return &IPMI{Handler: GetDev(f)}, nil
 }

pkg/ipmi/mock.go

@@ -0,0 +1,47 @@
+// Copyright 2019-2021 the u-root Authors. All rights reserved
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package ipmi
+
+import (
+	"os"
+	"syscall"
+	"time"
+
+	"golang.org/x/sys/unix"
+)
+
+type mock struct {
+	unixSyscall func(uintptr, uintptr, uintptr, uintptr) (uintptr, uintptr, unix.Errno)
+
+	// fileSyscallConn gives the option to overwrite the actual function call with a dummy function when writing tests.
+	fileSyscallConn func(f *os.File) (syscall.RawConn, error)
+
+	// fileSetReadDeadline gives the option to overwrite the actual function call with a dummy function when writing tests.
+	fileSetReadDeadline func(f *os.File, t time.Duration) error
+
+	// connRead gives the option to overwrite the actual function call with a dummy function when writing tests.
+	connRead func(f func(fd uintptr) bool, conn syscall.RawConn) error
+}
+
+func (m *mock) SendRequest(req *request) error {
+	return nil
+}
+
+func (m *mock) ReceiveResponse(msgID int64, resp *response, buf []byte) ([]byte, error) {
+	return buf, nil
+}
+
+func (m *mock) GetFile() *os.File {
+	return nil
+}
+
+func GetMockIPMI() *IPMI {
+	return &IPMI{Handler: &mock{
+		unixSyscall:         nil,
+		fileSyscallConn:     nil,
+		fileSetReadDeadline: nil,
+		connRead:            nil,
+	}}
+}

pkg/ipmi/structures.go

@@ -1,4 +1,4 @@
-// Copyright 2019 the u-root Authors. All rights reserved
+// Copyright 2019-2021 the u-root Authors. All rights reserved
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
@@ -66,7 +66,7 @@ type OEMTsEvent struct {
 // OEMNonTsEvent is a non-timestamped OEM-custom event.
 //
 // It holds 13 bytes of OEM-defined arbitrary data.
-type OEMNontsEvent struct {
+type OEMNonTsEvent struct {
 	OEMNontsDefinedData [13]uint8
 }
 
@@ -78,7 +78,7 @@ type Event struct {
 	RecordType uint8
 	StandardEvent
 	OEMTsEvent
-	OEMNontsEvent
+	OEMNonTsEvent
 }
 
 type setSystemInfoReq struct {
@@ -87,6 +87,7 @@ type setSystemInfoReq struct {
 	strData       [_SYSTEM_INFO_BLK_SZ]byte
 }
 
+// DevID holds information of a Device provided by the BMC via IPMI
 type DevID struct {
 	DeviceID          byte
 	DeviceRevision    byte
@@ -99,13 +100,15 @@ type DevID struct {
 	AuxFwRev          [4]byte
 }
 
+// CahssisStatus holds information about status of the chassis reported by the BMC via IPMI
 type ChassisStatus struct {
 	CurrentPowerState byte
 	LastPowerEvent    byte
 	MiscChassisState  byte
 	FrontPanelButton  byte
 }
 
+// SELInfo holds information about System Event Log reported by the BMV via IPMI
 type SELInfo struct {
 	Version     byte
 	Entries     uint16