avoid `defer x.Unlock()` pattern in loops

Deferring method calls on loop variables must be avoided by all means as
the calls will be invoked on the last item of the loop.

The intermediate fix used in this commit is to allocate a new variable
on the heap for each loop iteration.  An example transformation is:

FROM:
for _, x := range x_slice {
	x.Lock()
	defer x.Unlock()
}

TO:
for _, x_itr := range x_slice {
	x := x_itr
	x.Lock()
	defer x.Unlock()
}

Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
This commit is contained in:
Valentin Rothberg 2019-02-08 13:39:25 +01:00
parent 0d890fd313
commit d89252da40
1 changed files with 58 additions and 29 deletions

View File

@ -870,7 +870,8 @@ func (s *store) PutLayer(id, parent string, names []string, mountLabel string, w
gidMap := options.GIDMap
if parent != "" {
var ilayer *Layer
for _, lstore := range append([]ROLayerStore{rlstore}, rlstores...) {
for _, l := range append([]ROLayerStore{rlstore}, rlstores...) {
lstore := l
if lstore != rlstore {
lstore.Lock()
defer lstore.Unlock()
@ -949,7 +950,8 @@ func (s *store) CreateImage(id string, names []string, layer, metadata string, o
return nil, err
}
var ilayer *Layer
for _, store := range append([]ROLayerStore{lstore}, lstores...) {
for _, s := range append([]ROLayerStore{lstore}, lstores...) {
store := s
store.Lock()
defer store.Unlock()
if modified, err := store.Modified(); modified || err != nil {
@ -1010,7 +1012,8 @@ func (s *store) imageTopLayerForMapping(image *Image, ristore ROImageStore, crea
}
var layer, parentLayer *Layer
// Locate the image's top layer and its parent, if it has one.
for _, store := range append([]ROLayerStore{rlstore}, lstores...) {
for _, s := range append([]ROLayerStore{rlstore}, lstores...) {
store := s
if store != rlstore {
store.Lock()
defer store.Unlock()
@ -1143,7 +1146,8 @@ func (s *store) CreateContainer(id string, names []string, image, layer, metadat
}
}
var cimage *Image
for _, store := range append([]ROImageStore{istore}, istores...) {
for _, s := range append([]ROImageStore{istore}, istores...) {
store := s
store.Lock()
defer store.Unlock()
if modified, err := store.Modified(); modified || err != nil {
@ -1315,7 +1319,8 @@ func (s *store) Metadata(id string) (string, error) {
if err != nil {
return "", err
}
for _, store := range append([]ROLayerStore{lstore}, lstores...) {
for _, s := range append([]ROLayerStore{lstore}, lstores...) {
store := s
store.Lock()
defer store.Unlock()
if modified, err := store.Modified(); modified || err != nil {
@ -1336,7 +1341,8 @@ func (s *store) Metadata(id string) (string, error) {
if err != nil {
return "", err
}
for _, store := range append([]ROImageStore{istore}, istores...) {
for _, s := range append([]ROImageStore{istore}, istores...) {
store := s
store.Lock()
defer store.Unlock()
if modified, err := store.Modified(); modified || err != nil {
@ -1375,7 +1381,8 @@ func (s *store) ListImageBigData(id string) ([]string, error) {
if err != nil {
return nil, err
}
for _, store := range append([]ROImageStore{istore}, istores...) {
for _, s := range append([]ROImageStore{istore}, istores...) {
store := s
store.Lock()
defer store.Unlock()
if modified, err := store.Modified(); modified || err != nil {
@ -1400,7 +1407,8 @@ func (s *store) ImageBigDataSize(id, key string) (int64, error) {
if err != nil {
return -1, err
}
for _, store := range append([]ROImageStore{istore}, istores...) {
for _, s := range append([]ROImageStore{istore}, istores...) {
store := s
store.Lock()
defer store.Unlock()
if modified, err := store.Modified(); modified || err != nil {
@ -1426,7 +1434,8 @@ func (s *store) ImageBigDataDigest(id, key string) (digest.Digest, error) {
return "", err
}
stores = append([]ROImageStore{ristore}, stores...)
for _, ristore := range stores {
for _, r := range stores {
ristore := r
ristore.Lock()
defer ristore.Unlock()
if modified, err := ristore.Modified(); modified || err != nil {
@ -1451,7 +1460,8 @@ func (s *store) ImageBigData(id, key string) ([]byte, error) {
if err != nil {
return nil, err
}
for _, store := range append([]ROImageStore{istore}, istores...) {
for _, s := range append([]ROImageStore{istore}, istores...) {
store := s
store.Lock()
defer store.Unlock()
if modified, err := store.Modified(); modified || err != nil {
@ -1495,7 +1505,8 @@ func (s *store) ImageSize(id string) (int64, error) {
if err != nil {
return -1, errors.Wrapf(err, "error loading additional layer stores")
}
for _, store := range append([]ROLayerStore{lstore}, lstores...) {
for _, s := range append([]ROLayerStore{lstore}, lstores...) {
store := s
store.Lock()
defer store.Unlock()
if modified, err := store.Modified(); modified || err != nil {
@ -1516,7 +1527,8 @@ func (s *store) ImageSize(id string) (int64, error) {
}
// Look for the image's record.
for _, store := range append([]ROImageStore{istore}, istores...) {
for _, s := range append([]ROImageStore{istore}, istores...) {
store := s
store.Lock()
defer store.Unlock()
if modified, err := store.Modified(); modified || err != nil {
@ -1603,7 +1615,8 @@ func (s *store) ContainerSize(id string) (int64, error) {
if err != nil {
return -1, err
}
for _, store := range append([]ROLayerStore{lstore}, lstores...) {
for _, s := range append([]ROLayerStore{lstore}, lstores...) {
store := s
store.Lock()
defer store.Unlock()
if modified, err := store.Modified(); modified || err != nil {
@ -1772,7 +1785,8 @@ func (s *store) Exists(id string) bool {
if err != nil {
return false
}
for _, store := range append([]ROLayerStore{lstore}, lstores...) {
for _, s := range append([]ROLayerStore{lstore}, lstores...) {
store := s
store.Lock()
defer store.Unlock()
if modified, err := store.Modified(); modified || err != nil {
@ -1793,7 +1807,8 @@ func (s *store) Exists(id string) bool {
if err != nil {
return false
}
for _, store := range append([]ROImageStore{istore}, istores...) {
for _, s := range append([]ROImageStore{istore}, istores...) {
store := s
store.Lock()
defer store.Unlock()
if modified, err := store.Modified(); modified || err != nil {
@ -1895,7 +1910,8 @@ func (s *store) Names(id string) ([]string, error) {
if err != nil {
return nil, err
}
for _, store := range append([]ROLayerStore{lstore}, lstores...) {
for _, s := range append([]ROLayerStore{lstore}, lstores...) {
store := s
store.Lock()
defer store.Unlock()
if modified, err := store.Modified(); modified || err != nil {
@ -1916,7 +1932,8 @@ func (s *store) Names(id string) ([]string, error) {
if err != nil {
return nil, err
}
for _, store := range append([]ROImageStore{istore}, istores...) {
for _, s := range append([]ROImageStore{istore}, istores...) {
store := s
store.Lock()
defer store.Unlock()
if modified, err := store.Modified(); modified || err != nil {
@ -1955,7 +1972,8 @@ func (s *store) Lookup(name string) (string, error) {
if err != nil {
return "", err
}
for _, store := range append([]ROLayerStore{lstore}, lstores...) {
for _, s := range append([]ROLayerStore{lstore}, lstores...) {
store := s
store.Lock()
defer store.Unlock()
if modified, err := store.Modified(); modified || err != nil {
@ -1976,7 +1994,8 @@ func (s *store) Lookup(name string) (string, error) {
if err != nil {
return "", err
}
for _, store := range append([]ROImageStore{istore}, istores...) {
for _, s := range append([]ROImageStore{istore}, istores...) {
store := s
store.Lock()
defer store.Unlock()
if modified, err := store.Modified(); modified || err != nil {
@ -2486,7 +2505,8 @@ func (s *store) Changes(from, to string) ([]archive.Change, error) {
if err != nil {
return nil, err
}
for _, store := range append([]ROLayerStore{lstore}, lstores...) {
for _, s := range append([]ROLayerStore{lstore}, lstores...) {
store := s
store.Lock()
defer store.Unlock()
if modified, err := store.Modified(); modified || err != nil {
@ -2510,7 +2530,8 @@ func (s *store) DiffSize(from, to string) (int64, error) {
if err != nil {
return -1, err
}
for _, store := range append([]ROLayerStore{lstore}, lstores...) {
for _, s := range append([]ROLayerStore{lstore}, lstores...) {
store := s
store.Lock()
defer store.Unlock()
if modified, err := store.Modified(); modified || err != nil {
@ -2534,7 +2555,8 @@ func (s *store) Diff(from, to string, options *DiffOptions) (io.ReadCloser, erro
if err != nil {
return nil, err
}
for _, store := range append([]ROLayerStore{lstore}, lstores...) {
for _, s := range append([]ROLayerStore{lstore}, lstores...) {
store := s
store.Lock()
if modified, err := store.Modified(); modified || err != nil {
if err = store.Load(); err != nil {
@ -2588,7 +2610,8 @@ func (s *store) layersByMappedDigest(m func(ROLayerStore, digest.Digest) ([]Laye
if err != nil {
return nil, err
}
for _, store := range append([]ROLayerStore{lstore}, lstores...) {
for _, s := range append([]ROLayerStore{lstore}, lstores...) {
store := s
store.Lock()
defer store.Unlock()
if modified, err := store.Modified(); modified || err != nil {
@ -2634,7 +2657,8 @@ func (s *store) LayerSize(id string) (int64, error) {
if err != nil {
return -1, err
}
for _, store := range append([]ROLayerStore{lstore}, lstores...) {
for _, s := range append([]ROLayerStore{lstore}, lstores...) {
store := s
store.Lock()
defer store.Unlock()
if modified, err := store.Modified(); modified || err != nil {
@ -2712,7 +2736,8 @@ func (s *store) Layers() ([]Layer, error) {
return nil, err
}
for _, store := range append([]ROLayerStore{lstore}, lstores...) {
for _, s := range append([]ROLayerStore{lstore}, lstores...) {
store := s
store.Lock()
defer store.Unlock()
if modified, err := store.Modified(); modified || err != nil {
@ -2740,7 +2765,8 @@ func (s *store) Images() ([]Image, error) {
if err != nil {
return nil, err
}
for _, store := range append([]ROImageStore{istore}, istores...) {
for _, s := range append([]ROImageStore{istore}, istores...) {
store := s
store.Lock()
defer store.Unlock()
if modified, err := store.Modified(); modified || err != nil {
@ -2783,7 +2809,8 @@ func (s *store) Layer(id string) (*Layer, error) {
if err != nil {
return nil, err
}
for _, store := range append([]ROLayerStore{lstore}, lstores...) {
for _, s := range append([]ROLayerStore{lstore}, lstores...) {
store := s
store.Lock()
defer store.Unlock()
if modified, err := store.Modified(); modified || err != nil {
@ -2808,7 +2835,8 @@ func (s *store) Image(id string) (*Image, error) {
if err != nil {
return nil, err
}
for _, store := range append([]ROImageStore{istore}, istores...) {
for _, s := range append([]ROImageStore{istore}, istores...) {
store := s
store.Lock()
defer store.Unlock()
if modified, err := store.Modified(); modified || err != nil {
@ -2840,7 +2868,8 @@ func (s *store) ImagesByTopLayer(id string) ([]*Image, error) {
if err != nil {
return nil, err
}
for _, store := range append([]ROImageStore{istore}, istores...) {
for _, s := range append([]ROImageStore{istore}, istores...) {
store := s
store.Lock()
defer store.Unlock()
if modified, err := store.Modified(); modified || err != nil {