Client.MkdirAll - mimic os.MkdirAll code for consistency & perf

urjitbhatia · urjitbhatia · commit 831654e4aa2a · 2018-04-25T08:13:19.000-07:00
diff --git a/client.go b/client.go
@@ -3,12 +3,11 @@ package sftp
 import (
 	"bytes"
 	"encoding/binary"
-	"fmt"
 	"io"
 	"os"
 	"path"
-	"strings"
 	"sync/atomic"
+	"syscall"
 	"time"
 
 	"github.com/kr/fs"
@@ -687,24 +686,46 @@ func (c *Client) Mkdir(path string) error {
 // If path is already a directory, MkdirAll does nothing and returns nil.
 // If path contains a regular file, an error is returned
 func (c *Client) MkdirAll(path string) error {
-	parts := ""
-	for _, p := range strings.Split(path, "/") {
-		if p == "" {
-			continue
-		}
-		parts += "/" + p
-		dir, err := c.Stat(parts)
-		if err == nil {
-			if !dir.IsDir() {
-				return fmt.Errorf("Found a non-directory file on path: %s", parts)
-			}
-			continue
+	// Most of this code mimics https://golang.org/src/os/path.go?s=514:561#L13
+	// Fast path: if we can tell whether path is a directory or file, stop with success or error.
+	dir, err := c.Stat(path)
+	if err == nil {
+		if dir.IsDir() {
+			return nil
 		}
-		err = c.Mkdir(parts)
+		return &os.PathError{Op: "mkdir", Path: path, Err: syscall.ENOTDIR}
+	}
+
+	// Slow path: make sure parent exists and then call Mkdir for path.
+	i := len(path)
+	for i > 0 && os.IsPathSeparator(path[i-1]) { // Skip trailing path separator.
+		i--
+	}
+
+	j := i
+	for j > 0 && !os.IsPathSeparator(path[j-1]) { // Scan backward over element.
+		j--
+	}
+
+	if j > 1 {
+		// Create parent
+		err = c.MkdirAll(path[0 : j-1])
 		if err != nil {
 			return err
 		}
 	}
+
+	// Parent now exists; invoke Mkdir and use its result.
+	err = c.Mkdir(path)
+	if err != nil {
+		// Handle arguments like "foo/." by
+		// double-checking that directory doesn't exist.
+		dir, err1 := c.Lstat(path)
+		if err1 == nil && dir.IsDir() {
+			return nil
+		}
+		return err
+	}
 	return nil
 }
 
diff --git a/client_integration_test.go b/client_integration_test.go
@@ -273,9 +273,13 @@ func TestClientMkdirAll(t *testing.T) {
 	if err := sftp.MkdirAll(sub); err != nil {
 		t.Fatal(err)
 	}
-	if _, err := os.Lstat(sub); err != nil {
+	info, err := os.Lstat(sub)
+	if err != nil {
 		t.Fatal(err)
 	}
+	if !info.IsDir() {
+		t.Fatalf("Expected mkdirall to create dir at: %s", sub)
+	}
 }
 
 func TestClientOpen(t *testing.T) {

Original file line number	Diff line number	Diff line change
`@@ -273,9 +273,13 @@ func TestClientMkdirAll(t *testing.T) {`
`273`	`273`	`if err := sftp.MkdirAll(sub); err != nil {`
`274`	`274`	`t.Fatal(err)`
`275`	`275`	`}`
`276`		`- if _, err := os.Lstat(sub); err != nil {`
	`276`	`+ info, err := os.Lstat(sub)`
	`277`	`+ if err != nil {`
`277`	`278`	`t.Fatal(err)`
`278`	`279`	`}`
	`280`	`+ if !info.IsDir() {`
	`281`	`+ t.Fatalf("Expected mkdirall to create dir at: %s", sub)`
	`282`	`+ }`
`279`	`283`	`}`
`280`	`284`
`281`	`285`	`func TestClientOpen(t *testing.T) {`