Hi,
On Wed, 1 Nov 2006, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:
>
> > How about this instead:
> >
> > - mkdir(filename, 0777);
> > - if (adjust_shared_perm(filename))
> > + if (!mkdir(filename, 0777) && adjust_shared_perm(filename)) {
> > + *dir = '/';
> > return -2;
> > + }
>
> Not really. See the comment above the code you just touched.
/*
* Try to mkdir the last path component if that failed.
*
* Re-try the "link()" regardless of whether the mkdir
* succeeds, since a race might mean that somebody
* else succeeded.
*/
What I proposed does not break that. It only means that you only try to
adjust the permissions if the mkdir succeeded. If somebody else created
the directory in the mean time, it is not our job to adjust the
permissions. Worse, if the other mkdir was run by a different user, the
permission adjusting _cannot_ succeed, like you pointed out.
So I think that my patch is correct, but does not matter very much.
The other thing you asked for: adjust_shared_permissions should only try
to adjust the permissions only if they are not already correct. Here is my
attempt to implement that:
Totally untested.
Ciao,
Dscho
-- snip --
adjust_shared_perm(): Avoid unnecessary chmod()s
Sighed-off-by: Johannes E. Schindelin <johannes.schindelin@xxxxxx>
---
diff --git a/path.c b/path.c
index bb89fb0..0a2bc51 100644
--- a/path.c
+++ b/path.c
@@ -279,7 +279,7 @@ int adjust_shared_perm(const char *path)
: 0));
if (S_ISDIR(mode))
mode |= S_ISGID;
- if (chmod(path, mode) < 0)
+ if ((mode & st.st_mode != mode) && chmod(path, mode) < 0)
return -2;
return 0;
}
-
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