X Tutup
Skip to content

Commit ee04cfa

Browse files
committed
Add staticcheck linter
Fix issues with sync.Pool being passed an array and not a pointer. See https://github.com/dominikh/go-tools/blob/master/cmd/staticcheck/docs/checks/SA6002 Add missing tests for content.Copy Fix T.Fatal being called in a goroutine Signed-off-by: Daniel Nephin <dnephin@gmail.com>
1 parent 2556c59 commit ee04cfa

File tree

12 files changed

+165
-63
lines changed

12 files changed

+165
-63
lines changed

.gometalinter.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
"structcheck",
1414
"unused",
1515
"varcheck",
16+
"staticcheck",
1617

1718
"gofmt",
1819
"goimports",

archive/tar.go

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,12 @@ import (
1919
"github.com/pkg/errors"
2020
)
2121

22-
var (
23-
bufferPool = &sync.Pool{
24-
New: func() interface{} {
25-
return make([]byte, 32*1024)
26-
},
27-
}
28-
)
22+
var bufferPool = &sync.Pool{
23+
New: func() interface{} {
24+
buffer := make([]byte, 32*1024)
25+
return &buffer
26+
},
27+
}
2928

3029
// Diff returns a tar stream of the computed filesystem
3130
// difference between the provided directories.
@@ -404,8 +403,8 @@ func (cw *changeWriter) HandleChange(k fs.ChangeKind, p string, f os.FileInfo, e
404403
}
405404
defer file.Close()
406405

