Skip to content

Commit 5c2d1b4

Browse files
cpuguy83cpuguy83
committed
Add type to itterate directory
This prevents reading from directory from allocating an unbounded slice at the cost of potentially having to read more than once. It also prevents unccessary allocation in cases where an error occurs handling the file type. Signed-off-by: Brian Goff <[email protected]>
1 parent 1e0d26e commit 5c2d1b4

File tree

3 files changed

+257
-9
lines changed

3 files changed

+257
-9
lines changed

‎fs/copy.go

+23-9
Original file line numberDiff line numberDiff line change
@@ -103,11 +103,6 @@ func copyDirectory(dst, src string, inodes map[uint64]string, o *copyDirOpts) er
103103
}
104104
}
105105

106-
entries, err := os.ReadDir(src)
107-
if err != nil {
108-
return fmt.Errorf("failed to read %s: %w", src, err)
109-
}
110-
111106
if err := copyFileInfo(stat, src, dst); err != nil {
112107
return fmt.Errorf("failed to copy file info for %s: %w", dst, err)
113108
}
@@ -116,7 +111,15 @@ func copyDirectory(dst, src string, inodes map[uint64]string, o *copyDirOpts) er
116111
return fmt.Errorf("failed to copy xattrs: %w", err)
117112
}
118113

119-
for _, entry := range entries {
114+
f, err := os.Open(src)
115+
if err != nil {
116+
return err
117+
}
118+
defer f.Close()
119+
120+
dr := &dirReader{f: f}
121+
122+
handleEntry := func(entry os.DirEntry) error {
120123
source := filepath.Join(src, entry.Name())
121124
target := filepath.Join(dst, entry.Name())
122125

@@ -130,7 +133,7 @@ func copyDirectory(dst, src string, inodes map[uint64]string, o *copyDirOpts) er
130133
if err := copyDirectory(target, source, inodes, o); err != nil {
131134
return err
132135
}
133-
continue
136+
return nil
134137
case (fileInfo.Mode() & os.ModeType) == 0:
135138
link, err := getLinkSource(target, fileInfo, inodes)
136139
if err != nil {
@@ -159,7 +162,7 @@ func copyDirectory(dst, src string, inodes map[uint64]string, o *copyDirOpts) er
159162
}
160163
default:
161164
logrus.Warnf("unsupported mode: %s: %s", source, fileInfo.Mode())
162-
continue
165+
return nil
163166
}
164167

165168
if err := copyFileInfo(fileInfo, source, target); err != nil {
@@ -169,9 +172,20 @@ func copyDirectory(dst, src string, inodes map[uint64]string, o *copyDirOpts) er
169172
if err := copyXAttrs(target, source, o.xex, o.xeh); err != nil {
170173
return fmt.Errorf("failed to copy xattrs: %w", err)
171174
}
175+
return nil
172176
}
173177

174-
return nil
178+
for {
179+
entry := dr.Next()
180+
if entry == nil {
181+
break
182+
}
183+
184+
if err := handleEntry(entry); err != nil {
185+
return err
186+
}
187+
}
188+
return dr.Err()
175189
}
176190

177191
// CopyFile copies the source file to the target.

‎fs/dir.go

