Re: git's use of mkdir(2)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



>> Specifically, if /foo/bar does not exist but /foo does,
>> mkdir("/foo/bar/",...) works on one and fails showing ENOENT on the
>> other.  (Without the trailing slash, it works on both.)
> What vintage of BSD do you have that exhibits the problem?

NetBSD 1.4T.  (About a decade old at this point.)  It's unlikely that
the machine I most care about this on is going to switch versions
anytime soon, though I may be able to convince its mkdir(2) to strip
trailing slashes - I should look at the other implementation....

> It smells like a POSIX violation, considering what "4.12 Pathname
> Resolution" says on the matter:

>   A pathname that contains at least one non-<slash> character and
>   that ends with one or more trailing <slash> characters shall not be
>   resolved successfully unless the last pathname component before the
>   trailing <slash> characters names an existing directory or a
>   directory entry that is to be created for a directory immediately
>   after the pathname is resolved.

Well, notice that it does not say that it shall be successfully
resolved in either of those exception cases, just that it shall not be
successfully resolved in other cases.  I read this as "you can append a
slash to require that the last component be a directory", with the
second exception so that the mkdir-with-trailing-slash-works behaviour
is permitted; I don't read it as mandating that mkdir work with a
trailing slash.  Is there a rationale of any sort that elaborates on
this choice, or perhaps something about mkdir elsewhere?

> Not that I am saying that such a system does not deserve to be
> supported, but I am curious to know how widespread the damage is.

I don't know.  Of my own experience, I know one version that tolerates
what git does and one version that doesn't; I haven't experimented to
try to determine how widespread either is - though I can infer that
Linux falls in the trailing-slash-allowed camp.

For the moment, I bludgeoned it into working with the patch below, but
I haven't really exercised it very much yet, so it's entirely possible
it's going to trip over the same thing somewhere else - there are a lot
of mkdir calls scattered around and I haven't looked at any outside
builtin-init-db.c.

Personally, I'd tend to avoid depending on this detail of mkdir, when
it's easy (which it appears to be in the case of builtin-init-db.c)
just on general principles.  But I don't know whether this is outside
the scope of what git cares about - I just know _I_ care about it. :)

Given the existing code, a mkdir wrapper which strips trailing slashes
might be the least-pain answer; it feels wrong to accrete wrappers like
that all over the place, but sometimes they are right answers....

/~\ The ASCII				  Mouse
\ / Ribbon Campaign
 X  Against HTML		mouse@xxxxxxxxxxxxxxxxxxxx
/ \ Email!	     7D C8 61 52 5D E7 2D 39  4E F1 31 3E E8 B3 27 4B

Here's the patch I mentioned above.  (This is relative to 1.6.4.1's
builtin-init-db.c; so far I haven't tried to build anything newer.  I
have a handful more patches which I apply as part of my build script,
but they're not relevant to this particular thread; I don't think any
of them interact with this one.)

--- OLD/builtin-init-db.c	Thu Jan  1 00:00:00 1970
+++ NEW/builtin-init-db.c	Thu Jan  1 00:00:00 1970
@@ -46,6 +46,8 @@
 	 * be really carefully chosen.
 	 */
 	safe_create_dir(path, 1);
+	path[baselen++] = '/';
+	template[template_baselen++] = '/';
 	while ((de = readdir(dir)) != NULL) {
 		struct stat st_git, st_template;
 		int namelen;
@@ -75,8 +77,6 @@
 			int template_baselen_sub = template_baselen + namelen;
 			if (!subdir)
 				die_errno("cannot opendir '%s'", template);
-			path[baselen_sub++] =
-				template[template_baselen_sub++] = '/';
 			path[baselen_sub] =
 				template[template_baselen_sub] = 0;
 			copy_templates_1(path, baselen_sub,
@@ -127,10 +127,6 @@
 	if (PATH_MAX <= (template_len+strlen("/config")))
 		die("insanely long template path %s", template_dir);
 	strcpy(template_path, template_dir);
-	if (template_path[template_len-1] != '/') {
-		template_path[template_len++] = '/';
-		template_path[template_len] = 0;
-	}
 	dir = opendir(template_path);
 	if (!dir) {
 		warning("templates not found %s", template_dir);
@@ -138,7 +134,7 @@
 	}
 
 	/* Make sure that template is from the correct vintage */
-	strcpy(template_path + template_len, "config");
+	strcpy(template_path + template_len, "/config");
 	repository_format_version = 0;
 	git_config_from_file(check_repository_format_version,
 			     template_path, NULL);
@@ -155,8 +151,6 @@
 	}
 
 	memcpy(path, git_dir, len);
-	if (len && path[len - 1] != '/')
-		path[len++] = '/';
 	path[len] = 0;
 	copy_templates_1(path, len,
 			 template_path, template_len,
@@ -217,7 +211,7 @@
 	 * Create the default symlink from ".git/HEAD" to the "master"
 	 * branch, if it does not exist yet.
 	 */
-	strcpy(path + len, "HEAD");
+	strcpy(path + len, "/HEAD");
 	reinit = (!access(path, R_OK)
 		  || readlink(path, junk, sizeof(junk)-1) != -1);
 	if (!reinit) {
@@ -230,7 +224,7 @@
 	git_config_set("core.repositoryformatversion", repo_version_string);
 
 	path[len] = 0;
-	strcpy(path + len, "config");
+	strcpy(path + len, "/config");
 
 	/* Check filemode trustability */
 	filemode = TEST_FILEMODE;
@@ -259,7 +253,7 @@
 	if (!reinit) {
 		/* Check if symlink is supported in the work tree */
 		path[len] = 0;
-		strcpy(path + len, "tXXXXXX");
+		strcpy(path + len, "/tXXXXXX");
 		if (!close(xmkstemp(path)) &&
 		    !unlink(path) &&
 		    !symlink("testing", path) &&
@@ -271,7 +265,7 @@
 
 		/* Check if the filesystem is case-insensitive */
 		path[len] = 0;
-		strcpy(path + len, "CoNfIg");
+		strcpy(path + len, "/CoNfIg");
 		if (!access(path, F_OK))
 			git_config_set("core.ignorecase", "true");
 	}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]