Browse code

Improve ValidateContextDirectory error handling.

- Errors sent to the walker callback functions were ignored. This meant that
one could get a panic when calling methods on a nil FileInfo object. For
example when the file did not exists any more.

- Lstat calls inside walker callback are reduntant because walker already calls
Lstat and passes the result to the callback.

- Error returned from filepath.Rel() can never be EACCES because it compares
strings and does not care about actual files.

- If Matched() returns error then ValidateContextDirectory() must return error.
Currently it still kept walking although the outcome was already known.

- Function will now fail in case of unknown error(not EACCES nor ENOENT).
Previous implementation did not make a clear decision about this (but
panicked because of the issues above).

Signed-off-by: Tõnis Tiigi <tonistiigi@gmail.com> (github: tonistiigi)

Tonis Tiigi authored on 2014/09/15 00:13:40
Showing 1 changed files
... ...
@@ -523,48 +523,44 @@ func TreeSize(dir string) (size int64, err error) {
523 523
 // can be read and returns an error if some files can't be read
524 524
 // symlinks which point to non-existing files don't trigger an error
525 525
 func ValidateContextDirectory(srcPath string, excludes []string) error {
526
-	var finalError error
527
-
528
-	filepath.Walk(filepath.Join(srcPath, "."), func(filePath string, f os.FileInfo, err error) error {
526
+	return filepath.Walk(filepath.Join(srcPath, "."), func(filePath string, f os.FileInfo, err error) error {
529 527
 		// skip this directory/file if it's not in the path, it won't get added to the context
530
-		relFilePath, err := filepath.Rel(srcPath, filePath)
531
-		if err != nil && os.IsPermission(err) {
532
-			return nil
533
-		}
534
-
535
-		skip, err := Matches(relFilePath, excludes)
536
-		if err != nil {
537
-			finalError = err
538
-		}
539
-		if skip {
528
+		if relFilePath, err := filepath.Rel(srcPath, filePath); err != nil {
529
+			return err
530
+		} else if skip, err := Matches(relFilePath, excludes); err != nil {
531
+			return err
532
+		} else if skip {
540 533
 			if f.IsDir() {
541 534
 				return filepath.SkipDir
542 535
 			}
543 536
 			return nil
544 537
 		}
545 538
 
546
-		if _, err := os.Stat(filePath); err != nil && os.IsPermission(err) {
547
-			finalError = fmt.Errorf("can't stat '%s'", filePath)
539
+		if err != nil {
540
+			if os.IsPermission(err) {
541
+				return fmt.Errorf("can't stat '%s'", filePath)
542
+			}
543
+			if os.IsNotExist(err) {
544
+				return nil
545
+			}
548 546
 			return err
549 547
 		}
548
+
550 549
 		// skip checking if symlinks point to non-existing files, such symlinks can be useful
551 550
 		// also skip named pipes, because they hanging on open
552
-		lstat, _ := os.Lstat(filePath)
553
-		if lstat != nil && lstat.Mode()&(os.ModeSymlink|os.ModeNamedPipe) != 0 {
551
+		if f.Mode()&(os.ModeSymlink|os.ModeNamedPipe) != 0 {
554 552
 			return nil
555 553
 		}
556 554
 
557 555
 		if !f.IsDir() {
558 556
 			currentFile, err := os.Open(filePath)
559 557
 			if err != nil && os.IsPermission(err) {
560
-				finalError = fmt.Errorf("no permission to read from '%s'", filePath)
561
-				return err
558
+				return fmt.Errorf("no permission to read from '%s'", filePath)
562 559
 			}
563 560
 			currentFile.Close()
564 561
 		}
565 562
 		return nil
566 563
 	})
567
-	return finalError
568 564
 }
569 565
 
570 566
 func StringsContainsNoCase(slice []string, s string) bool {