+53
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
/*
2+
Copyright The containerd Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package fs
18+
19+
import (
20+
"io"
21+
"os"
22+
)
23+
24+
type dirReader struct {
25+
buf []os.DirEntry
26+
f *os.File
27+
err error
28+
}
29+
30+
func (r *dirReader) Next() os.DirEntry {
31+
if len(r.buf) == 0 {
32+
infos, err := r.f.ReadDir(32)
33+
if err != nil {
34+
if err != io.EOF {
35+
r.err = err
36+
}
37+
return nil
38+
}
39+
r.buf = infos
40+
}
41+
42+
if len(r.buf) == 0 {
43+
return nil
44+
}
45+
out := r.buf[0]
46+
r.buf[0] = nil
47+
r.buf = r.buf[1:]
48+
return out
49+
}
50+
51+
func (r *dirReader) Err() error {
52+
return r.err
53+
}

‎fs/dir_test.go

+181
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
/*
2+
Copyright The containerd Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package fs
18+
19+
import (
20+
"bytes"
21+
"os"
22+
"path/filepath"
23+
"runtime"
24+
"strings"
25+
"testing"
26+
)
27+
28+
func TestDirReader(t *testing.T) {
29+
t.Run("empty dir", func(t *testing.T) {
30+
t.Parallel()
31+
32+
dr := newTestDirReader(t, nil)
33+
if dr.Next() != nil {
34+
t.Fatal("expected nil dir entry for empty dir")
35+
}
36+
// validate that another call will still be nil and not panic
37+
if dr.Next() != nil {
38+
t.Fatal("expected nil dir entry for empty dir")
39+
}
40+
})
41+
42+
t.Run("populated dir", func(t *testing.T) {
43+
t.Parallel()
44+
45+
content := map[string]*testFile{
46+
"foo": newTestFile([]byte("hello"), 0644),
47+
"bar/baz": newTestFile([]byte("world"), 0600),
48+
"bar": newTestFile(nil, os.ModeDir|0710),
49+
}
50+
found := make(map[string]bool, len(content))
51+
shouldSkip := map[string]bool{
52+
"bar/baz": true,
53+
}
54+
dr := newTestDirReader(t, content)
55+
56+
check := func(entry os.DirEntry) {
57+
tf := content[entry.Name()]
58+
if tf == nil {
59+
t.Errorf("got unknown entry: %s", entry)
60+
return
61+
}
62+
63+
fi, err := entry.Info()
64+
if err != nil {
65+
t.Error()
66+
return
67+
}
68+
69+
// Windows file permissions are not accurately represented in mode like this and will show 0666 (files) and 0777 (dirs)
70+
// As such, do not try to compare mode equality
71+
var modeOK bool
72+
if runtime.GOOS == "windows" {
73+
modeOK = fi.Mode().IsRegular() == tf.mode.IsRegular() && fi.Mode().IsDir() == tf.mode.IsDir()
74+
} else {
75+
modeOK = fi.Mode() == tf.mode
76+
}
77+
if !modeOK {
78+
t.Errorf("%s: file modes do not match, expected: %s, got: %s", fi.Name(), tf.mode, fi.Mode())
79+
}
80+
81+
if fi.Mode().IsRegular() {
82+
dt, err := os.ReadFile(filepath.Join(dr.f.Name(), entry.Name()))
83+
if err != nil {
84+
t.Error(err)
85+
return
86+
}
87+
if !bytes.Equal(tf.dt, dt) {
88+
t.Errorf("expected %q, got: %q", string(tf.dt), string(dt))
89+
}
90+
}
91+
}
92+
93+
for {
94+
entry := dr.Next()
95+
if entry == nil {
96+
break
97+
}
98+
found[entry.Name()] = true
99+
check(entry)
100+
}
101+
102+
if err := dr.Err(); err != nil {
103+
t.Fatal(err)
104+
}
105+
106+
if len(found) != len(content)-len(shouldSkip) {
107+
t.Fatalf("exected files [%s], got: [%s]", mapToStringer(content), mapToStringer(found))
108+
}
109+
for k := range shouldSkip {
110+
if found[k] {
111+
t.Errorf("expected dir reader to skip %s", k)
112+
}
113+
}
114+
})
115+
}
116+
117+
type stringerFunc func() string
118+
119+
func (f stringerFunc) String() string {
120+
return f()
121+
}
122+
123+
func mapToStringer[T any](in map[string]T) stringerFunc {
124+
return func() string {
125+
out := make([]string, 0, len(in))
126+
for k := range in {
127+
out = append(out, k)
128+
}
129+
return strings.Join(out, ",")
130+
}
131+
}
132+
133+
type testFile struct {
134+
dt []byte
135+
mode os.FileMode
136+
}
137+
138+
func newTestFile(dt []byte, mode os.FileMode) *testFile {
139+
return &testFile{
140+
dt: dt,
141+
mode: mode,
142+
}
143+
}
144+
145+
func newTestDirReader(t *testing.T, content map[string]*testFile) *dirReader {
146+
p := t.TempDir()
147+
148+
for cp, info := range content {
149+
fp := filepath.Join(p, cp)
150+
151+
switch {
152+
case info.mode.IsRegular():
153+
if err := os.MkdirAll(filepath.Dir(fp), 0755); err != nil {
154+
t.Fatal(err)
155+
}
156+
if err := os.WriteFile(fp, info.dt, info.mode.Perm()); err != nil {
157+
t.Fatal(err)
158+
}
159+
if err := os.Chmod(fp, info.mode.Perm()); err != nil {
160+
t.Fatal(err)
161+
}
162+
case info.mode.IsDir():
163+
if err := os.MkdirAll(fp, info.mode); err != nil {
164+
t.Fatal(err)
165+
}
166+
// make sure the dir has the right perms in case it was created earlier while writing a file
167+
if err := os.Chmod(fp, info.mode.Perm()); err != nil {
168+
t.Fatal(err)
169+
}
170+
default:
171+
t.Fatal("unexpected file mode")
172+
}
173+
}
174+
175+
f, err := os.Open(p)
176+
if err != nil {
177+
t.Fatal(err)
178+
}
179+
t.Cleanup(func() { f.Close() })
180+
return &dirReader{f: f}
181+
}

0 commit comments

Comments
 (0)