407-
buf := bufferPool.Get().([]byte)
408-
n, err := io.CopyBuffer(cw.tw, file, buf)
406+
buf := bufferPool.Get().(*[]byte)
407+
n, err := io.CopyBuffer(cw.tw, file, *buf)
409408
bufferPool.Put(buf)
410409
if err != nil {
411410
return errors.Wrap(err, "failed to copy")
@@ -529,7 +528,7 @@ func createTarFile(ctx context.Context, path, extractDir string, hdr *tar.Header
529528
}
530529

531530
func copyBuffered(ctx context.Context, dst io.Writer, src io.Reader) (written int64, err error) {
532-
buf := bufferPool.Get().([]byte)
531+
buf := bufferPool.Get().(*[]byte)
533532
defer bufferPool.Put(buf)
534533

535534
for {
@@ -540,9 +539,9 @@ func copyBuffered(ctx context.Context, dst io.Writer, src io.Reader) (written in
540539
default:
541540
}
542541

543-
nr, er := src.Read(buf)
542+
nr, er := src.Read(*buf)
544543
if nr > 0 {
545-
nw, ew := dst.Write(buf[0:nr])
544+
nw, ew := dst.Write((*buf)[0:nr])
546545
if nw > 0 {
547546
written += int64(nw)
548547
}

content/helpers.go

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,12 @@ import (
1010
"github.com/pkg/errors"
1111
)
1212

13-
var (
14-
bufPool = sync.Pool{
15-
New: func() interface{} {
16-
return make([]byte, 1<<20)
17-
},
18-
}
19-
)
13+
var bufPool = sync.Pool{
14+
New: func() interface{} {
15+
buffer := make([]byte, 1<<20)
16+
return &buffer
17+
},
18+
}
2019

2120
// NewReader returns a io.Reader from a ReaderAt
2221
func NewReader(ra ReaderAt) io.Reader {
@@ -88,10 +87,10 @@ func Copy(ctx context.Context, cw Writer, r io.Reader, size int64, expected dige
8887
}
8988
}
9089

91-
buf := bufPool.Get().([]byte)
90+
buf := bufPool.Get().(*[]byte)
9291
defer bufPool.Put(buf)
9392

94-
if _, err := io.CopyBuffer(cw, r, buf); err != nil {
93+
if _, err := io.CopyBuffer(cw, r, *buf); err != nil {
9594
return err
9695
}
9796

content/helpers_test.go

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
package content
2+
3+
import (
4+
"bytes"
5+
"context"
6+
"io"
7+
"strings"
8+
"testing"
9+
10+
"github.com/containerd/containerd/errdefs"
11+
"github.com/opencontainers/go-digest"
12+
"github.com/stretchr/testify/assert"
13+
"github.com/stretchr/testify/require"
14+
)
15+
16+
type copySource struct {
17+
reader io.Reader
18+
size int64
19+
digest digest.Digest
20+
}
21+
22+
func TestCopy(t *testing.T) {
23+
defaultSource := newCopySource("this is the source to copy")
24+
25+
var testcases = []struct {
26+
name string
27+
source copySource
28+
writer fakeWriter
29+
expected string
30+
}{
31+
{
32+
name: "copy no offset",
33+
source: defaultSource,
34+
writer: fakeWriter{},
35+
expected: "this is the source to copy",
36+
},
37+
{
38+
name: "copy with offset from seeker",
39+
source: defaultSource,
40+
writer: fakeWriter{status: Status{Offset: 8}},
41+
expected: "the source to copy",
42+
},
43+
{
44+
name: "copy with offset from unseekable source",
45+
source: copySource{reader: bytes.NewBufferString("foo"), size: 3},
46+
writer: fakeWriter{status: Status{Offset: 8}},
47+
expected: "foo",
48+
},
49+
{
50+
name: "commit already exists",
51+
source: defaultSource,
52+
writer: fakeWriter{commitFunc: func() error {
53+
return errdefs.ErrAlreadyExists
54+
}},
55+
},
56+
}
57+
58+
for _, testcase := range testcases {
59+
t.Run(testcase.name, func(t *testing.T) {
60+
err := Copy(context.Background(),
61+
&testcase.writer,
62+
testcase.source.reader,
63+
testcase.source.size,
64+
testcase.source.digest)
65+
66+
require.NoError(t, err)
67+
assert.Equal(t, testcase.source.digest, testcase.writer.commitedDigest)
68+
assert.Equal(t, testcase.expected, testcase.writer.String())
69+
})
70+
}
71+
}
72+
73+
func newCopySource(raw string) copySource {
74+
return copySource{
75+
reader: strings.NewReader(raw),
76+
size: int64(len(raw)),
77+
digest: digest.FromBytes([]byte(raw)),
78+
}
79+
}
80+
81+
type fakeWriter struct {
82+
bytes.Buffer
83+
commitedDigest digest.Digest
84+
status Status
85+
commitFunc func() error
86+
}
87+
88+
func (f *fakeWriter) Close() error {
89+
f.Buffer.Reset()
90+
return nil
91+
}
92+
93+
func (f *fakeWriter) Commit(ctx context.Context, size int64, expected digest.Digest, opts ...Opt) error {
94+
f.commitedDigest = expected
95+
if f.commitFunc == nil {
96+
return nil
97+
}
98+
return f.commitFunc()
99+
}
100+
101+
func (f *fakeWriter) Digest() digest.Digest {
102+
return f.commitedDigest
103+
}
104+
105+
func (f *fakeWriter) Status() (Status, error) {
106+
return f.status, nil
107+
}
108+
109+
func (f *fakeWriter) Truncate(size int64) error {
110+
f.Buffer.Truncate(int(size))
111+
return nil
112+
}

content/local/store.go

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,12 @@ import (
2121
"github.com/pkg/errors"
2222
)
2323

24-
var (
25-
bufPool = sync.Pool{
26-
New: func() interface{} {
27-
return make([]byte, 1<<20)
28-
},
29-
}
30-
)
24+
var bufPool = sync.Pool{
25+
New: func() interface{} {
26+
buffer := make([]byte, 1<<20)
27+
return &buffer
28+
},
29+
}
3130

3231
// LabelStore is used to store mutable labels for digests
3332
type LabelStore interface {
@@ -463,10 +462,10 @@ func (s *store) writer(ctx context.Context, ref string, total int64, expected di
463462
}
464463
defer fp.Close()
465464

466-
p := bufPool.Get().([]byte)
465+
p := bufPool.Get().(*[]byte)
467466
defer bufPool.Put(p)
468467

469-
offset, err = io.CopyBuffer(digester.Hash(), fp, p)
468+
offset, err = io.CopyBuffer(digester.Hash(), fp, *p)
470469
if err != nil {
471470
return nil, err
472471
}

events/exchange/exchange_test.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package exchange
22

33
import (
44
"context"
5-
"fmt"
65
"reflect"
76
"sync"
87
"testing"
@@ -39,13 +38,14 @@ func TestExchangeBasic(t *testing.T) {
3938
t.Log("publish")
4039
var wg sync.WaitGroup
4140
wg.Add(1)
41+
errChan := make(chan error)
4242
go func() {
4343
defer wg.Done()
44+
defer close(errChan)
4445
for _, event := range testevents {
45-
fmt.Println("publish", event)
4646
if err := exchange.Publish(ctx, "/test", event); err != nil {
47-
fmt.Println("publish error", err)
48-
t.Fatal(err)
47+
errChan <- err
48+
return
4949
}
5050
}
5151

@@ -54,6 +54,9 @@ func TestExchangeBasic(t *testing.T) {
5454

5555
t.Log("waiting")
5656
wg.Wait()
57+
if err := <-errChan; err != nil {
58+
t.Fatal(err)
59+
}
5760

5861
for _, subscriber := range []struct {
5962
eventq <-chan *events.Envelope

fs/copy.go

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,12 @@ import (
99
"github.com/pkg/errors"
1010
)
1111

12-
var (
13-
bufferPool = &sync.Pool{
14-
New: func() interface{} {
15-
return make([]byte, 32*1024)
16-
},
17-
}
18-
)
12+
var bufferPool = &sync.Pool{
13+
New: func() interface{} {
14+
buffer := make([]byte, 32*1024)
15+
return &buffer
16+
},
17+
}
1918

2019
// CopyDir copies the directory from src to dst.
2120
// Most efficient copy of files is attempted.

fs/copy_linux.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,8 @@ func copyFileContent(dst, src *os.File) error {
4343
return errors.Wrap(err, "copy file range failed")
4444
}
4545

46-
buf := bufferPool.Get().([]byte)
47-
_, err = io.CopyBuffer(dst, src, buf)
46+
buf := bufferPool.Get().(*[]byte)
47+
_, err = io.CopyBuffer(dst, src, *buf)
4848
bufferPool.Put(buf)
4949
return err
5050
}

fs/copy_unix.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ func copyFileInfo(fi os.FileInfo, name string) error {
3434
}
3535

3636
func copyFileContent(dst, src *os.File) error {
37-
buf := bufferPool.Get().([]byte)
38-
_, err := io.CopyBuffer(dst, src, buf)
37+
buf := bufferPool.Get().(*[]byte)
38+
_, err := io.CopyBuffer(dst, src, *buf)
3939
bufferPool.Put(buf)
4040

4141
return err

fs/copy_windows.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ func copyFileInfo(fi os.FileInfo, name string) error {
1818
}
1919

2020
func copyFileContent(dst, src *os.File) error {
21-
buf := bufferPool.Get().([]byte)
22-
_, err := io.CopyBuffer(dst, src, buf)
21+
buf := bufferPool.Get().(*[]byte)
22+
_, err := io.CopyBuffer(dst, src, *buf)
2323
bufferPool.Put(buf)
2424
return err
2525
}

0 commit comments

Comments
 (0)
X Tutup