Skip to content
This repository was archived by the owner on Sep 11, 2020. It is now read-only.

Commit 2d10f10

Browse files
authored
Merge pull request #491 from smola/error-checks
*: add more IO error checks
2 parents 8738a04 + 87888ea commit 2d10f10

File tree

11 files changed

+118
-47
lines changed

11 files changed

+118
-47
lines changed

plumbing/format/packfile/common.go

+14-8
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"io"
55

66
"gopkg.in/src-d/go-git.v4/plumbing/storer"
7+
"gopkg.in/src-d/go-git.v4/utils/ioutil"
78
)
89

910
var signature = []byte{'P', 'A', 'C', 'K'}
@@ -24,14 +25,7 @@ const (
2425
// packfile.
2526
func UpdateObjectStorage(s storer.EncodedObjectStorer, packfile io.Reader) error {
2627
if sw, ok := s.(storer.PackfileWriter); ok {
27-
w, err := sw.PackfileWriter()
28-
if err != nil {
29-
return err
30-
}
31-
32-
defer w.Close()
33-
_, err = io.Copy(w, packfile)
34-
return err
28+
return writePackfileToObjectStorage(sw, packfile)
3529
}
3630

3731
stream := NewScanner(packfile)
@@ -43,3 +37,15 @@ func UpdateObjectStorage(s storer.EncodedObjectStorer, packfile io.Reader) error
4337
_, err = d.Decode()
4438
return err
4539
}
40+
41+
func writePackfileToObjectStorage(sw storer.PackfileWriter, packfile io.Reader) error {
42+
var err error
43+
w, err := sw.PackfileWriter()
44+
if err != nil {
45+
return err
46+
}
47+
48+
defer ioutil.CheckClose(w, &err)
49+
_, err = io.Copy(w, packfile)
50+
return err
51+
}

plumbing/format/packfile/scanner.go

+8-13
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,11 @@ import (
88
"hash"
99
"hash/crc32"
1010
"io"
11-
"io/ioutil"
11+
stdioutil "io/ioutil"
1212

1313
"gopkg.in/src-d/go-git.v4/plumbing"
1414
"gopkg.in/src-d/go-git.v4/utils/binary"
15+
"gopkg.in/src-d/go-git.v4/utils/ioutil"
1516
)
1617

1718
var (
@@ -198,7 +199,7 @@ func (s *Scanner) discardObjectIfNeeded() error {
198199
}
199200

200201
h := s.pendingObject
201-
n, _, err := s.NextObject(ioutil.Discard)
202+
n, _, err := s.NextObject(stdioutil.Discard)
202203
if err != nil {
203204
return err
204205
}
@@ -275,8 +276,7 @@ func (s *Scanner) NextObject(w io.Writer) (written int64, crc32 uint32, err erro
275276

276277
// ReadRegularObject reads and write a non-deltified object
277278
// from it zlib stream in an object entry in the packfile.
278-
func (s *Scanner) copyObject(w io.Writer) (int64, error) {
279-
var err error
279+
func (s *Scanner) copyObject(w io.Writer) (n int64, err error) {
280280
if s.zr == nil {
281281
zr, err := zlib.NewReader(s.r)
282282
if err != nil {
@@ -290,14 +290,9 @@ func (s *Scanner) copyObject(w io.Writer) (int64, error) {
290290
}
291291
}
292292

293-
defer func() {
294-
closeErr := s.zr.Close()
295-
if err == nil {
296-
err = closeErr
297-
}
298-
}()
299-
300-
return io.Copy(w, s.zr)
293+
defer ioutil.CheckClose(s.zr, &err)
294+
n, err = io.Copy(w, s.zr)
295+
return
301296
}
302297

303298
// SeekFromStart sets a new offset from start, returns the old position before
@@ -329,7 +324,7 @@ func (s *Scanner) Checksum() (plumbing.Hash, error) {
329324

330325
// Close reads the reader until io.EOF
331326
func (s *Scanner) Close() error {
332-
_, err := io.Copy(ioutil.Discard, s.r)
327+
_, err := io.Copy(stdioutil.Discard, s.r)
333328
return err
334329
}
335330

storage/filesystem/config.go

+6-5
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
package filesystem
22

33
import (
4-
"io/ioutil"
4+
stdioutil "io/ioutil"
55
"os"
66

77
"gopkg.in/src-d/go-git.v4/config"
88
"gopkg.in/src-d/go-git.v4/storage/filesystem/internal/dotgit"
9+
"gopkg.in/src-d/go-git.v4/utils/ioutil"
910
)
1011

1112
type ConfigStorage struct {
@@ -24,9 +25,9 @@ func (c *ConfigStorage) Config() (*config.Config, error) {
2425
return nil, err
2526
}
2627

27-
defer f.Close()
28+
defer ioutil.CheckClose(f, &err)
2829

29-
b, err := ioutil.ReadAll(f)
30+
b, err := stdioutil.ReadAll(f)
3031
if err != nil {
3132
return nil, err
3233
}
@@ -35,7 +36,7 @@ func (c *ConfigStorage) Config() (*config.Config, error) {
3536
return nil, err
3637
}
3738

38-
return cfg, nil
39+
return cfg, err
3940
}
4041

4142
func (c *ConfigStorage) SetConfig(cfg *config.Config) error {
@@ -48,7 +49,7 @@ func (c *ConfigStorage) SetConfig(cfg *config.Config) error {
4849
return err
4950
}
5051

51-
defer f.Close()
52+
defer ioutil.CheckClose(f, &err)
5253

5354
b, err := cfg.Marshal()
5455
if err != nil {

storage/filesystem/index.go

+7-4
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55

66
"gopkg.in/src-d/go-git.v4/plumbing/format/index"
77
"gopkg.in/src-d/go-git.v4/storage/filesystem/internal/dotgit"
8+
"gopkg.in/src-d/go-git.v4/utils/ioutil"
89
)
910

1011
type IndexStorage struct {
@@ -17,10 +18,11 @@ func (s *IndexStorage) SetIndex(idx *index.Index) error {
1718
return err
1819
}
1920

20-
defer f.Close()
21+
defer ioutil.CheckClose(f, &err)
2122

2223
e := index.NewEncoder(f)
23-
return e.Encode(idx)
24+
err = e.Encode(idx)
25+
return err
2426
}
2527

2628
func (s *IndexStorage) Index() (*index.Index, error) {
@@ -37,8 +39,9 @@ func (s *IndexStorage) Index() (*index.Index, error) {
3739
return nil, err
3840
}
3941

40-
defer f.Close()
42+
defer ioutil.CheckClose(f, &err)
4143

4244
d := index.NewDecoder(f)
43-
return idx, d.Decode(idx)
45+
err = d.Decode(idx)
46+
return idx, err
4447
}

storage/filesystem/internal/dotgit/dotgit.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -253,10 +253,10 @@ func (d *DotGit) SetRef(r *plumbing.Reference) error {
253253
return err
254254
}
255255

256-
if _, err := f.Write([]byte(content)); err != nil {
257-
return err
258-
}
259-
return f.Close()
256+
defer ioutil.CheckClose(f, &err)
257+
258+
_, err = f.Write([]byte(content))
259+
return err
260260
}
261261

262262
// Refs scans the git directory collecting references, which it returns.

storage/filesystem/internal/dotgit/writers_test.go

+26-2
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212

1313
. "gopkg.in/check.v1"
1414
"gopkg.in/src-d/go-billy.v3/osfs"
15+
"gopkg.in/src-d/go-git.v4/plumbing/format/packfile"
1516
)
1617

1718
func (s *SuiteDotGit) TestNewObjectPack(c *C) {
@@ -35,13 +36,30 @@ func (s *SuiteDotGit) TestNewObjectPack(c *C) {
3536

3637
c.Assert(w.Close(), IsNil)
3738

38-
stat, err := fs.Stat(fmt.Sprintf("objects/pack/pack-%s.pack", f.PackfileHash))
39+
pfPath := fmt.Sprintf("objects/pack/pack-%s.pack", f.PackfileHash)
40+
idxPath := fmt.Sprintf("objects/pack/pack-%s.idx", f.PackfileHash)
41+
42+
stat, err := fs.Stat(pfPath)
3943
c.Assert(err, IsNil)
4044
c.Assert(stat.Size(), Equals, int64(84794))
4145

42-
stat, err = fs.Stat(fmt.Sprintf("objects/pack/pack-%s.idx", f.PackfileHash))
46+
stat, err = fs.Stat(idxPath)
4347
c.Assert(err, IsNil)
4448
c.Assert(stat.Size(), Equals, int64(1940))
49+
50+
pf, err := fs.Open(pfPath)
51+
c.Assert(err, IsNil)
52+
pfs := packfile.NewScanner(pf)
53+
_, objects, err := pfs.Header()
54+
c.Assert(err, IsNil)
55+
for i := uint32(0); i < objects; i++ {
56+
_, err := pfs.NextObjectHeader()
57+
if err != nil {
58+
c.Assert(err, IsNil)
59+
break
60+
}
61+
}
62+
c.Assert(pfs.Close(), IsNil)
4563
}
4664

4765
func (s *SuiteDotGit) TestNewObjectPackUnused(c *C) {
@@ -63,6 +81,12 @@ func (s *SuiteDotGit) TestNewObjectPackUnused(c *C) {
6381
info, err := fs.ReadDir("objects/pack")
6482
c.Assert(err, IsNil)
6583
c.Assert(info, HasLen, 0)
84+
85+
// check clean up of temporary files
86+
info, err = fs.ReadDir("")
87+
for _, fi := range info {
88+
c.Assert(fi.IsDir(), Equals, true)
89+
}
6690
}
6791

6892
func (s *SuiteDotGit) TestSyncedReader(c *C) {

storage/filesystem/object.go

+12-9
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"gopkg.in/src-d/go-git.v4/plumbing/storer"
1212
"gopkg.in/src-d/go-git.v4/storage/filesystem/internal/dotgit"
1313
"gopkg.in/src-d/go-git.v4/storage/memory"
14+
"gopkg.in/src-d/go-git.v4/utils/ioutil"
1415

1516
"gopkg.in/src-d/go-billy.v3"
1617
)
@@ -53,10 +54,11 @@ func (s *ObjectStorage) loadIdxFile(h plumbing.Hash) error {
5354
if err != nil {
5455
return err
5556
}
56-
defer idxfile.Close()
5757

58+
defer ioutil.CheckClose(idxfile, &err)
5859
s.index[h] = make(idx)
59-
return s.index[h].Decode(idxfile)
60+
err = s.index[h].Decode(idxfile)
61+
return err
6062
}
6163

6264
func (s *ObjectStorage) NewEncodedObject() plumbing.EncodedObject {
@@ -94,14 +96,14 @@ func (s *ObjectStorage) SetEncodedObject(o plumbing.EncodedObject) (plumbing.Has
9496
return plumbing.ZeroHash, err
9597
}
9698

97-
defer ow.Close()
99+
defer ioutil.CheckClose(ow, &err)
98100

99101
or, err := o.Reader()
100102
if err != nil {
101103
return plumbing.ZeroHash, err
102104
}
103105

104-
defer or.Close()
106+
defer ioutil.CheckClose(or, &err)
105107

106108
if err := ow.WriteHeader(o.Type(), o.Size()); err != nil {
107109
return plumbing.ZeroHash, err
@@ -111,7 +113,7 @@ func (s *ObjectStorage) SetEncodedObject(o plumbing.EncodedObject) (plumbing.Has
111113
return plumbing.ZeroHash, err
112114
}
113115

114-
return o.Hash(), nil
116+
return o.Hash(), err
115117
}
116118

117119
// EncodedObject returns the object with the given hash, by searching for it in
@@ -143,15 +145,15 @@ func (s *ObjectStorage) getFromUnpacked(h plumbing.Hash) (obj plumbing.EncodedOb
143145
return nil, err
144146
}
145147

146-
defer f.Close()
148+
defer ioutil.CheckClose(f, &err)
147149

148150
obj = s.NewEncodedObject()
149151
r, err := objfile.NewReader(f)
150152
if err != nil {
151153
return nil, err
152154
}
153155

154-
defer r.Close()
156+
defer ioutil.CheckClose(r, &err)
155157

156158
t, size, err := r.Header()
157159
if err != nil {
@@ -186,7 +188,7 @@ func (s *ObjectStorage) getFromPackfile(h plumbing.Hash) (plumbing.EncodedObject
186188
return nil, err
187189
}
188190

189-
defer f.Close()
191+
defer ioutil.CheckClose(f, &err)
190192

191193
p := packfile.NewScanner(f)
192194
d, err := packfile.NewDecoder(p, memory.NewStorage())
@@ -195,7 +197,8 @@ func (s *ObjectStorage) getFromPackfile(h plumbing.Hash) (plumbing.EncodedObject
195197
}
196198

197199
d.SetOffsets(s.index[pack])
198-
return d.DecodeObjectAt(offset)
200+
obj, err := d.DecodeObjectAt(offset)
201+
return obj, err
199202
}
200203

201204
func (s *ObjectStorage) findObjectInPackfile(h plumbing.Hash) (plumbing.Hash, int64) {

storage/filesystem/shallow.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66

77
"gopkg.in/src-d/go-git.v4/plumbing"
88
"gopkg.in/src-d/go-git.v4/storage/filesystem/internal/dotgit"
9+
"gopkg.in/src-d/go-git.v4/utils/ioutil"
910
)
1011

1112
// ShallowStorage where the shallow commits are stored, an internal to
@@ -23,14 +24,14 @@ func (s *ShallowStorage) SetShallow(commits []plumbing.Hash) error {
2324
return err
2425
}
2526

26-
defer f.Close()
27+
defer ioutil.CheckClose(f, &err)
2728
for _, h := range commits {
2829
if _, err := fmt.Fprintf(f, "%s\n", h); err != err {
2930
return err
3031
}
3132
}
3233

33-
return nil
34+
return err
3435
}
3536

3637
// Shallow return the shallow commits reading from shallo file from .git

storage/filesystem/storage_test.go

+1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ func (s *StorageSuite) SetUpTest(c *C) {
2626
c.Assert(err, IsNil)
2727

2828
s.BaseStorageSuite = test.NewBaseStorageSuite(storage)
29+
s.BaseStorageSuite.SetUpTest(c)
2930
}
3031

3132
func (s *StorageSuite) TestFilesystem(c *C) {

storage/memory/storage_test.go

+1
Original file line numberDiff line numberDiff line change
@@ -17,4 +17,5 @@ var _ = Suite(&StorageSuite{})
1717

1818
func (s *StorageSuite) SetUpTest(c *C) {
1919
s.BaseStorageSuite = test.NewBaseStorageSuite(NewStorage())
20+
s.BaseStorageSuite.SetUpTest(c)
2021
}

0 commit comments

Comments
 (0)