Skip to content

Commit 2a8b604

Browse files
fix(y): shall always return empty slice rather than nil (#2245)
Fixes #2067 **Description** This PR fixes an issue where Go's `append` returns `nil` when appending an empty slice to a nil slice. I added a check in `SafeCopy` to ensure we always return `[]byte{}` in this case, along with a new unit test. I also verified it using the test offered in the issue **Checklist** - [x] Code compiles correctly and linting passes locally - [ ] For all _code_ changes, an entry added to the `CHANGELOG.md` file describing and linking to this PR - [x] Tests added for new functionality, or regression tests for bug fixes added as applicable --------- Co-authored-by: Matthew McNeely <matthew.mcneely@gmail.com>
1 parent 81b3cb9 commit 2a8b604

File tree

4 files changed

+56
-8
lines changed

4 files changed

+56
-8
lines changed

stream_writer_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -754,16 +754,16 @@ func TestStreamWriterIncremental(t *testing.T) {
754754
require.NoError(t, sw.Write(buf), "sw.Write() failed")
755755
require.NoError(t, sw.Flush(), "sw.Flush() failed")
756756

757-
buf = z.NewBuffer(10<<20, "test")
758-
defer func() { require.NoError(t, buf.Release()) }()
757+
buf2 := z.NewBuffer(10<<20, "test")
758+
defer func() { require.NoError(t, buf2.Release()) }()
759759
KVToBuffer(&pb.KV{
760760
Key: []byte("a2"),
761761
Value: []byte("val2"),
762762
Version: 9,
763-
}, buf)
763+
}, buf2)
764764
sw = db.NewStreamWriter()
765765
require.NoError(t, sw.PrepareIncremental(), "sw.PrepareIncremental() failed")
766-
require.NoError(t, sw.Write(buf), "sw.Write() failed")
766+
require.NoError(t, sw.Write(buf2), "sw.Write() failed")
767767
require.NoError(t, sw.Flush(), "sw.Flush() failed")
768768

769769
// This will move the maxTs to 10 (earlier, without the fix)

test.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,6 @@ write_coverage() {
106106
# parallel tests currently not working
107107
# parallel --halt now,fail=1 --progress --line-buffer ::: stream manual root
108108
# run tests in sequence
109-
#root
110-
#stream
109+
root
110+
stream
111111
manual

y/y.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,11 @@ func OpenTruncFile(filename string, sync bool) (*os.File, error) {
9595

9696
// SafeCopy does append(a[:0], src...).
9797
func SafeCopy(a, src []byte) []byte {
98-
return append(a[:0], src...)
98+
b := append(a[:0], src...)
99+
if b == nil {
100+
return []byte{}
101+
}
102+
return b
99103
}
100104

101105
// Copy copies a byte slice and returns the copied slice.
@@ -134,7 +138,7 @@ func CompareKeys(key1, key2 []byte) int {
134138

135139
// ParseKey parses the actual key from the key bytes.
136140
func ParseKey(key []byte) []byte {
137-
if key == nil {
141+
if len(key) < 8 {
138142
return nil
139143
}
140144

y/y_test.go

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,3 +328,47 @@ func TestAllocatorReuse(t *testing.T) {
328328
}
329329
t.Logf("Allocator: %s\n", a)
330330
}
331+
332+
func TestSafeCopy_Issue2067(t *testing.T) {
333+
type args struct {
334+
a []byte
335+
src []byte
336+
}
337+
tests := []struct {
338+
name string
339+
args args
340+
want []byte
341+
}{
342+
{
343+
name: "Nil src should return empty slice not nil",
344+
args: args{a: nil, src: nil},
345+
want: []byte{},
346+
},
347+
{
348+
name: "Empty src should return empty slice not nil",
349+
args: args{a: nil, src: []byte{}},
350+
want: []byte{},
351+
},
352+
{
353+
name: "Normal src should return src content",
354+
args: args{a: nil, src: []byte("hello")},
355+
want: []byte("hello"),
356+
},
357+
{
358+
name: "Buffer reuse with nil src should return empty slice",
359+
args: args{a: make([]byte, 10), src: nil},
360+
want: []byte{},
361+
},
362+
}
363+
for _, tt := range tests {
364+
t.Run(tt.name, func(t *testing.T) {
365+
got := SafeCopy(tt.args.a, tt.args.src)
366+
require.Equal(t, tt.want, got)
367+
368+
// Explicit check for nil vs empty slice distinction
369+
if len(tt.want) == 0 {
370+
require.NotNil(t, got, "SafeCopy returned nil, but we expected an empty slice []byte{}")
371+
}
372+
})
373+
}
374+
}

0 commit comments

Comments
 (0)