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

From: Duy Nguyen <pclouds@gmail.com>
Date: Jul 21, 2014, 13:47 UTC
Message-ID: <CACsJy8AXc4jvLPNpGyGdY9uzrnN-SbEeiksLDpS_=29gJ1KMnQ@mail.gmail.com>
In-Reply-To: <53CD1529.9080102@ramsay1.demon.co.uk>

On Mon, Jul 21, 2014 at 8:27 PM, Ramsay Jones <ramsay@ramsay1.demon.co.uk> wrote:

Show 17 quoted lines

>> +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_...*

-- 
Duy