Remove unsafe package usage (#1021)
Removes all unsafe operations from go-toml, making the codebase fully safe Go code. The internal/danger package that contained unsafe operations has been deleted. Changes: - Replace pointer-based node navigation with index-based navigation - Node.next and Node.child now store absolute indices into the backing nodes slice instead of relative offsets - Add nodes pointer to Node and Iterator for safe navigation - Replace danger.TypeID with reflect.Type for cache keys - Delete internal/danger package entirely Performance overhead is under 10% compared to the unsafe version, which is acceptable for the safety and maintainability benefits. [Cursor][claude-sonnet-4-20250514]
This commit is contained in:
Thomas Pelletier
@@ -1,64 +0,0 @@
|
||||
// Package danger provides optimized unsafe functions.
|
||||
package danger
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"unsafe"
|
||||
)
|
||||
|
||||
const maxInt = uintptr(int(^uint(0) >> 1))
|
||||
|
||||
func SubsliceOffset(data []byte, subslice []byte) int {
|
||||
datap := uintptr(unsafe.Pointer(unsafe.SliceData(data))) // #nosec G103
|
||||
hlp := uintptr(unsafe.Pointer(unsafe.SliceData(subslice))) // #nosec G103
|
||||
|
||||
if hlp < datap {
|
||||
panic(fmt.Errorf("subslice address (%d) is before data address (%d)", hlp, datap))
|
||||
}
|
||||
offset := hlp - datap
|
||||
|
||||
if offset > maxInt {
|
||||
panic(fmt.Errorf("slice offset larger than int (%d)", offset))
|
||||
}
|
||||
|
||||
intoffset := int(offset)
|
||||
|
||||
if intoffset > len(data) {
|
||||
panic(fmt.Errorf("slice offset (%d) is farther than data length (%d)", intoffset, len(data)))
|
||||
}
|
||||
|
||||
if intoffset+len(subslice) > len(data) {
|
||||
panic(fmt.Errorf("slice ends (%d+%d) is farther than data length (%d)", intoffset, len(subslice), len(data)))
|
||||
}
|
||||
|
||||
return intoffset
|
||||
}
|
||||
|
||||
func BytesRange(start []byte, end []byte) []byte {
|
||||
if start == nil || end == nil {
|
||||
panic("cannot call BytesRange with nil")
|
||||
}
|
||||
|
||||
startp := uintptr(unsafe.Pointer(unsafe.SliceData(start))) // #nosec G103
|
||||
endp := uintptr(unsafe.Pointer(unsafe.SliceData(end))) // #nosec G103
|
||||
|
||||
if startp > endp {
|
||||
panic(fmt.Errorf("start pointer address (%d) is after end pointer address (%d)", startp, endp))
|
||||
}
|
||||
|
||||
l := len(start)
|
||||
endLen := int(endp-startp) + len(end)
|
||||
if endLen > l {
|
||||
l = endLen
|
||||
}
|
||||
|
||||
if l > cap(start) {
|
||||
panic("range length is larger than capacity")
|
||||
}
|
||||
|
||||
return start[:l]
|
||||
}
|
||||
|
||||
func Stride(ptr unsafe.Pointer, size uintptr, offset int) unsafe.Pointer {
|
||||
return unsafe.Add(ptr, size*uintptr(offset))
|
||||
}
|
||||
@@ -1,176 +0,0 @@
|
||||
package danger_test
|
||||
|
||||
import (
|
||||
"testing"
|
||||
"unsafe"
|
||||
|
||||
"github.com/pelletier/go-toml/v2/internal/assert"
|
||||
"github.com/pelletier/go-toml/v2/internal/danger"
|
||||
)
|
||||
|
||||
func TestSubsliceOffsetValid(t *testing.T) {
|
||||
examples := []struct {
|
||||
desc string
|
||||
test func() ([]byte, []byte)
|
||||
offset int
|
||||
}{
|
||||
{
|
||||
desc: "simple",
|
||||
test: func() ([]byte, []byte) {
|
||||
data := []byte("hello")
|
||||
return data, data[1:]
|
||||
},
|
||||
offset: 1,
|
||||
},
|
||||
}
|
||||
|
||||
for _, e := range examples {
|
||||
t.Run(e.desc, func(t *testing.T) {
|
||||
d, s := e.test()
|
||||
offset := danger.SubsliceOffset(d, s)
|
||||
assert.Equal(t, e.offset, offset)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestSubsliceOffsetInvalid(t *testing.T) {
|
||||
examples := []struct {
|
||||
desc string
|
||||
test func() ([]byte, []byte)
|
||||
}{
|
||||
{
|
||||
desc: "unrelated arrays",
|
||||
test: func() ([]byte, []byte) {
|
||||
return []byte("one"), []byte("two")
|
||||
},
|
||||
},
|
||||
{
|
||||
desc: "slice starts before data",
|
||||
test: func() ([]byte, []byte) {
|
||||
full := []byte("hello world")
|
||||
return full[5:], full[1:]
|
||||
},
|
||||
},
|
||||
{
|
||||
desc: "slice starts after data",
|
||||
test: func() ([]byte, []byte) {
|
||||
full := []byte("hello world")
|
||||
return full[:3], full[5:]
|
||||
},
|
||||
},
|
||||
{
|
||||
desc: "slice ends after data",
|
||||
test: func() ([]byte, []byte) {
|
||||
full := []byte("hello world")
|
||||
return full[:5], full[3:8]
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
for _, e := range examples {
|
||||
t.Run(e.desc, func(t *testing.T) {
|
||||
d, s := e.test()
|
||||
assert.Panics(t, func() {
|
||||
danger.SubsliceOffset(d, s)
|
||||
})
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestStride(t *testing.T) {
|
||||
a := []byte{1, 2, 3, 4}
|
||||
x := &a[1]
|
||||
n := (*byte)(danger.Stride(unsafe.Pointer(x), unsafe.Sizeof(byte(0)), 1))
|
||||
assert.Equal(t, &a[2], n)
|
||||
n = (*byte)(danger.Stride(unsafe.Pointer(x), unsafe.Sizeof(byte(0)), -1))
|
||||
assert.Equal(t, &a[0], n)
|
||||
}
|
||||
|
||||
func TestBytesRange(t *testing.T) {
|
||||
type fn = func() ([]byte, []byte)
|
||||
examples := []struct {
|
||||
desc string
|
||||
test fn
|
||||
expected []byte
|
||||
}{
|
||||
{
|
||||
desc: "simple",
|
||||
test: func() ([]byte, []byte) {
|
||||
full := []byte("hello world")
|
||||
return full[1:3], full[6:8]
|
||||
},
|
||||
expected: []byte("ello wo"),
|
||||
},
|
||||
{
|
||||
desc: "full",
|
||||
test: func() ([]byte, []byte) {
|
||||
full := []byte("hello world")
|
||||
return full[0:1], full[len(full)-1:]
|
||||
},
|
||||
expected: []byte("hello world"),
|
||||
},
|
||||
{
|
||||
desc: "end before start",
|
||||
test: func() ([]byte, []byte) {
|
||||
full := []byte("hello world")
|
||||
return full[len(full)-1:], full[0:1]
|
||||
},
|
||||
},
|
||||
{
|
||||
desc: "nils",
|
||||
test: func() ([]byte, []byte) {
|
||||
return nil, nil
|
||||
},
|
||||
},
|
||||
{
|
||||
desc: "nils start",
|
||||
test: func() ([]byte, []byte) {
|
||||
return nil, []byte("foo")
|
||||
},
|
||||
},
|
||||
{
|
||||
desc: "nils end",
|
||||
test: func() ([]byte, []byte) {
|
||||
return []byte("foo"), nil
|
||||
},
|
||||
},
|
||||
{
|
||||
desc: "start is end",
|
||||
test: func() ([]byte, []byte) {
|
||||
full := []byte("hello world")
|
||||
return full[1:3], full[1:3]
|
||||
},
|
||||
expected: []byte("el"),
|
||||
},
|
||||
{
|
||||
desc: "end contained in start",
|
||||
test: func() ([]byte, []byte) {
|
||||
full := []byte("hello world")
|
||||
return full[1:7], full[2:4]
|
||||
},
|
||||
expected: []byte("ello w"),
|
||||
},
|
||||
{
|
||||
desc: "different backing arrays",
|
||||
test: func() ([]byte, []byte) {
|
||||
one := []byte("hello world")
|
||||
two := []byte("hello world")
|
||||
return one, two
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
for _, e := range examples {
|
||||
t.Run(e.desc, func(t *testing.T) {
|
||||
start, end := e.test()
|
||||
if e.expected == nil {
|
||||
assert.Panics(t, func() {
|
||||
danger.BytesRange(start, end)
|
||||
})
|
||||
} else {
|
||||
res := danger.BytesRange(start, end)
|
||||
assert.Equal(t, e.expected, res)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -1,23 +0,0 @@
|
||||
package danger
|
||||
|
||||
import (
|
||||
"reflect"
|
||||
"unsafe"
|
||||
)
|
||||
|
||||
// TypeID is used as key in encoder and decoder caches to enable using
|
||||
// the optimize runtime.mapaccess2_fast64 function instead of the more
|
||||
// expensive lookup if we were to use reflect.Type as map key.
|
||||
//
|
||||
// typeID holds the pointer to the reflect.Type value, which is unique
|
||||
// in the program.
|
||||
//
|
||||
// https://github.com/segmentio/encoding/blob/master/json/codec.go#L59-L61
|
||||
type TypeID unsafe.Pointer
|
||||
|
||||
func MakeTypeID(t reflect.Type) TypeID {
|
||||
// reflect.Type has the fields:
|
||||
// typ unsafe.Pointer
|
||||
// ptr unsafe.Pointer
|
||||
return TypeID((*[2]unsafe.Pointer)(unsafe.Pointer(&t))[1]) // #nosec G103
|
||||
}
|
||||
@@ -1,8 +1,8 @@
|
||||
package tracker
|
||||
|
||||
import (
|
||||
"reflect"
|
||||
"testing"
|
||||
"unsafe"
|
||||
|
||||
"github.com/pelletier/go-toml/v2/internal/assert"
|
||||
)
|
||||
@@ -12,9 +12,10 @@ func TestEntrySize(t *testing.T) {
|
||||
// performance of unmarshaling documents. Should only be increased with care
|
||||
// and a very good reason.
|
||||
maxExpectedEntrySize := 48
|
||||
entrySize := int(reflect.TypeOf(entry{}).Size())
|
||||
assert.True(t,
|
||||
int(unsafe.Sizeof(entry{})) <= maxExpectedEntrySize,
|
||||
entrySize <= maxExpectedEntrySize,
|
||||
"Expected entry to be less than or equal to %d, got: %d",
|
||||
maxExpectedEntrySize, int(unsafe.Sizeof(entry{})),
|
||||
maxExpectedEntrySize, entrySize,
|
||||
)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user