From 6af42ee6f99e86a9312736873375f9351b30dd0d Mon Sep 17 00:00:00 2001 From: Lijiajie Date: Sat, 23 Jan 2021 21:04:35 +0800 Subject: [PATCH 1/2] make it possible to build a dynamic length frame --- fields.go | 23 +++++++++++++++++++++++ parse.go | 30 ++++++++++++++++++++++++++++- parse_test.go | 18 +++++++++--------- struc.go | 8 ++++++++ struc_test.go | 52 +++++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 121 insertions(+), 10 deletions(-) diff --git a/fields.go b/fields.go index 5d591bf..4838d73 100644 --- a/fields.go +++ b/fields.go @@ -103,26 +103,46 @@ func (f Fields) Pack(buf []byte, val reflect.Value, options *Options) (int, erro } func (f Fields) Unpack(r io.Reader, val reflect.Value, options *Options) error { + for val.Kind() == reflect.Ptr { val = val.Elem() } + var tmp [8]byte var buf []byte + for i, field := range f { + if field == nil { continue } + v := val.Field(i) + length := field.Len if field.Sizefrom != nil { length = f.sizefrom(val, field.Sizefrom) } + + if length == -1 { + + length = v.Len() + + if length == 0 { + return fmt.Errorf("struc: field `%s` is a slice with no length or sizeof field", field.Name) + } + } + if v.Kind() == reflect.Ptr && !v.Elem().IsValid() { v.Set(reflect.New(v.Type().Elem())) } + if field.Type == Struct { + if field.Slice { + vals := reflect.MakeSlice(v.Type(), length, length) + for i := 0; i < length; i++ { v := vals.Index(i) fields, err := parseFields(v) @@ -134,7 +154,9 @@ func (f Fields) Unpack(r io.Reader, val reflect.Value, options *Options) error { } } v.Set(vals) + } else { + // TODO: DRY (we repeat the inner loop above) fields, err := parseFields(v) if err != nil { @@ -146,6 +168,7 @@ func (f Fields) Unpack(r io.Reader, val reflect.Value, options *Options) error { } continue } else { + typ := field.Type.Resolve(options) if typ == CustomType { if err := v.Addr().Interface().(Custom).Unpack(r, length, options); err != nil { diff --git a/parse.go b/parse.go index 43c5875..a248a67 100644 --- a/parse.go +++ b/parse.go @@ -120,28 +120,39 @@ func parseField(f reflect.StructField) (fd *Field, tag *strucTag, err error) { } func parseFieldsLocked(v reflect.Value) (Fields, error) { + // we need to repeat this logic because parseFields() below can't be recursively called due to locking for v.Kind() == reflect.Ptr { v = v.Elem() } + t := v.Type() + if v.NumField() < 1 { return nil, errors.New("struc: Struct has no fields.") } + sizeofMap := make(map[string][]int) fields := make(Fields, v.NumField()) + for i := 0; i < t.NumField(); i++ { + field := t.Field(i) + f, tag, err := parseField(field) + if tag.Skip { continue } + if err != nil { return nil, err } + if !v.Field(i).CanSet() { continue } + f.Index = i if tag.Sizeof != "" { target, ok := t.FieldByName(tag.Sizeof) @@ -151,9 +162,11 @@ func parseFieldsLocked(v reflect.Value) (Fields, error) { f.Sizeof = target.Index sizeofMap[tag.Sizeof] = field.Index } + if sizefrom, ok := sizeofMap[field.Name]; ok { f.Sizefrom = sizefrom } + if tag.Sizefrom != "" { source, ok := t.FieldByName(tag.Sizefrom) if !ok { @@ -161,9 +174,14 @@ func parseFieldsLocked(v reflect.Value) (Fields, error) { } f.Sizefrom = source.Index } + if f.Len == -1 && f.Sizefrom == nil { - return nil, fmt.Errorf("struc: field `%s` is a slice with no length or sizeof field", field.Name) + // f.Len = v.Field(i).Len() + // if f.Len == 0 { + // return nil, fmt.Errorf("struc: field `%s` is a slice with no length or sizeof field", field.Name) + // } } + // recurse into nested structs // TODO: handle loops (probably by indirecting the []Field and putting pointer in cache) if f.Type == Struct { @@ -179,8 +197,10 @@ func parseFieldsLocked(v reflect.Value) (Fields, error) { return nil, err } } + fields[i] = f } + return fields, nil } @@ -189,18 +209,23 @@ var fieldCacheLock sync.RWMutex var parseLock sync.Mutex func fieldCacheLookup(t reflect.Type) Fields { + fieldCacheLock.RLock() defer fieldCacheLock.RUnlock() + if cached, ok := fieldCache[t]; ok { return cached } + return nil } func parseFields(v reflect.Value) (Fields, error) { + for v.Kind() == reflect.Ptr { v = v.Elem() } + t := v.Type() // fast path: hopefully the field parsing is already cached @@ -220,11 +245,14 @@ func parseFields(v reflect.Value) (Fields, error) { // no luck, time to parse and fill the cache ourselves fields, err := parseFieldsLocked(v) + if err != nil { return nil, err } + fieldCacheLock.Lock() fieldCache[t] = fields fieldCacheLock.Unlock() + return fields, nil } diff --git a/parse_test.go b/parse_test.go index 861fdd1..7dde2e9 100644 --- a/parse_test.go +++ b/parse_test.go @@ -40,15 +40,15 @@ func TestBadSizeof(t *testing.T) { } } -type missingSize struct { - Test []byte -} - -func TestMissingSize(t *testing.T) { - if err := parseTest(&missingSize{}); err == nil { - t.Fatal("failed to error on missing field size") - } -} +// type missingSize struct { +// Test []byte +// } + +// func TestMissingSize(t *testing.T) { +// if err := parseTest(&missingSize{}); err == nil { +// t.Fatal("failed to error on missing field size") +// } +// } type badNested struct { Empty empty diff --git a/struc.go b/struc.go index 3d85fe3..4a327f0 100644 --- a/struc.go +++ b/struc.go @@ -29,7 +29,9 @@ func (o *Options) Validate() error { var emptyOptions = &Options{} func prep(data interface{}) (reflect.Value, Packer, error) { + value := reflect.ValueOf(data) + for value.Kind() == reflect.Ptr { next := value.Elem().Kind() if next == reflect.Struct || next == reflect.Ptr { @@ -38,17 +40,23 @@ func prep(data interface{}) (reflect.Value, Packer, error) { break } } + switch value.Kind() { case reflect.Struct: + fields, err := parseFields(value) return value, fields, err + default: + if !value.IsValid() { return reflect.Value{}, nil, fmt.Errorf("Invalid reflect.Value for %+v", data) } + if c, ok := data.(Custom); ok { return value, customFallback{c}, nil } + return value, binaryFallback(value), nil } } diff --git a/struc_test.go b/struc_test.go index fe0b103..d3a2cb7 100644 --- a/struc_test.go +++ b/struc_test.go @@ -5,6 +5,7 @@ import ( "encoding/binary" "fmt" "reflect" + "strings" "testing" ) @@ -229,3 +230,54 @@ func TestSliceUnderrun(t *testing.T) { t.Fatal(err) } } + +type dynamic struct { + Buf []byte +} + +func TestParse(t *testing.T) { + + { + dm := &dynamic{Buf: make([]byte, 3)} + + r := strings.NewReader("123") + + err := Unpack(r, dm) + + if string(dm.Buf) != "123" { + t.Fatal("invalid len") + } + + if err != nil { + t.Fatal("unable to parse this one!") + } + } + + { + + dm := &dynamic{Buf: make([]byte, 2)} + r := strings.NewReader("123") + + err := Unpack(r, dm) + + if string(dm.Buf) != "12" { + t.Fatal("invalid len") + } + + if err != nil { + t.Fatal("unable to parse this one!") + } + } + + { + + dm := &dynamic{} + r := strings.NewReader("123") + + err := Unpack(r, dm) + + if err == nil { + t.Fatal("missing size of []byte") + } + } +} From 78d9a4f84e6a7aac487d840ec8c76b7c9a84d31a Mon Sep 17 00:00:00 2001 From: Lijiajie Date: Sat, 23 Jan 2021 21:31:59 +0800 Subject: [PATCH 2/2] make it possible to build && unpack a dynamic length frame --- fields.go | 6 +++--- struc_test.go | 17 +++++++++++++---- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/fields.go b/fields.go index 6437637..e3e4aec 100644 --- a/fields.go +++ b/fields.go @@ -128,9 +128,9 @@ func (f Fields) Unpack(r io.Reader, val reflect.Value, options *Options) error { length = v.Len() - if length == 0 { - return fmt.Errorf("struc: field `%s` is a slice with no length or sizeof field", field.Name) - } + // if length == 0 { + // return fmt.Errorf("struc: field `%s` is a slice with no length or sizeof field", field.Name) + // } } if v.Kind() == reflect.Ptr && !v.Elem().IsValid() { diff --git a/struc_test.go b/struc_test.go index bd55cc2..07836fa 100644 --- a/struc_test.go +++ b/struc_test.go @@ -312,6 +312,7 @@ func TestSliceUnderrun(t *testing.T) { type dynamic struct { Buf []byte + V int } func TestParse(t *testing.T) { @@ -319,7 +320,7 @@ func TestParse(t *testing.T) { { dm := &dynamic{Buf: make([]byte, 3)} - r := strings.NewReader("123") + r := strings.NewReader("1231234") err := Unpack(r, dm) @@ -335,7 +336,7 @@ func TestParse(t *testing.T) { { dm := &dynamic{Buf: make([]byte, 2)} - r := strings.NewReader("123") + r := strings.NewReader("1231234") err := Unpack(r, dm) @@ -351,12 +352,20 @@ func TestParse(t *testing.T) { { dm := &dynamic{} - r := strings.NewReader("123") + r := strings.NewReader("12334") err := Unpack(r, dm) - if err == nil { + if err != nil { t.Fatal("missing size of []byte") } + + if len(dm.Buf) != 0 { + t.Fatal("mismatch size of []byte") + } + + if dm.V == 0 { + t.Fatal("mismatch size of []byte") + } } }