go-restruct / restruct

Rich binary (de)serialization library for Golang
https://restruct.io/
ISC License
360 stars 17 forks source link

[Question] use of if expression for dynamically sized field #40

Closed SterlingAr closed 4 years ago

SterlingAr commented 4 years ago

Hello, I have an problem with a struct that has a dynamically sized byte slice, that depends on another field within the struct.

This is the struct, within comments is the original struct i'm trying to port to golang:


//struct PROTO_NC_BRIEFINFO_REGENMOB_CMD
//{
//  unsigned __int16 handle;
//  char mode;
//  unsigned __int16 mobid;
//  COORD_TYPE coord;
//  char flagstate;
//  PROTO_NC_BRIEFINFO_REGENMOB_CMD::<unnamed-type-flag> flag;
//  char sAnimation[32];
//  char nAnimationLevel;
//  char nKQTeamType;
//  char bRegenAni;
//};
type NcBriefInfoRegenMobCmd struct {
    Handle uint16
    Mode byte
    MobID uint16
    Coord CoordType
    // 0,1
    FlagState byte

    //union PROTO_NC_BRIEFINFO_REGENMOB_CMD::<unnamed-type-flag>
    //{
    //  char statebit[103];
    //  char gate2where[12];
    //};
    FlagData []byte `struct-size:"FlagState == 0 ? 103 : 12"`

    Animation [32]byte
    AnimationLevel byte
    KQTeamType byte
    RegenAni byte
}

As far as I understand, the amount of bytes I need to read is indicated by the flagstate field, which can be 0 or 1 (according to reverse engineered code that uses this struct).

My first thought was to use the expression language for this, but I cannot make it work, i've looked through the code but I can't find a way to combine if expression with size.

Here is a full example with real packet data that I use to populate the struct: https://play.golang.org/p/G6RxwzjY6bI

I would like to know if what I am trying to do is possible with restruct or if I should tackle this struct in particular manually.

PD: by the way, awesome library!, i'm using right now to make an emulator and i'm porting packet structures with real ease thanks to it.

jchv commented 4 years ago

Looks like a bug. I believe what's happening here is that the addition of expressions that can make a field non-trivial broke the logic for isTypeTrivial.

The isTypeTrivial function is used to allow for optimizations that significantly improve operations on many kinds of arrays where the element size is static - what's happening here is the size of the array is being calculated based on the size of the first array element because restruct believes it is "trivial" (or constant-sized) when it is not.

I think isTypeTrivial now needs to check if the expression is trivial before deciding if the struct type is trivial. I'll try to get that fixed shortly.

edit: Although I'm not 100% sure. Normally a slice is considered not trivial, so this is actually a bit strange really.

SterlingAr commented 4 years ago

Ok, so I managed to solve it...

I must apologize for opening this issue when I was doing something wrong on my end.

I was under the wrong impression that the size of the struct was dynamic and that was not the case, since its a union it'll always take 103 bytes. So I have no need to use restruct-size

And also the actual size for this was 99, not 103.

Now it works: https://play.golang.org/p/lESfqV0bvTX

I haven't used much restruct expressions, so I don't know if the struct-size example i was using is correct.

jchv commented 4 years ago

Oh, cool, glad to hear. Yes that makes sense. I think the struct-size usage was fine, but I am curious if there is a bug in restruct with struct-size as a field of a struct in a slice.