Merge pull request #2087 from mtrmac/shorten

Avoid manually-coded loops

check.go

@@ -8,6 +8,7 @@ import (
 	"os"
 	"path"
 	"path/filepath"
+	"slices"
 	"sort"
 	"strings"
 	"sync"
@@ -769,12 +770,9 @@ func (s *store) Repair(report CheckReport, options *RepairOptions) []error {
 		return d
 	}
 	isUnaccounted := func(errs []error) bool {
-		for _, err := range errs {
-			if errors.Is(err, ErrLayerUnaccounted) {
-				return true
-			}
-		}
-		return false
+		return slices.ContainsFunc(errs, func(err error) bool {
+			return errors.Is(err, ErrLayerUnaccounted)
+		})
 	}
 	sort.Slice(layersToDelete, func(i, j int) bool {
 		// we've not heard of either of them, so remove them in the order the driver suggested
@@ -1044,7 +1042,7 @@ func (c *checkDirectory) header(hdr *tar.Header) {
 
 // headers updates a checkDirectory using information from the passed-in header slice
 func (c *checkDirectory) headers(hdrs []*tar.Header) {
-	hdrs = append([]*tar.Header{}, hdrs...)
+	hdrs = slices.Clone(hdrs)
 	// sort the headers from the diff to ensure that whiteouts appear
 	// before content when they both appear in the same directory, per
 	// https://github.com/opencontainers/image-spec/blob/main/layer.md#whiteouts

check_test.go

@@ -2,7 +2,6 @@ package storage
 
 import (
 	"archive/tar"
-	"sort"
 	"testing"
 
 	"github.com/containers/storage/pkg/archive"
@@ -62,10 +61,7 @@ func TestCheckDirectory(t *testing.T) {
 				cd.header(&hdr)
 			}
 			actual := cd.names()
-			sort.Strings(actual)
-			expected := append([]string{}, vectors[i].expected...)
-			sort.Strings(expected)
-			assert.Equal(t, expected, actual)
+			assert.ElementsMatch(t, vectors[i].expected, actual)
 		})
 	}
 }

containers.go

@@ -3,8 +3,10 @@ package storage
 import (
 	"errors"
 	"fmt"
+	"maps"
 	"os"
 	"path/filepath"
+	"slices"
 	"sync"
 	"time"
 
@@ -162,17 +164,17 @@ type containerStore struct {
 func copyContainer(c *Container) *Container {
 	return &Container{
 		ID:             c.ID,
-		Names:          copyStringSlice(c.Names),
+		Names:          slices.Clone(c.Names),
 		ImageID:        c.ImageID,
 		LayerID:        c.LayerID,
 		Metadata:       c.Metadata,
-		BigDataNames:   copyStringSlice(c.BigDataNames),
-		BigDataSizes:   copyStringInt64Map(c.BigDataSizes),
-		BigDataDigests: copyStringDigestMap(c.BigDataDigests),
+		BigDataNames:   slices.Clone(c.BigDataNames),
+		BigDataSizes:   maps.Clone(c.BigDataSizes),
+		BigDataDigests: maps.Clone(c.BigDataDigests),
 		Created:        c.Created,
 		UIDMap:         copyIDMap(c.UIDMap),
 		GIDMap:         copyIDMap(c.GIDMap),
-		Flags:          copyStringInterfaceMap(c.Flags),
+		Flags:          maps.Clone(c.Flags),
 		volatileStore:  c.volatileStore,
 	}
 }
@@ -696,7 +698,7 @@ func (r *containerStore) create(id string, names []string, image, layer string,
 		volatileStore:  options.Volatile,
 	}
 	if options.MountOpts != nil {
-		container.Flags[mountOptsFlag] = append([]string{}, options.MountOpts...)
+		container.Flags[mountOptsFlag] = slices.Clone(options.MountOpts)
 	}
 	if options.Volatile {
 		container.Flags[volatileFlag] = true
@@ -788,13 +790,6 @@ func (r *containerStore) Delete(id string) error {
 		return ErrContainerUnknown
 	}
 	id = container.ID
-	toDeleteIndex := -1
-	for i, candidate := range r.containers {
-		if candidate.ID == id {
-			toDeleteIndex = i
-			break
-		}
-	}
 	delete(r.byid, id)
 	// This can only fail if the ID is already missing, which shouldn’t happen — and in that case the index is already in the desired state anyway.
 	// The store’s Delete method is used on various paths to recover from failures, so this should be robust against partially missing data.
@@ -803,14 +798,9 @@ func (r *containerStore) Delete(id string) error {
 	for _, name := range container.Names {
 		delete(r.byname, name)
 	}
-	if toDeleteIndex != -1 {
-		// delete the container at toDeleteIndex
-		if toDeleteIndex == len(r.containers)-1 {
-			r.containers = r.containers[:len(r.containers)-1]
-		} else {
-			r.containers = append(r.containers[:toDeleteIndex], r.containers[toDeleteIndex+1:]...)
-		}
-	}
+	r.containers = slices.DeleteFunc(r.containers, func(candidate *Container) bool {
+		return candidate.ID == id
+	})
 	if err := r.saveFor(container); err != nil {
 		return err
 	}
@@ -948,14 +938,7 @@ func (r *containerStore) SetBigData(id, key string, data []byte) error {
 		if !sizeOk || oldSize != c.BigDataSizes[key] || !digestOk || oldDigest != newDigest {
 			save = true
 		}
-		addName := true
-		for _, name := range c.BigDataNames {
-			if name == key {
-				addName = false
-				break
-			}
-		}
-		if addName {
+		if !slices.Contains(c.BigDataNames, key) {
 			c.BigDataNames = append(c.BigDataNames, key)
 			save = true
 		}

drivers/graphtest/graphtest_unix.go

@@ -205,17 +205,13 @@ func DriverTestCreateFromTemplate(t testing.TB, drivername string, driverOptions
 	if err != nil {
 		t.Fatal(err)
 	}
-	if err = checkChanges(noChanges, changes); err != nil {
-		t.Fatal(err)
-	}
+	require.ElementsMatch(t, noChanges, changes)
 
 	changes, err = driver.Changes("ROFromTemplate", nil, "Snap3", nil, "")
 	if err != nil {
 		t.Fatal(err)
 	}
-	if err = checkChanges(noChanges, changes); err != nil {
-		t.Fatal(err)
-	}
+	require.ElementsMatch(t, noChanges, changes)
 
 	if err := checkFile(driver, "FromTemplate", "testfile.txt", content); err != nil {
 		t.Fatal(err)
@@ -236,25 +232,19 @@ func DriverTestCreateFromTemplate(t testing.TB, drivername string, driverOptions
 	if err != nil {
 		t.Fatal(err)
 	}
-	if err = checkChanges(expectedChanges, changes); err != nil {
-		t.Fatal(err)
-	}
+	require.ElementsMatch(t, expectedChanges, changes)
 
 	changes, err = driver.Changes("FromTemplate", nil, "Base3", nil, "")
 	if err != nil {
 		t.Fatal(err)
 	}
-	if err = checkChanges(expectedChanges, changes); err != nil {
-		t.Fatal(err)
-	}
+	require.ElementsMatch(t, expectedChanges, changes)
 
 	changes, err = driver.Changes("ROFromTemplate", nil, "Base3", nil, "")
 	if err != nil {
 		t.Fatal(err)
 	}
-	if err = checkChanges(expectedChanges, changes); err != nil {
-		t.Fatal(err)
-	}
+	require.ElementsMatch(t, expectedChanges, changes)
 
 	verifyBase(t, driver, "Base3", defaultPerms)
 }
@@ -417,10 +407,7 @@ func DriverTestChanges(t testing.TB, drivername string, driverOptions ...string)
 	if err != nil {
 		t.Fatal(err)
 	}
-
-	if err = checkChanges(expectedChanges, changes); err != nil {
-		t.Fatal(err)
-	}
+	require.ElementsMatch(t, expectedChanges, changes)
 }
 
 func writeRandomFile(path string, size uint64) error {
@@ -513,10 +500,7 @@ func DriverTestEcho(t testing.TB, drivername string, driverOptions ...string) {
 		if err != nil {
 			t.Fatal(err)
 		}
-
-		if err = checkChanges(expectedChanges, changes); err != nil {
-			t.Fatal(err)
-		}
+		require.ElementsMatch(t, expectedChanges, changes)
 
 		if err := driver.Create(second, base, nil); err != nil {
 			t.Fatal(err)
@@ -540,10 +524,7 @@ func DriverTestEcho(t testing.TB, drivername string, driverOptions ...string) {
 		if err != nil {
 			t.Fatal(err)
 		}
-
-		if err = checkChanges(expectedChanges, changes); err != nil {
-			t.Fatal(err)
-		}
+		require.ElementsMatch(t, expectedChanges, changes)
 
 		if err = driver.Create(third, second, nil); err != nil {
 			t.Fatal(err)
@@ -573,10 +554,7 @@ func DriverTestEcho(t testing.TB, drivername string, driverOptions ...string) {
 		if err != nil {
 			t.Fatal(err)
 		}
-
-		if err = checkChanges(expectedChanges, changes); err != nil {
-			t.Fatal(err)
-		}
+		require.ElementsMatch(t, expectedChanges, changes)
 
 		err = driver.Put(third)
 		if err != nil {

drivers/graphtest/testutil.go

@@ -12,7 +12,6 @@ import (
 	"math/rand"
 	"os"
 	"path"
-	"sort"
 	"testing"
 
 	graphdriver "github.com/containers/storage/drivers"
@@ -270,33 +269,6 @@ func checkManyFiles(drv graphdriver.Driver, layer string, count int, seed int64)
 	return nil
 }
 
-type changeList []archive.Change
-
-func (c changeList) Less(i, j int) bool {
-	if c[i].Path == c[j].Path {
-		return c[i].Kind < c[j].Kind
-	}
-	return c[i].Path < c[j].Path
-}
-func (c changeList) Len() int      { return len(c) }
-func (c changeList) Swap(i, j int) { c[j], c[i] = c[i], c[j] }
-
-func checkChanges(expected, actual []archive.Change) error {
-	if len(expected) != len(actual) {
-		return fmt.Errorf("unexpected number of changes, expected %d, got %d", len(expected), len(actual))
-	}
-	sort.Sort(changeList(expected))
-	sort.Sort(changeList(actual))
-
-	for i := range expected {
-		if expected[i] != actual[i] {
-			return fmt.Errorf("unexpected change, expecting %v, got %v", expected[i], actual[i])
-		}
-	}
-
-	return nil
-}
-
 func addLayerFiles(drv graphdriver.Driver, layer, parent string, i int) error {
 	root, err := drv.Get(layer, graphdriver.MountOpts{})
 	if err != nil {

drivers/overlay/overlay.go

@@ -14,6 +14,7 @@ import (
 	"os/exec"
 	"path"
 	"path/filepath"
+	"slices"
 	"strconv"
 	"strings"
 	"sync"
@@ -158,30 +159,7 @@ func init() {
 }
 
 func hasMetacopyOption(opts []string) bool {
-	for _, s := range opts {
-		if s == "metacopy=on" {
-			return true
-		}
-	}
-	return false
-}
-
-func stripOption(opts []string, option string) []string {
-	for i, s := range opts {
-		if s == option {
-			return stripOption(append(opts[:i], opts[i+1:]...), option)
-		}
-	}
-	return opts
-}
-
-func hasVolatileOption(opts []string) bool {
-	for _, s := range opts {
-		if s == "volatile" {
-			return true
-		}
-	}
-	return false
+	return slices.Contains(opts, "metacopy=on")
 }
 
 func getMountProgramFlagFile(path string) string {
@@ -1526,14 +1504,13 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO
 				logrus.Debugf("Ignoring global metacopy option, the mount program doesn't support it")
 			}
 		}
-		optsList = stripOption(optsList, "metacopy=on")
+		optsList = slices.DeleteFunc(optsList, func(opt string) bool {
+			return opt == "metacopy=on"
+		})
 	}
 
-	for _, o := range optsList {
-		if o == "ro" {
-			readWrite = false
-			break
-		}
+	if slices.Contains(optsList, "ro") {
+		readWrite = false
 	}
 
 	lowers, err := os.ReadFile(path.Join(dir, lowerFile))
@@ -1732,7 +1709,7 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO
 		optsList = append(optsList, "userxattr")
 	}
 
-	if options.Volatile && !hasVolatileOption(optsList) {
+	if options.Volatile && !slices.Contains(optsList, "volatile") {
 		supported, err := d.getSupportsVolatile()
 		if err != nil {
 			return "", err