All of lore.kernel.org
 help / color / mirror / Atom feed
From: Duy Nguyen <pclouds@gmail.com>
To: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Cc: Git Mailing List <git@vger.kernel.org>,
	Junio C Hamano <gitster@pobox.com>, Johannes Sixt <j6t@kdbg.org>
Subject: Re: [PATCH v2 2/2] Make locked paths absolute when current directory is changed
Date: Mon, 21 Jul 2014 20:47:39 +0700	[thread overview]
Message-ID: <CACsJy8AXc4jvLPNpGyGdY9uzrnN-SbEeiksLDpS_=29gJ1KMnQ@mail.gmail.com> (raw)
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:
>> +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

  reply	other threads:[~2014-07-21 13:48 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-18 13:08 [PATCH] Make locked paths absolute when current directory is changed Nguyễn Thái Ngọc Duy
2014-07-18 17:47 ` Junio C Hamano
2014-07-19 12:40   ` Duy Nguyen
2014-07-18 20:44 ` Johannes Sixt
2014-07-20 12:13 ` [PATCH v2 1/2] lockfile.c: remove PATH_MAX limitation (except in resolve_symlink) Nguyễn Thái Ngọc Duy
2014-07-20 12:13   ` [PATCH v2 2/2] Make locked paths absolute when current directory is changed Nguyễn Thái Ngọc Duy
2014-07-21 13:27     ` Ramsay Jones
2014-07-21 13:47       ` Duy Nguyen [this message]
2014-07-21 14:23         ` Ramsay Jones
2014-07-21 17:04         ` Junio C Hamano
2014-07-23 11:55           ` Duy Nguyen
2014-07-31  3:01             ` Yue Lin Ho
2014-07-31  9:58               ` Duy Nguyen
2014-07-20 12:47   ` [PATCH v2 1/2] lockfile.c: remove PATH_MAX limitation (except in resolve_symlink) Philip Oakley
2014-07-20 12:50     ` Duy Nguyen
2014-07-31 13:43   ` [PATCH v3 0/3] Keep .lock file paths absolute Nguyễn Thái Ngọc Duy
2014-07-31 13:43     ` [PATCH v3 1/3] lockfile.c: remove PATH_MAX limitation (except in resolve_symlink) Nguyễn Thái Ngọc Duy
2014-08-01 16:53       ` Junio C Hamano
2014-08-01 17:55         ` Junio C Hamano
2014-08-02 18:13           ` Torsten Bögershausen
2014-08-04 10:13             ` Duy Nguyen
2014-08-04 17:42               ` Junio C Hamano
2014-08-05 16:10               ` Michael Haggerty
2014-09-03  8:00                 ` Yue Lin Ho
2014-08-01 17:34       ` Junio C Hamano
2014-07-31 13:43     ` [PATCH v3 2/3] lockfile.c: remove PATH_MAX limit in resolve_symlink() Nguyễn Thái Ngọc Duy
2014-07-31 13:43     ` [PATCH v3 3/3] lockfile.c: store absolute path Nguyễn Thái Ngọc Duy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CACsJy8AXc4jvLPNpGyGdY9uzrnN-SbEeiksLDpS_=29gJ1KMnQ@mail.gmail.com' \
    --to=pclouds@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j6t@kdbg.org \
    --cc=ramsay@ramsay1.demon.co.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.