From 4622e17bccc7c40a2698e8314d29bbde87090cec Mon Sep 17 00:00:00 2001 From: Ayush Ranjan Date: Mon, 8 Nov 2021 00:37:24 -0800 Subject: Simplify {Un}MarshalUnsafeSlice method signatures. Earlier this function was returning (int, error) much like the Copy{In/Out} methods. The returned error was always nil. The returned int was never used. Instead make it returned the shifted buffer which is more useful. Updates #6450 PiperOrigin-RevId: 408268327 --- pkg/lisafs/message.go | 52 +++++----------------- pkg/lisafs/sample_message.go | 12 +---- pkg/marshal/marshal.go | 4 +- .../generator_interfaces_primitive_newtype.go | 20 ++++----- .../gomarshal/generator_interfaces_struct.go | 33 +++++++------- 5 files changed, 42 insertions(+), 79 deletions(-) diff --git a/pkg/lisafs/message.go b/pkg/lisafs/message.go index 0d7c30ce3..c5474d804 100644 --- a/pkg/lisafs/message.go +++ b/pkg/lisafs/message.go @@ -307,11 +307,7 @@ func (m *MountResp) MarshalBytes(dst []byte) []byte { dst = m.MaxMessageSize.MarshalUnsafe(dst) numSupported := primitive.Uint16(len(m.SupportedMs)) dst = numSupported.MarshalBytes(dst) - n, err := MarshalUnsafeMIDSlice(m.SupportedMs, dst) - if err != nil { - panic(err) - } - return dst[n:] + return MarshalUnsafeMIDSlice(m.SupportedMs, dst) } // UnmarshalBytes implements marshal.Marshallable.UnmarshalBytes. @@ -320,12 +316,12 @@ func (m *MountResp) UnmarshalBytes(src []byte) []byte { src = m.MaxMessageSize.UnmarshalUnsafe(src) var numSupported primitive.Uint16 src = numSupported.UnmarshalBytes(src) - m.SupportedMs = make([]MID, numSupported) - n, err := UnmarshalUnsafeMIDSlice(m.SupportedMs, src) - if err != nil { - panic(err) + if cap(m.SupportedMs) < int(numSupported) { + m.SupportedMs = make([]MID, numSupported) + } else { + m.SupportedMs = m.SupportedMs[:numSupported] } - return src[n:] + return UnmarshalUnsafeMIDSlice(m.SupportedMs, src) } // ChannelResp is the response to the create channel request. @@ -440,11 +436,7 @@ func (w *WalkResp) MarshalBytes(dst []byte) []byte { numInodes := primitive.Uint32(len(w.Inodes)) dst = numInodes.MarshalUnsafe(dst) - n, err := MarshalUnsafeInodeSlice(w.Inodes, dst) - if err != nil { - panic(err) - } - return dst[n:] + return MarshalUnsafeInodeSlice(w.Inodes, dst) } // UnmarshalBytes implements marshal.Marshallable.UnmarshalBytes. @@ -459,11 +451,7 @@ func (w *WalkResp) UnmarshalBytes(src []byte) []byte { } else { w.Inodes = w.Inodes[:numInodes] } - n, err := UnmarshalUnsafeInodeSlice(w.Inodes, src) - if err != nil { - panic(err) - } - return src[n:] + return UnmarshalUnsafeInodeSlice(w.Inodes, src) } // WalkStatResp is used to communicate stat results for WalkStat. @@ -481,11 +469,7 @@ func (w *WalkStatResp) MarshalBytes(dst []byte) []byte { numStats := primitive.Uint32(len(w.Stats)) dst = numStats.MarshalUnsafe(dst) - n, err := linux.MarshalUnsafeStatxSlice(w.Stats, dst) - if err != nil { - panic(err) - } - return dst[n:] + return linux.MarshalUnsafeStatxSlice(w.Stats, dst) } // UnmarshalBytes implements marshal.Marshallable.UnmarshalBytes. @@ -498,11 +482,7 @@ func (w *WalkStatResp) UnmarshalBytes(src []byte) []byte { } else { w.Stats = w.Stats[:numStats] } - n, err := linux.UnmarshalUnsafeStatxSlice(w.Stats, src) - if err != nil { - panic(err) - } - return src[n:] + return linux.UnmarshalUnsafeStatxSlice(w.Stats, src) } // OpenAtReq is used to open existing FDs with the specified flags. @@ -578,11 +558,7 @@ func (f *FdArray) SizeBytes() int { func (f *FdArray) MarshalBytes(dst []byte) []byte { arrLen := primitive.Uint32(len(*f)) dst = arrLen.MarshalUnsafe(dst) - n, err := MarshalUnsafeFDIDSlice(*f, dst) - if err != nil { - panic(err) - } - return dst[n:] + return MarshalUnsafeFDIDSlice(*f, dst) } // UnmarshalBytes implements marshal.Marshallable.UnmarshalBytes. @@ -594,11 +570,7 @@ func (f *FdArray) UnmarshalBytes(src []byte) []byte { } else { *f = (*f)[:arrLen] } - n, err := UnmarshalUnsafeFDIDSlice(*f, src) - if err != nil { - panic(err) - } - return src[n:] + return UnmarshalUnsafeFDIDSlice(*f, src) } // CloseReq is used to close(2) FDs. diff --git a/pkg/lisafs/sample_message.go b/pkg/lisafs/sample_message.go index 745736b6d..3d4c090e4 100644 --- a/pkg/lisafs/sample_message.go +++ b/pkg/lisafs/sample_message.go @@ -55,22 +55,14 @@ func (m *MsgDynamic) SizeBytes() int { // MarshalBytes implements marshal.Marshallable.MarshalBytes. func (m *MsgDynamic) MarshalBytes(dst []byte) []byte { dst = m.N.MarshalUnsafe(dst) - n, err := MarshalUnsafeMsg1Slice(m.Arr, dst) - if err != nil { - panic(err) - } - return dst[n:] + return MarshalUnsafeMsg1Slice(m.Arr, dst) } // UnmarshalBytes implements marshal.Marshallable.UnmarshalBytes. func (m *MsgDynamic) UnmarshalBytes(src []byte) []byte { src = m.N.UnmarshalUnsafe(src) m.Arr = make([]MsgSimple, m.N) - n, err := UnmarshalUnsafeMsg1Slice(m.Arr, src) - if err != nil { - panic(err) - } - return src[n:] + return UnmarshalUnsafeMsg1Slice(m.Arr, src) } // Randomize randomizes the contents of m. diff --git a/pkg/marshal/marshal.go b/pkg/marshal/marshal.go index 9e34eae80..a48a5835d 100644 --- a/pkg/marshal/marshal.go +++ b/pkg/marshal/marshal.go @@ -150,13 +150,13 @@ type Marshallable interface { // // might be more efficient that repeatedly calling Foo.MarshalUnsafe // // over a []Foo in a loop if the type is Packed. // // Preconditions: dst must be at least len(src)*Foo.SizeBytes() in length. -// func MarshalUnsafeFooSlice(src []Foo, dst []byte) (int, error) { ... } +// func MarshalUnsafeFooSlice(src []Foo, dst []byte) []byte { ... } // // // UnmarshalUnsafeFooSlice is like Foo.UnmarshalUnsafe, buf for a []Foo. It // // might be more efficient that repeatedly calling Foo.UnmarshalUnsafe // // over a []Foo in a loop if the type is Packed. // // Preconditions: src must be at least len(dst)*Foo.SizeBytes() in length. -// func UnmarshalUnsafeFooSlice(dst []Foo, src []byte) (int, error) { ... } +// func UnmarshalUnsafeFooSlice(dst []Foo, src []byte) []byte { ... } // // // CopyFooSliceIn copies in a slice of Foo objects from the task's memory. // func CopyFooSliceIn(cc marshal.CopyContext, addr hostarch.Addr, dst []Foo) (int, error) { ... } diff --git a/tools/go_marshal/gomarshal/generator_interfaces_primitive_newtype.go b/tools/go_marshal/gomarshal/generator_interfaces_primitive_newtype.go index e2387f032..86c2dbdd5 100644 --- a/tools/go_marshal/gomarshal/generator_interfaces_primitive_newtype.go +++ b/tools/go_marshal/gomarshal/generator_interfaces_primitive_newtype.go @@ -264,36 +264,36 @@ func (g *interfaceGenerator) emitMarshallableSliceForPrimitiveNewtype(nt *ast.Id g.emit("}\n\n") g.emit("// MarshalUnsafe%s is like %s.MarshalUnsafe, but for a []%s.\n", slice.ident, g.typeName(), g.typeName()) - g.emit("func MarshalUnsafe%s(src []%s, dst []byte) (int, error) {\n", slice.ident, g.typeName()) + g.emit("func MarshalUnsafe%s(src []%s, dst []byte) []byte {\n", slice.ident, g.typeName()) g.inIndent(func() { g.emit("count := len(src)\n") g.emit("if count == 0 {\n") g.inIndent(func() { - g.emit("return 0, nil\n") + g.emit("return dst\n") }) g.emit("}\n") g.emit("size := (*%s)(nil).SizeBytes()\n\n", g.typeName()) - g.emit("dst = dst[:size*count]\n") - g.emit("gohacks.Memmove(unsafe.Pointer(&dst[0]), unsafe.Pointer(&src[0]), uintptr(len(dst)))\n") - g.emit("return size*count, nil\n") + g.emit("buf := dst[:size*count]\n") + g.emit("gohacks.Memmove(unsafe.Pointer(&buf[0]), unsafe.Pointer(&src[0]), uintptr(len(buf)))\n") + g.emit("return dst[size*count:]\n") }) g.emit("}\n\n") g.emit("// UnmarshalUnsafe%s is like %s.UnmarshalUnsafe, but for a []%s.\n", slice.ident, g.typeName(), g.typeName()) - g.emit("func UnmarshalUnsafe%s(dst []%s, src []byte) (int, error) {\n", slice.ident, g.typeName()) + g.emit("func UnmarshalUnsafe%s(dst []%s, src []byte) []byte {\n", slice.ident, g.typeName()) g.inIndent(func() { g.emit("count := len(dst)\n") g.emit("if count == 0 {\n") g.inIndent(func() { - g.emit("return 0, nil\n") + g.emit("return src\n") }) g.emit("}\n") g.emit("size := (*%s)(nil).SizeBytes()\n\n", g.typeName()) - g.emit("src = src[:(size*count)]\n") - g.emit("gohacks.Memmove(unsafe.Pointer(&dst[0]), unsafe.Pointer(&src[0]), uintptr(len(src)))\n") - g.emit("return size*count, nil\n") + g.emit("buf := src[:size*count]\n") + g.emit("gohacks.Memmove(unsafe.Pointer(&dst[0]), unsafe.Pointer(&buf[0]), uintptr(len(buf)))\n") + g.emit("return src[size*count:]\n") }) g.emit("}\n\n") } diff --git a/tools/go_marshal/gomarshal/generator_interfaces_struct.go b/tools/go_marshal/gomarshal/generator_interfaces_struct.go index 21177d39c..e7e4aef76 100644 --- a/tools/go_marshal/gomarshal/generator_interfaces_struct.go +++ b/tools/go_marshal/gomarshal/generator_interfaces_struct.go @@ -545,15 +545,14 @@ func (g *interfaceGenerator) emitMarshallableSliceForStruct(st *ast.StructType, g.emit("}\n\n") g.emit("// MarshalUnsafe%s is like %s.MarshalUnsafe, but for a []%s.\n", slice.ident, g.typeName(), g.typeName()) - g.emit("func MarshalUnsafe%s(src []%s, dst []byte) (int, error) {\n", slice.ident, g.typeName()) + g.emit("func MarshalUnsafe%s(src []%s, dst []byte) []byte {\n", slice.ident, g.typeName()) g.inIndent(func() { g.emit("count := len(src)\n") g.emit("if count == 0 {\n") g.inIndent(func() { - g.emit("return 0, nil\n") + g.emit("return dst\n") }) - g.emit("}\n") - g.emit("size := (*%s)(nil).SizeBytes()\n\n", g.typeName()) + g.emit("}\n\n") fallback := func() { g.emit("// Type %s doesn't have a packed layout in memory, fall back to MarshalBytes.\n", g.typeName()) @@ -562,7 +561,7 @@ func (g *interfaceGenerator) emitMarshallableSliceForStruct(st *ast.StructType, g.emit("dst = src[idx].MarshalBytes(dst)\n") }) g.emit("}\n") - g.emit("return size * count, nil\n") + g.emit("return dst\n") } if thisPacked { g.recordUsedImport("reflect") @@ -574,9 +573,10 @@ func (g *interfaceGenerator) emitMarshallableSliceForStruct(st *ast.StructType, g.inIndent(fallback) g.emit("}\n\n") } - g.emit("dst = dst[:size*count]\n") - g.emit("gohacks.Memmove(unsafe.Pointer(&dst[0]), unsafe.Pointer(&src[0]), uintptr(len(dst)))\n") - g.emit("return size * count, nil\n") + g.emit("size := (*%s)(nil).SizeBytes()\n", g.typeName()) + g.emit("buf := dst[:size*count]\n") + g.emit("gohacks.Memmove(unsafe.Pointer(&buf[0]), unsafe.Pointer(&src[0]), uintptr(len(buf)))\n") + g.emit("return dst[size*count:]\n") } else { fallback() } @@ -584,15 +584,14 @@ func (g *interfaceGenerator) emitMarshallableSliceForStruct(st *ast.StructType, g.emit("}\n\n") g.emit("// UnmarshalUnsafe%s is like %s.UnmarshalUnsafe, but for a []%s.\n", slice.ident, g.typeName(), g.typeName()) - g.emit("func UnmarshalUnsafe%s(dst []%s, src []byte) (int, error) {\n", slice.ident, g.typeName()) + g.emit("func UnmarshalUnsafe%s(dst []%s, src []byte) []byte {\n", slice.ident, g.typeName()) g.inIndent(func() { g.emit("count := len(dst)\n") g.emit("if count == 0 {\n") g.inIndent(func() { - g.emit("return 0, nil\n") + g.emit("return src\n") }) - g.emit("}\n") - g.emit("size := (*%s)(nil).SizeBytes()\n\n", g.typeName()) + g.emit("}\n\n") fallback := func() { g.emit("// Type %s doesn't have a packed layout in memory, fall back to UnmarshalBytes.\n", g.typeName()) @@ -601,7 +600,7 @@ func (g *interfaceGenerator) emitMarshallableSliceForStruct(st *ast.StructType, g.emit("src = dst[idx].UnmarshalBytes(src)\n") }) g.emit("}\n") - g.emit("return size * count, nil\n") + g.emit("return src\n") } if thisPacked { g.recordUsedImport("gohacks") @@ -613,10 +612,10 @@ func (g *interfaceGenerator) emitMarshallableSliceForStruct(st *ast.StructType, g.emit("}\n\n") } - g.emit("src = src[:(size*count)]\n") - g.emit("gohacks.Memmove(unsafe.Pointer(&dst[0]), unsafe.Pointer(&src[0]), uintptr(len(src)))\n") - - g.emit("return count*size, nil\n") + g.emit("size := (*%s)(nil).SizeBytes()\n", g.typeName()) + g.emit("buf := src[:size*count]\n") + g.emit("gohacks.Memmove(unsafe.Pointer(&dst[0]), unsafe.Pointer(&buf[0]), uintptr(len(buf)))\n") + g.emit("return src[size*count:]\n") } else { fallback() } -- cgit v1.2.3