Re: [PATCH v2 2/2] Make locked paths absolute when current directory is changed

On 21/07/14 14:47, Duy Nguyen wrote:

On Mon, Jul 21, 2014 at 8:27 PM, Ramsay Jones
[off-list ref] wrote:

quoted

quoted

+void make_locked_paths_absolute(void)
+{
+     struct lock_file *lk;
+     for (lk = lock_file_list; lk != NULL; lk = lk->next) {
+             if (lk->filename && !is_absolute_path(lk->filename)) {
+                     char *to_free = lk->filename;
+                     lk->filename = xstrdup(absolute_path(lk->filename));
+                     free(to_free);
+             }
+     }
+}

I just have to ask, why are we putting relative pathnames in this
list to begin with? Why not use an absolute path when taking the
lock in all cases? (calling absolute_path() and using the result
to take the lock, storing it in the lock_file list, should not be
in the critical path, right? Not that I have measured it, of course! :)

Conservative :) I'm still scared from 044bbbc (Make git_dir a path
relative to work_tree in setup_work_tree() - 2008-06-19). But yeah
looking through "grep hold_" I think none of the locks is in critical
path. absolute_path() can die() if cwd is longer than PATH_MAX (and
doing this reduces the chances of that happening). But René is adding
strbuf_getcwd() that can remove that PATH_MAX. So I guess we should be
fine with putting absolute_path() in hold_lock_file_...*

Hmm, yes, thank you for reminding me about 044bbbc. So, yes it could
cause a (small) performance hit and a change in behaviour (die) in
deeply nested working directories. Hmm, OK.

ATB,
Ramsay Jones