Skip to content

Commit

Permalink
Avoid unnecessary manually-coded loops
Browse files Browse the repository at this point in the history
Use the "slices", "maps" standard library packages, or other
readily-available features.

Should not change behavior.

Signed-off-by: Miloslav Trmač <[email protected]>
  • Loading branch information
mtrmac committed Sep 4, 2024
1 parent c76bce2 commit d9edc70
Show file tree
Hide file tree
Showing 15 changed files with 81 additions and 268 deletions.
9 changes: 3 additions & 6 deletions check.go
Original file line number Diff line number Diff line change
Expand Up @@ -770,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
Expand Down
20 changes: 7 additions & 13 deletions containers.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package storage
import (
"errors"
"fmt"
"maps"
"os"
"path/filepath"
"slices"
Expand Down Expand Up @@ -163,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,
}
}
Expand Down Expand Up @@ -937,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
}
Expand Down
40 changes: 9 additions & 31 deletions drivers/graphtest/graphtest_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down
28 changes: 0 additions & 28 deletions drivers/graphtest/testutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"math/rand"
"os"
"path"
"sort"
"testing"

graphdriver "github.com/containers/storage/drivers"
Expand Down Expand Up @@ -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 {
Expand Down
25 changes: 4 additions & 21 deletions drivers/overlay/overlay.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,21 +159,7 @@ func init() {
}

func hasMetacopyOption(opts []string) bool {
for _, s := range opts {
if s == "metacopy=on" {
return true
}
}
return false
}

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 {
Expand Down Expand Up @@ -1523,11 +1509,8 @@ func (d *Driver) get(id string, disableShifting bool, options graphdriver.MountO
})
}

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))
Expand Down Expand Up @@ -1726,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
Expand Down
49 changes: 17 additions & 32 deletions images.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package storage

import (
"fmt"
"maps"
"os"
"path/filepath"
"slices"
Expand Down Expand Up @@ -182,18 +183,18 @@ func copyImage(i *Image) *Image {
return &Image{
ID: i.ID,
Digest: i.Digest,
Digests: copyDigestSlice(i.Digests),
Names: copyStringSlice(i.Names),
NamesHistory: copyStringSlice(i.NamesHistory),
Digests: slices.Clone(i.Digests),
Names: slices.Clone(i.Names),
NamesHistory: slices.Clone(i.NamesHistory),
TopLayer: i.TopLayer,
MappedTopLayers: copyStringSlice(i.MappedTopLayers),
MappedTopLayers: slices.Clone(i.MappedTopLayers),
Metadata: i.Metadata,
BigDataNames: copyStringSlice(i.BigDataNames),
BigDataSizes: copyStringInt64Map(i.BigDataSizes),
BigDataDigests: copyStringDigestMap(i.BigDataDigests),
BigDataNames: slices.Clone(i.BigDataNames),
BigDataSizes: maps.Clone(i.BigDataSizes),
BigDataDigests: maps.Clone(i.BigDataDigests),
Created: i.Created,
ReadOnly: i.ReadOnly,
Flags: copyStringInterfaceMap(i.Flags),
Flags: maps.Clone(i.Flags),
}
}

Expand Down Expand Up @@ -872,7 +873,9 @@ func (r *imageStore) Delete(id string) error {
delete(r.byname, name)
}
for _, digest := range image.Digests {
prunedList := imageSliceWithoutValue(r.bydigest[digest], image)
prunedList := slices.DeleteFunc(r.bydigest[digest], func(i *Image) bool {
return i == image
})
if len(prunedList) == 0 {
delete(r.bydigest, digest)
} else {
Expand Down Expand Up @@ -967,17 +970,6 @@ func (r *imageStore) BigDataNames(id string) ([]string, error) {
return copyStringSlice(image.BigDataNames), nil
}

func imageSliceWithoutValue(slice []*Image, value *Image) []*Image {
modified := make([]*Image, 0, len(slice))
for _, v := range slice {
if v == value {
continue
}
modified = append(modified, v)
}
return modified
}

// Requires startWriting.
func (r *imageStore) SetBigData(id, key string, data []byte, digestManifest func([]byte) (digest.Digest, error)) error {
if !r.lockfile.IsReadWrite() {
Expand Down Expand Up @@ -1027,21 +1019,16 @@ func (r *imageStore) setBigData(image *Image, key string, data []byte, newDigest
if !sizeOk || oldSize != image.BigDataSizes[key] || !digestOk || oldDigest != newDigest {
save = true
}
addName := true
for _, name := range image.BigDataNames {
if name == key {
addName = false
break
}
}
if addName {
if !slices.Contains(image.BigDataNames, key) {
image.BigDataNames = append(image.BigDataNames, key)
save = true
}
for _, oldDigest := range image.Digests {
// remove the image from the list of images in the digest-based index
if list, ok := r.bydigest[oldDigest]; ok {
prunedList := imageSliceWithoutValue(list, image)
prunedList := slices.DeleteFunc(list, func(i *Image) bool {
return i == image
})
if len(prunedList) == 0 {
delete(r.bydigest, oldDigest)
} else {
Expand All @@ -1056,9 +1043,7 @@ func (r *imageStore) setBigData(image *Image, key string, data []byte, newDigest
// add the image to the list of images in the digest-based index which
// corresponds to the new digest for this item, unless it's already there
list := r.bydigest[newDigest]
if len(list) == len(imageSliceWithoutValue(list, image)) {
// the list isn't shortened by trying to prune this image from it,
// so it's not in there yet
if !slices.Contains(list, image) {
r.bydigest[newDigest] = append(list, image)
}
}
Expand Down
Loading

0 comments on commit d9edc70

Please sign in to comment.