Re: [PATCH 0/3] avoiding unintended consequences of git_path() usage
- From
Ramsay Jones <ramsay@ramsay1.demon.co.uk>
- Date
- Nov 19, 2011, 19:25 UTC
- Message-ID
- <4EC802AD.1060405@ramsay1.demon.co.uk>
- In-Reply-To
- <CACsJy8CYj_s92zG-LnBKtHxV2uaG8-rq-VNJiQYwNJXGKbFeDw@mail.gmail.com>
Nguyen Thai Ngoc Duy wrote:
Show 29 quoted lines
> 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_directoriesThis 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