Handle nil pointers. (#125)

* Handle nil pointers.

* Fix golint warnings.

Author: ernstvonoelsen

pkg/docker/dockerimage/topobjects.go

@@ -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
@@ -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)
@@ -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

pkg/docker/dockerimage/topobjects_test.go

@@ -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())
+	}
+}