Commit 999d4405a69e8d4d3d3bcd4a5d4bf45d8283b172

Russell Belfer 2013-06-05T12:02:28

Simplify git_futils_mkdir This routine was (is) pretty complicated, but given the recent changes, it seemed like it could be simplified a bit.

diff --git a/src/fileops.c b/src/fileops.c
index 088ae5e..73b03f0 100644
--- a/src/fileops.c
+++ b/src/fileops.c
@@ -277,10 +277,11 @@ int git_futils_mkdir(
 	mode_t mode,
 	uint32_t flags)
 {
-	int error = -1, tmp_errno;
+	int error = -1, verify_final;
 	git_buf make_path = GIT_BUF_INIT;
 	ssize_t root = 0, min_root_len;
 	char lastch, *tail;
+	struct stat st;
 
 	/* build path and find "root" where we should start calling mkdir */
 	if (git_path_join_unrooted(&make_path, path, base, &root) < 0)
@@ -288,7 +289,7 @@ int git_futils_mkdir(
 
 	if (make_path.size == 0) {
 		giterr_set(GITERR_OS, "Attempt to create empty path");
-		goto fail;
+		goto done;
 	}
 
 	/* remove trailing slashes on path */
@@ -315,12 +316,15 @@ int git_futils_mkdir(
 	if (root < 0)
 		root = 0;
 
+	tail = &make_path.ptr[root];
+
+	/* is there any path to make? */
+	verify_final = ((flags & GIT_MKDIR_VERIFY_DIR) != 0) && (*tail != 0);
+
 	/* make sure mkdir root is at least after filesystem root */
 	min_root_len = git_path_root(make_path.ptr);
 	if (root < min_root_len)
-		root = min_root_len;
-
-	tail = & make_path.ptr[root];
+		tail = &make_path.ptr[min_root_len];
 
 	while (*tail) {
 		/* advance tail to include next path component */
@@ -332,72 +336,58 @@ int git_futils_mkdir(
 		/* truncate path at next component */
 		lastch = *tail;
 		*tail = '\0';
+		st.st_mode = 0;
 
 		/* make directory */
 		if (p_mkdir(make_path.ptr, mode) < 0) {
-			int already_exists = 0;
-
-			switch (errno) {
-			case EEXIST:
-				if (!lastch && (flags & GIT_MKDIR_VERIFY_DIR) != 0 &&
-					!git_path_isdir(make_path.ptr)) {
-					giterr_set(
-						GITERR_OS, "Existing path is not a directory '%s'",
-						make_path.ptr);
-					error = GIT_ENOTFOUND;
-					goto fail;
-				}
-
-				already_exists = 1;
-				break;
-			case ENOSYS:
-			case EACCES:
-				/* Possible recoverable errors.  These errors could occur
-				 * on some OS if we try to mkdir at a network mount point
-				 * or at the root of a volume.  If the path is a dir, just
-				 * treat as EEXIST.
-				 */
-				tmp_errno = errno;
-
-				if (git_path_isdir(make_path.ptr)) {
-					already_exists = 1;
-					break;
-				}
-
-				/* Fall through */
+			int tmp_errno = errno;
+
+			/* ignore error if directory already exists */
+			if (p_stat(make_path.ptr, &st) < 0 || !S_ISDIR(st.st_mode)) {
 				errno = tmp_errno;
-			default:
 				giterr_set(GITERR_OS, "Failed to make directory '%s'",
 					make_path.ptr);
-				goto fail;
+				goto done;
 			}
 
-			if (already_exists && (flags & GIT_MKDIR_EXCL) != 0) {
+			/* with exclusive create, existing dir is an error */
+			if ((flags & GIT_MKDIR_EXCL) != 0) {
 				giterr_set(GITERR_OS, "Directory already exists '%s'",
 					make_path.ptr);
 				error = GIT_EEXISTS;
-				goto fail;
+				goto done;
 			}
 		}
 
-		/* chmod if requested */
+		/* chmod if requested and necessary */
 		if ((flags & GIT_MKDIR_CHMOD_PATH) != 0 ||
-			((flags & GIT_MKDIR_CHMOD) != 0 && lastch == '\0'))
+			(lastch == '\0' && (flags & GIT_MKDIR_CHMOD) != 0))
 		{
-			if (p_chmod(make_path.ptr, mode) < 0) {
+			if (st.st_mode != mode && p_chmod(make_path.ptr, mode) < 0) {
 				giterr_set(GITERR_OS, "Failed to set permissions on '%s'",
 					make_path.ptr);
-				goto fail;
+				goto done;
 			}
 		}
 
+		/* if we made it to the end, then final isdir check is not needed */
+		if (lastch == '\0')
+			verify_final = false;
+
 		*tail = lastch;
 	}
 
-	git_buf_free(&make_path);
-	return 0;
+	error = 0;
+
+	/* check that full path really is a directory if requested & needed */
+	if (verify_final &&
+		(p_stat(make_path.ptr, &st) < 0 || !S_ISDIR(st.st_mode))) {
+		giterr_set(
+			GITERR_OS, "Path is not a directory '%s'", make_path.ptr);
+		error = GIT_ENOTFOUND;
+	}
 
-fail:
+done:
 	git_buf_free(&make_path);
 	return error;
 }