From: Duy Nguyen Date: Sat, 19 Jul 2014 12:40:48 GMT Subject: Re: [PATCH] Make locked paths absolute when current directory is changed Message-ID: In-Reply-To: On Sat, Jul 19, 2014 at 12:47 AM, Junio C Hamano 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