Skip to content

Commit 089ffdb

Browse files
authored
Fix bugs, resource leaks, and test issues in cmd/desync (folbricht#310)
- store.go: Fix wrong variable in S3 bucket lookup error message (printed nil IndexStore instead of the lookup string) - chunkserver.go, mount-index.go: Fix errors.Wrapf passing err as format arg instead of the store-file path - info.go: Fix missing closing quote in unsupported format error string - cat.go, inspectchunks.go: Close output file when writing to disk - chunkserver.go: Initialize loggingResponseWriter statusCode to 200 so implicit WriteHeader is logged correctly - config.go: Write config JSON to stdout instead of stderr in read mode - prune.go: Check error from fmt.Fscanln to prevent infinite loop on closed stdin - chunkserver_test.go, indexserver_test.go: Replace require.NoError (which calls t.FailNow) with t.Errorf in server goroutines - verifyindex_test.go: Remove no-op assertions that check for empty string
1 parent 1666f06 commit 089ffdb

11 files changed

Lines changed: 21 additions & 15 deletions

cmd/desync/cat.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,12 @@ func runCat(ctx context.Context, opt catOptions, args []string) error {
6060
)
6161
if len(args) == 2 {
6262
outFileName := args[1]
63-
outFile, err = os.Create(outFileName)
63+
f, err := os.Create(outFileName)
6464
if err != nil {
6565
return err
6666
}
67+
defer f.Close()
68+
outFile = f
6769
} else {
6870
outFile = stdout
6971
}

cmd/desync/chunkserver.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ func runChunkServer(ctx context.Context, opt chunkServerOptions, args []string)
157157
// Wrapper for http.HandlerFunc to add logging for requests (and response codes)
158158
func withLog(h http.Handler, log *log.Logger) http.HandlerFunc {
159159
return func(w http.ResponseWriter, r *http.Request) {
160-
lrw := &loggingResponseWriter{ResponseWriter: w}
160+
lrw := &loggingResponseWriter{ResponseWriter: w, statusCode: http.StatusOK}
161161
h.ServeHTTP(lrw, r)
162162
log.Printf("Client: %s, Request: %s %s, Response: %d", r.RemoteAddr, r.Method, r.RequestURI, lrw.statusCode)
163163
}
@@ -178,7 +178,7 @@ func chunkServerStore(opt chunkServerOptions) (desync.Store, error) {
178178
}
179179
stores, cache, err = readStoreFile(opt.storeFile)
180180
if err != nil {
181-
return nil, errors.Wrapf(err, "failed to read store-file '%s'", err)
181+
return nil, errors.Wrapf(err, "failed to read store-file '%s'", opt.storeFile)
182182
}
183183
}
184184

cmd/desync/chunkserver_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,8 +173,9 @@ func startChunkServer(t *testing.T, args ...string) (string, context.CancelFunc)
173173
cmd := newChunkServerCommand(ctx)
174174
cmd.SetArgs(append(args, "-l", addr))
175175
go func() {
176-
_, err = cmd.ExecuteC()
177-
require.NoError(t, err)
176+
if _, err := cmd.ExecuteC(); err != nil && ctx.Err() == nil {
177+
t.Errorf("chunk server error: %v", err)
178+
}
178179
}()
179180

180181
// Wait a little for the server to start

cmd/desync/config.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ func runConfig(ctx context.Context, write bool) error {
120120
if err != nil {
121121
return err
122122
}
123-
var w io.Writer = os.Stderr
123+
var w io.Writer = os.Stdout
124124
if write {
125125
if err = os.MkdirAll(filepath.Dir(cfgFile), 0755); err != nil {
126126
return err

cmd/desync/indexserver_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,9 @@ func startIndexServer(t *testing.T, args ...string) (string, context.CancelFunc)
8383
cmd := newIndexServerCommand(ctx)
8484
cmd.SetArgs(append(args, "-l", addr))
8585
go func() {
86-
_, err = cmd.ExecuteC()
87-
require.NoError(t, err)
86+
if _, err := cmd.ExecuteC(); err != nil && ctx.Err() == nil {
87+
t.Errorf("index server error: %v", err)
88+
}
8889
}()
8990

9091
// Wait a little for the server to start

cmd/desync/info.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ func runInfo(ctx context.Context, opt infoOptions, args []string) error {
242242
fmt.Println("Chunk size avg:", results.ChunkSizeAvg)
243243
fmt.Println("Chunk size max:", results.ChunkSizeMax)
244244
default:
245-
return fmt.Errorf("unsupported output format '%s", opt.printFormat)
245+
return fmt.Errorf("unsupported output format %q", opt.printFormat)
246246
}
247247
return nil
248248
}

cmd/desync/inspectchunks.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,10 +48,12 @@ func runInspectChunks(ctx context.Context, opt inspectChunksOptions, args []stri
4848
)
4949
if len(args) == 2 {
5050
outFileName := args[1]
51-
outFile, err = os.Create(outFileName)
51+
f, err := os.Create(outFileName)
5252
if err != nil {
5353
return err
5454
}
55+
defer f.Close()
56+
outFile = f
5557
} else {
5658
outFile = stdout
5759
}

cmd/desync/mount-index.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ func mountIndexStore(opt mountIndexOptions) (desync.Store, error) {
157157
}
158158
stores, cache, err = readStoreFile(opt.storeFile)
159159
if err != nil {
160-
return nil, errors.Wrapf(err, "failed to read store-file '%s'", err)
160+
return nil, errors.Wrapf(err, "failed to read store-file '%s'", opt.storeFile)
161161
}
162162
}
163163

cmd/desync/prune.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,9 @@ func runPrune(ctx context.Context, opt pruneOptions, args []string) error {
8585
for {
8686
var a string
8787
fmt.Printf("[y/N]: ")
88-
fmt.Fscanln(os.Stdin, &a)
88+
if _, err := fmt.Fscanln(os.Stdin, &a); err != nil {
89+
return err
90+
}
8991
switch a {
9092
case "y", "Y":
9193
break ask

cmd/desync/store.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,7 @@ func indexStoreFromLocation(location string, cmdOpt cmdStoreOptions) (desync.Ind
257257
lookup = minio.BucketLookupPath
258258
case "", "auto":
259259
default:
260-
return nil, "", fmt.Errorf("unknown S3 bucket lookup type: %q", s)
260+
return nil, "", fmt.Errorf("unknown S3 bucket lookup type: %q", ls)
261261
}
262262
s, err = desync.NewS3IndexStore(&p, s3Creds, region, opt, lookup)
263263
if err != nil {

0 commit comments

Comments
 (0)