On Sat, Jul 19, 2014 at 12:47 AM, Junio C Hamano <gits...@pobox.com> wrote:
>> +             abspath = absolute_path(lk->filename);
>> +             if (strlen(abspath) >= sizeof(lk->filename))
>> +                     warning("locked path %s is relative when current 
>> directory "
>> +                             "is changed", lk->filename);
> Shouldn't this be a die() or an error return (which will kill the
> caller anyway)?

We don't know for sure there will be a die() or something to trigger
the roll back (or commit). If the chdir() is temporary, absolute path
not fitting in PATH_MAX chars is not fatal because cwd will be
reverted before commit/rollback. A better solution is probably avoid
PATH_MAX in lk->filename. But yeah, changing it to die() is safer
(especially when cwd is moved permanently for some options in
update-index and read-tree)

>> @@ -636,6 +636,7 @@ static const char *setup_git_directory_gently_1(int 
>> *nongit_ok)
>>               die_errno("Unable to read current working directory");
>>       offset = len = strlen(cwd);
>> +     make_locked_paths_absolute();
> Just being curious, but this early in the start-up sequence, what
> files do we have locks on?

We don't know. For most builtin commands, the setup is done early and
we can be sure of no locks. Some commands (especially non-builtin) can
still delay calling setup_git_directory() until later and they might
do something in between, so better be safe than sorry.
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