On Fri, Oct 18, 2013 at 4:00 PM, Duy Nguyen <pclo...@gmail.com> wrote:
> On Fri, Oct 18, 2013 at 8:55 PM, Duy Nguyen <pclo...@gmail.com> wrote:
>> On Fri, Oct 18, 2013 at 8:17 PM, Johan Herland <jo...@herland.net> wrote:
>>> diff --git a/sha1_file.c b/sha1_file.c
>>> index f80bbe4..00ffffe 100644
>>> --- a/sha1_file.c
>>> +++ b/sha1_file.c
>>> @@ -2857,7 +2857,9 @@ static int create_tmpfile(char *buffer, size_t 
>>> bufsiz, const char *filename)
>>>                 /* Make sure the directory exists */
>>>                 memcpy(buffer, filename, dirlen);
>>>                 buffer[dirlen-1] = 0;
>>> -               if (mkdir(buffer, 0777) || adjust_shared_perm(buffer))
>>> +               if (mkdir(buffer, 0777) && errno != EEXIST)
>>> +                       return -1;
>>> +               if (adjust_shared_perm(buffer))
>>>                         return -1;
>> I was going to ask what if the created directory does not have
>> permission 0777. But adjust_shared_perm follows immediately after, so
>> we're good.
> And I sent too early. adjust_shared_perm() does rely on current dir's
> permission. So if the creator does "mkdir(buffer, 0)", we still have a
> bad permission in the end. And adjust_shared_perm() is only active
> when the repository is configured to be shared.

I'm not sure where you're going with this... Whatever happens in
another process between the first call git_mkstemp_mode() and our
mkdir() call, one of three things will happen in this code:

 (a) mkdir() succeeds, i.e. we won the race. This case is unchanged by my patch.

 (b) mkdir() fails with errno != EEXIST. This results in us returning
-1 to our caller. This case is also unchanged by my patch.

 (c) mkdir() fails with EEXIST, i.e. we lost the race. We do the
adjust_shared_perms() which might fail (-> abort) or succeed, and we
then _retry_ the git_mkstemp_mode(). There is no case where the
"whatever" that happened between the first git_mkstemp_mode() and
mkdir() will have a different outcome (create_tmpfile() failing or
suceeding) than if it had happened _before_ the first
git_mkstemp_mode(). And it is not our responsibility to control what
other/unrelated processes might end up doing with directories inside

What am I missing?


Johan Herland, <jo...@herland.net>
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to