lunixbochs / struc

Better binary packing for Go
MIT License
572 stars 45 forks source link

How to specify that one field is used as the length for two distinct slices? #55

Closed mewmew closed 6 years ago

mewmew commented 6 years ago

Note: this issue is mirrored on the restruct issue tracker (https://github.com/go-restruct/restruct/issues/25), as it seems to be present in both struc and restruct.

The file format I've been trying to parse uses a single Length field to specify the length of two distinct slices. However, I've been unable to model this using restruct.

Using the following code did not seem to work:

struct {
    Length uint32 `struc:"sizeof=FrameSizes,sizeof=FrameTypes"`
    FrameSizes []uint32
    FrameTypes []uint8
}

As it gives the following error:

2018/04/08 18:24:29 struc: field `FrameSizes` is a slice with no length or sizeof field

And using this:

struct {
    Length uint32 `struc:"sizeof=FrameSizes"`
    FrameSizes []uint32
    FrameTypes []uint8
}

gives the following error:

2018/04/08 18:24:29 struc: field `FrameTypes` is a slice with no length or sizeof field

Any suggestions? Is it or should it be possible to use reflect for this purpose?

mewmew commented 6 years ago

One way to solve this may be to have the slices define where they get their length from.

E.g.

struct {
    Length uint32
    FrameSizes []uint32 `struct:"size=Length"`
    FrameTypes []uint8 `struct:"size=Length"`
}
lunixbochs commented 6 years ago

The sizefrom keyword does what you're asking iirc. Sizeof only works once, because it's a single attribute on the internal Field struct.

lunixbochs commented 6 years ago

Oh, sizefrom is maybe only tracked internally and doesn't have a struct tag keyword. parseStrucTag could be modified to set it.

lunixbochs commented 6 years ago

You could also make field.Sizeof an array instead of a string so it can handle multiple sizeof tags.

mewmew commented 6 years ago

Hi Ryan,

Thanks for the quick response! I appreciate it.

The sizefrom keyword does what you're asking iirc. Sizeof only works once, because it's a single attribute on the internal Field struct.

Oh, sizefrom is maybe only tracked internally and doesn't have a struct tag keyword. parseStrucTag could be modified to set it.

A sizefrom keyword would work perfectly in my situation I think. If it is not exported, would you accept a PR to export it?

I'm not yet familiar with the internals of the struc code base but would be happy to learn more.

Cheers, /u

lunixbochs commented 6 years ago

You should both try exporting sizefrom and modifying sizeof to be tracked with an array instead of just a string field, then write a simple decode(encode(decode(encode()))) test for each. The tag code is mostly all at the top of parse.go.

mewmew commented 6 years ago

You should both try exporting sizefrom and modifying sizeof to be tracked with an array instead of just a string field, then write a simple decode(encode(decode(encode()))) test for each. The tag code is mostly all at the top of parse.go.

I submitted an initial PR (#56) which seem to work just fine. Your code base was quite intuitive once you got a chance to explore it a bit.

Let me know if I've misinterpreted something, or made some mistakes. At least, it seems to work for the test cases added.

Cheers /u

mewmew commented 6 years ago

Note from the test case, the canonical example usage refers to Size4 from two distinct slices Str4a and Str4b.

struct {
...
    Size4 int    `struc:"little"`                // 07 00 00 00
    Str4a string `struc:"[]byte,sizefrom=Size4"` // "ijklmno"
    Str4b string `struc:"[]byte,sizefrom=Size4"` // "pqrstuv"
...
}
mewmew commented 6 years ago

Just as a side note, with #56, struc is now capable of parsing the original example perfectly.

struct {
    Length     int     `struc:"uint32,little"`
    FrameSizes []int   `struc:"[]uint32,little,sizefrom=Length"`
    FrameTypes []uint8 `struc:"sizefrom=Length"`
}
    FrameSizes: {0, 33368, 2572, 2544, 2616, 2648, ...},
    FrameTypes: {0x3c, 0x1, 0x0, 0x0, ...},