From: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Date: Sat, 19 Nov 2011 19:25:33 GMT
Subject: Re: [PATCH 0/3] avoiding unintended consequences of git_path() usage
Message-ID: <4EC802AD.1060405@ramsay1.demon.co.uk>
In-Reply-To: <CACsJy8CYj_s92zG-LnBKtHxV2uaG8-rq-VNJiQYwNJXGKbFeDw@mail.gmail.com>

Nguyen Thai Ngoc Duy wrote:
> On Wed, Nov 16, 2011 at 3:59 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
>> Nguyen Thai Ngoc Duy wrote:
>>
>>> Or perhaps
>> [...]
>>>  - git_path(const char *path) maintains a small hash table to keep
>>> track of all returned strings based with "path" as key.
>>>
>>> Out of 142 git_path() calls in my tree, 97 of them are in form
>>> git_path("some static string").
>> The main bit I dislike about patch 3/3 is that constructs like
>> 'unlink(git_path("MERGE_HEAD"));' are not actually unsafe
> 
> Well, we can create wrappers (e.g. repo_unlink(const char *) that
> calls git_path internally). According to grep/sed these functions are
> used in form xxx(git_path(xxx))
> 
>      16 unlink
>       8 file_exists
>       7 stat
>       6 fopen
>       5 rename
>       5 open
>       4 unlink_or_warn
>       3 safe_create_dir
>       3 adjust_shared_perm
>       3 access
>       2 xstrdup
>       2 safe_create_leading_directories

This one at least, maybe others, is unsafe on cygwin. Indeed it causes
a test failure in t3200-branch.sh; patch is on it's way ...

ATB,
Ramsay Jones