Skip to content
This repository was archived by the owner on May 10, 2024. It is now read-only.

Conversation

@edaniszewski
Copy link

Using ipmitool (version 1.8.16) to control chassis identify will result in multiple lines of raw response to be returned

$ ipmitool -H 127.0.0.1 -U ADMIN -P ADMIN -I lanplus raw 0x00 0x04 0x01 0x00
 7f 00 00 90 4a 57 f8 fd 7f 00 00 b0 49 57 f8 fd
 7f 00 00 90 4a 57 f8 fd 7f 00 00 20 f1 b9 8a 8c
 55 00 00

The newline in this response is not handled, so when parsed/decoded, it panics - as shown by the regression tests.

--- FAIL: TestResponseFromString (0.00s)
panic: encoding/hex: odd length hex string [recovered]
	panic: encoding/hex: odd length hex string

goroutine 70 [running]:
testing.tRunner.func1(0xc4201f6000)
	/usr/local/Cellar/go/1.9.1/libexec/src/testing/testing.go:711 +0x2d2
panic(0x12a4340, 0xc420076560)
	/usr/local/Cellar/go/1.9.1/libexec/src/runtime/panic.go:491 +0x283
github.com/vapor-ware/goipmi.rawDecode(0x130fa6d, 0x68, 0x130fa6d, 0x68, 0x12c3800)
	/Users/edaniszewski/go/src/github.com/vapor-ware/goipmi/tool.go:148 +0x18e
github.com/vapor-ware/goipmi.responseFromString(0x130fa6c, 0x69, 0x1456d80, 0xc4201b1461, 0xc42003af08, 0x1)
	/Users/edaniszewski/go/src/github.com/vapor-ware/goipmi/tool.go:138 +0x51
github.com/vapor-ware/goipmi.TestResponseFromString(0xc4201f6000)
	/Users/edaniszewski/go/src/github.com/vapor-ware/goipmi/tool_test.go:186 +0xd7
testing.tRunner(0xc4201f6000, 0x13135d0)
	/usr/local/Cellar/go/1.9.1/libexec/src/testing/testing.go:746 +0xd0
created by testing.(*T).Run
	/usr/local/Cellar/go/1.9.1/libexec/src/testing/testing.go:789 +0x2de
exit status 2

With these changes, newlines are stripped out of the response prior to further parsing/decoding, so newline-separated byte list gets handles as a continuous list of bytes. Tests should pass:

$ go test
PASS
ok  	github.com/vapor-ware/goipmi	0.075s

@vmwclabot
Copy link

@edaniszewski, you must sign our contributor license agreement before your changes are merged. Click here to sign the agreement. If you are a VMware employee, read this for further instruction.

Copy link
Contributor

@dougm dougm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks @edaniszewski

We can merge once @vmwclabot approves.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants