threads / patch / 3280

patchstat() for existence in safe_create_leading_directories()

Subject: [PATCH] stat() for existence in safe_create_leading_directories()

## tl;dr

2 messages between Feb 10, 2006 and Feb 10, 2006. Diffs are folded; open one to read it.

replies: 1people: 2as markdown or json

Jason Riedy· Feb 10, 2006, 01:56 UTC · lore

Use stat() to explicitly check for existence rather than relying on the non-portable EEXIST error in sha1_file.c's safe_create_leading_directories(). There certainly are optimizations possible, but then the code becomes almost the same as that in coreutil's lib/mkdir-p.c.

Other uses of EEXIST seem ok. Tested on Solaris 8, AIX 5.2L, and a few Linux versions. AIX has some unrelated (I think) failures right now; I haven't tried many recent gits there. Anyone have an old Ultrix box to break everything? ;)

Also remove extraneous #includes. Everything's already in git-compat-util.h, included through cache.h.

Signed-off-by: Jason Riedy <ejr@cs.berkeley.edu>
---
 sha1_file.c |   15 ++++++++++-----
 1 files changed, 10 insertions(+), 5 deletions(-)
597de7026f284d7bb5ea5b929a70b3cf3077e4e7
Show changes to sha1_file.c +10 −5
diff --git a/sha1_file.c b/sha1_file.c
index 20f6419..3d11a9b 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -6,8 +6,6 @@
  * This handles basic git sha1 object files - packing, unpacking,
  * creation etc.
  */
-#include <sys/types.h>
-#include <dirent.h>
 #include "cache.h"
 #include "delta.h"
 #include "pack.h"
@@ -74,6 +72,8 @@ int adjust_shared_perm(const char *path)
 int safe_create_leading_directories(char *path)
 {
 	char *pos = path;
+	struct stat st;
+
 	if (*pos == '/')
 		pos++;
 
@@ -82,12 +82,17 @@ int safe_create_leading_directories(char
 		if (!pos)
 			break;
 		*pos = 0;
-		if (mkdir(path, 0777) < 0) {
-			if (errno != EEXIST) {
+		if (!stat(path, &st)) {
+			/* path exists */
+			if (!S_ISDIR(st.st_mode)) {
 				*pos = '/';
-				return -1;
+				return -3;
 			}
 		}
+		else if (mkdir(path, 0777)) {
+			*pos = '/';
+			return -1;
+		}
 		else if (adjust_shared_perm(path)) {
 			*pos = '/';
 			return -2;
-- 
1.1.5
Junio C Hamano· Feb 10, 2006, 02:30 UTC · re: Jason Riedy · lore

Re: [PATCH] stat() for existence in safe_create_leading_directories()

Thanks.
BTW, in an earlier message when I said this:
Jason Riedy <ejr@EECS.Berkeley.EDU> writes:
Show 9 quoted lines
> And Junio C Hamano writes:
>
>  - Somehow I started to trust your ability to code portably a lot
>  - better than I trust myself, [...]
>
> eep.  ;)  I only have access to three major variations at the
> moment (Linux, semi-old Solaris, recent AIX), so I'm not an
> authority...  Solaris 8 is from before the general Linux/glibc 
> compatability movement, so it's pretty useful for testing.

I was talking more about competence rather than access to various environments to test on. I forgot to mention that.

← back to recent threads