Fix arrayField unpack segfault

[sknat]

The following snippet generates a segfault. I made this fix proposal & added the corresponding test.

Feel free to comment :)

package main

import (
    "bytes"
    "fmt"
    "github.com/lunixbochs/struc"
)

// FibPath represents VPP binary API type 'fib_path'.
type SomeStruct struct {
    a  uint32
}

type Example struct {
    ExampleProps      [1]SomeStruct `struc:"[1]SomeStruct"`
}

func main() {
    fmt.Printf("Hello\n")
    var buf bytes.Buffer
    t := &Example{[1]SomeStruct{}}
    _ = struc.Pack(&buf, t)
    o := &Example{}
    _ = struc.Unpack(&buf, o)
}

panic: reflect.MakeSlice of non-slice type
...
github.com/lunixbochs/struc.Fields.Unpack(0xc0000a00c0, 0x1, 0x1, 0x1193f80, 0xc000092b10, 0x113e6e0, 0xc0000a75b4, 0x199, 0x127afa0, 0x127bbe0, ...)
        /Users/.../struc/fields.go:125 +0x1cf
github.com/lunixbochs/struc.UnpackWithOptions(0x1193f80, 0xc000092b10, 0x1127e20, 0xc0000a75b4, 0x127afa0, 0x0, 0x0)
        /Users/.../struc/struc.go:103 +0xe1
github.com/lunixbochs/struc.Unpack(...)
        /Users/.../struc/struc.go:89
github.com/lunixbochs/struc.TestStrucArray(0xc0000dec60)
        /Users/.../struc/fields_test.go:77 +0x122

Signed-off-by: Nathan Skrzypczak nathan.skrzypczak@gmail.com

[sknat]

Hi @lunixbochs Any thoughts on this ?

[lunixbochs]

Please run gofmt on your changed files and force push a commit with test.go and the test binary removed

[sknat]

Right, sorry I went a bit quick pushing this. Things should be better now

[lunixbochs]

I’m unsure about the performance of creating innerV in a loop. You can probably get a reference to the elements of the array and unpack directly to those, or even take a slice of the array and treat it like a slice.

It might be worth adding a benchmark and playing with some options: https://github.com/lunixbochs/struc/blob/master/bench_test.go

[sknat]

Hi @lunixbochs sorry for the delayed reply, I got dragged onto other things.

Right for the performance of the of the innerV in the loop, didn't figure out a way to reflect Array in the first place. Added benchmarks for slices & arrays to the PR with comparable results on my machine

BenchmarkArrayEncode-4            576452              1858 ns/op
BenchmarkSliceEncode-4            581444              1774 ns/op
BenchmarkArrayDecode-4            430996              2401 ns/op
BenchmarkSliceDecode-4            429949              2356 ns/op

[sknat]

Hi @lunixbochs Did you have a chance to take a look at the updated patch ? Thanks in advance !

[sknat]

Hi @lunixbochs sorry to push for this merge, but we're moving to a more 'official' repo for our project. We'd be better off without a "replace" line in our go.mod 😄

[lunixbochs]

I just merged manually with a minor change to avoid allocating an intermediate array.

[sknat]

Great thanks a lot