On Sat, Jul 19, 2014 at 12:47 AM, Junio C Hamano <gitster@xxxxxxxxx> 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.
--
Duy
--
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