Skip to content

Handle nil pointers. #125

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 45 additions & 15 deletions pkg/docker/dockerimage/topobjects.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@ import (
"container/heap"
)

// TopObjects is a slice of ObjectMetadata pointers that implements the heap.Interface
// for maintaining a priority queue of objects, typically sorted by size.
type TopObjects []*ObjectMetadata

// NewTopObjects creates and returns a new, empty TopObjects slice with a specified capacity.
func NewTopObjects(n int) TopObjects {
if n < 1 {
n = 1
Expand All @@ -14,34 +17,51 @@ func NewTopObjects(n int) TopObjects {
return make(TopObjects, 0, n)
}

// Len returns the number of elements in the TopObjects slice.
func (to TopObjects) Len() int { return len(to) }

// Less compares two elements in the slice for sorting.
// Nil elements are considered smaller than non-nil elements.
// Two nil elements are considered equal.
func (to TopObjects) Less(i, j int) bool {
if to[i] == nil && to[j] != nil {
return true
// Handle cases where either element is nil
if to[i] == nil && to[j] == nil {
return false // Equal, so not less than
}
if to[i] != nil && to[j] == nil {
return false
if to[i] == nil {
return true // nil is considered smaller than non-nil
}
if to[i] == nil && to[j] == nil {
return false
if to[j] == nil {
return false // Non-nil is considered larger than nil
}

// Both elements are non-nil, compare their sizes
return to[i].Size < to[j].Size
}

// Swap swaps the elements with indexes i and j.
// It performs bounds checking to prevent panics.
func (to TopObjects) Swap(i, j int) {
if i < 0 || i >= len(to) || j < 0 || j >= len(to) {
return // Out of bounds, no-op
}
to[i], to[j] = to[j], to[i]
}

// Push adds an element to the heap. It handles nil values safely and only adds
// valid *ObjectMetadata pointers to the slice.
func (to *TopObjects) Push(x interface{}) {
item := x.(*ObjectMetadata)
if item == nil {
if x == nil {
return
}
item, ok := x.(*ObjectMetadata)
if !ok || item == nil {
return
}
*to = append(*to, item)
}

// Pop removes and returns the smallest element from the heap.
// The result is the element that would be returned by Pop() from the heap package.
func (to *TopObjects) Pop() interface{} {
old := *to
n := len(old)
Expand All @@ -51,14 +71,24 @@ func (to *TopObjects) Pop() interface{} {
return item
}

// List returns a sorted slice of ObjectMetadata, with the largest elements first.
// It creates a copy of the underlying data to preserve the original heap.
// Returns nil if the receiver is nil.
func (to TopObjects) List() []*ObjectMetadata {
list := []*ObjectMetadata{}
for len(to) > 0 {
item := heap.Pop(&to).(*ObjectMetadata)
if item == nil {
continue
if to == nil {
return nil
}

tmp := make(TopObjects, len(to))
copy(tmp, to)
heap.Init(&tmp)

list := make([]*ObjectMetadata, 0, len(to))
for tmp.Len() > 0 {
item := heap.Pop(&tmp).(*ObjectMetadata)
if item != nil {
list = append([]*ObjectMetadata{item}, list...) // prepend to maintain order
}
list = append([]*ObjectMetadata{item}, list...)
}

return list
Expand Down
94 changes: 94 additions & 0 deletions pkg/docker/dockerimage/topobjects_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
package dockerimage

import (
"container/heap"
"testing"
)

func TestTopObjects_List_NilSafety(t *testing.T) {
tests := []struct {
name string
setup func(t *testing.T) TopObjects
wantLen int
}{
{
name: "nil TopObjects",
setup: func(t *testing.T) TopObjects {
return nil
},
wantLen: 0,
},
{
name: "empty TopObjects",
setup: func(t *testing.T) TopObjects {
return NewTopObjects(0)
},
wantLen: 0,
},
{
name: "TopObjects with nil elements",
setup: func(t *testing.T) TopObjects {
to := NewTopObjects(3)
heap.Push(&to, &ObjectMetadata{Name: "file1", Size: 100})
// Test both interface{} nil and typed nil
heap.Push(&to, nil)
heap.Push(&to, (*ObjectMetadata)(nil))
heap.Push(&to, &ObjectMetadata{Name: "file2", Size: 200})
return to
},
wantLen: 2, // Should only contain non-nil elements
},
{
name: "TopObjects with multiple elements",
setup: func(t *testing.T) TopObjects {
to := NewTopObjects(3)
heap.Push(&to, &ObjectMetadata{Name: "file1", Size: 100})
heap.Push(&to, &ObjectMetadata{Name: "file2", Size: 200})
heap.Push(&to, &ObjectMetadata{Name: "file3", Size: 50})
return to
},
wantLen: 3,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
to := tt.setup(t)
result := to.List()

if len(result) != tt.wantLen {
t.Errorf("List() returned %d items, want %d", len(result), tt.wantLen)
}

// Verify the order is correct (descending by size)
for i := 1; i < len(result); i++ {
if result[i-1] == nil || result[i] == nil {
t.Fatal("List() contains nil elements")
}
if result[i-1].Size < result[i].Size {
t.Errorf("List() is not sorted in descending order: %d < %d at index %d", result[i-1].Size, result[i].Size, i)
}
}
})
}
}

func TestTopObjects_List_ModificationSafety(t *testing.T) {
// Test that the original TopObjects is not modified by List()
to := NewTopObjects(3)
heap.Push(&to, &ObjectMetadata{Name: "file1", Size: 100})
heap.Push(&to, &ObjectMetadata{Name: "file2", Size: 200})

originalLen := to.Len()
result := to.List()

// Modify the result slice
if len(result) > 0 {
result[0] = &ObjectMetadata{Name: "modified", Size: 999}
}

// The original TopObjects should not be affected
if to.Len() != originalLen {
t.Errorf("Original TopObjects length changed from %d to %d", originalLen, to.Len())
}
}
Loading