Skip to content

Commit

Permalink
Fix unsafe use of pointers in sample code
Browse files Browse the repository at this point in the history
benchmark_marshaler_test.go and its copy in the README demonstrated
unsafe conversion of uintptr -> unsafe.Pointer. Fixing this will
hopefully reduce the likelihood that users will write unsafe code.
  • Loading branch information
zolstein-clumio committed Jul 5, 2022
1 parent e1eb8c5 commit 9d6f0d8
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 18 deletions.
17 changes: 8 additions & 9 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -106,15 +106,14 @@ type buffer struct {
b []byte
}

type encoder func(*buffer, uintptr) error
type encoder func(*buffer, unsafe.Pointer) error

func Marshal(v interface{}) ([]byte, error) {

// Technique 1.
// Get type information and pointer from interface{} value without allocation.
typ, ptr := reflect.TypeAndPtrOf(v)
typeID := reflect.TypeID(typ)
p := uintptr(ptr)

// Technique 2.
// Reuse the buffer once allocated using sync.Pool
Expand All @@ -125,7 +124,7 @@ func Marshal(v interface{}) ([]byte, error) {
// Technique 3.
// builds a optimized path by typeID and caches it
if enc, ok := typeToEncoderMap.Load(typeID); ok {
if err := enc.(encoder)(buf, p); err != nil {
if err := enc.(encoder)(buf, ptr); err != nil {
return nil, err
}

Expand All @@ -142,7 +141,7 @@ func Marshal(v interface{}) ([]byte, error) {
return nil, err
}
typeToEncoderMap.Store(typeID, enc)
if err := enc(buf, p); err != nil {
if err := enc(buf, ptr); err != nil {
return nil, err
}

Expand Down Expand Up @@ -173,11 +172,11 @@ func compileStruct(typ reflect.Type) (encoder, error) {
return nil, err
}
offset := field.Offset
encoders = append(encoders, func(buf *buffer, p uintptr) error {
return enc(buf, p+offset)
encoders = append(encoders, func(buf *buffer, p unsafe.Pointer) error {
return enc(buf, unsafe.Pointer(uintptr(p)+offset))
})
}
return func(buf *buffer, p uintptr) error {
return func(buf *buffer, p unsafe.Pointer) error {
buf.b = append(buf.b, '{')
for _, enc := range encoders {
if err := enc(buf, p); err != nil {
Expand All @@ -190,8 +189,8 @@ func compileStruct(typ reflect.Type) (encoder, error) {
}

func compileInt(typ reflect.Type) (encoder, error) {
return func(buf *buffer, p uintptr) error {
value := *(*int)(unsafe.Pointer(p))
return func(buf *buffer, p unsafe.Pointer) error {
value := *(*int)(p)
buf.b = strconv.AppendInt(buf.b, int64(value), 10)
return nil
}, nil
Expand Down
17 changes: 8 additions & 9 deletions benchmark_marshaler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,14 @@ type buffer struct {
b []byte
}

type encoder func(*buffer, uintptr) error
type encoder func(*buffer, unsafe.Pointer) error

func Marshal(v interface{}) ([]byte, error) {

// Technique 1.
// Get type information and pointer from interface{} value without allocation.
typ, ptr := reflect.TypeAndPtrOf(v)
typeID := reflect.TypeID(typ)
p := uintptr(ptr)

// Technique 2.
// Reuse the buffer once allocated using sync.Pool
Expand All @@ -44,7 +43,7 @@ func Marshal(v interface{}) ([]byte, error) {
// Technique 3.
// builds a optimized path by typeID and caches it
if enc, ok := typeToEncoderMap.Load(typeID); ok {
if err := enc.(encoder)(buf, p); err != nil {
if err := enc.(encoder)(buf, ptr); err != nil {
return nil, err
}

Expand All @@ -61,7 +60,7 @@ func Marshal(v interface{}) ([]byte, error) {
return nil, err
}
typeToEncoderMap.Store(typeID, enc)
if err := enc(buf, p); err != nil {
if err := enc(buf, ptr); err != nil {
return nil, err
}

Expand Down Expand Up @@ -92,11 +91,11 @@ func compileStruct(typ reflect.Type) (encoder, error) {
return nil, err
}
offset := field.Offset
encoders = append(encoders, func(buf *buffer, p uintptr) error {
return enc(buf, p+offset)
encoders = append(encoders, func(buf *buffer, p unsafe.Pointer) error {
return enc(buf, unsafe.Pointer(uintptr(p)+offset))
})
}
return func(buf *buffer, p uintptr) error {
return func(buf *buffer, p unsafe.Pointer) error {
buf.b = append(buf.b, '{')
for _, enc := range encoders {
if err := enc(buf, p); err != nil {
Expand All @@ -109,8 +108,8 @@ func compileStruct(typ reflect.Type) (encoder, error) {
}

func compileInt(typ reflect.Type) (encoder, error) {
return func(buf *buffer, p uintptr) error {
value := *(*int)(unsafe.Pointer(p))
return func(buf *buffer, p unsafe.Pointer) error {
value := *(*int)(p)
buf.b = strconv.AppendInt(buf.b, int64(value), 10)
return nil
}, nil
Expand Down

0 comments on commit 9d6f0d8

Please sign in to comment.