Re: [PATCH] read-cache: avoid git_path() race in freshen_shared_index()

From: Duy Nguyen <pclouds@gmail.com>
Date: Mar 30, 2017, 09:24 UTC
Message-ID: <CACsJy8BR9=Ju_7tSmZEH+UDqWG3Ud1KXY-m6r_jHb-aQUoUMAg@mail.gmail.com>
In-Reply-To: <20170329175614.uecvtvmzoj2wfle3@sigill.intra.peff.net>

On Thu, Mar 30, 2017 at 12:56 AM, Jeff King <peff@peff.net> wrote:

Show 13 quoted lines

> But in the end it doesn't really matter. I think code like:
>
>   const char *filename = git_path(...);
>
> or
>
>   nontrivial_function(git_path(...));
>
> is an anti-pattern. It _might_ be safe, but it's really hard to tell
> without following the complete lifetime of the return value. I've been
> tempted to suggest we should abolish git_path() entirely. But it's so
> darn useful for things like unlink(git_path(...)), or other direct
> system calls.

Yeah. I thought we killed most of those (was it your patches?).

I had a quick look at "git grep -w git_path" again. The ones in builtin/am.c, builtin/grep.c and submodule.c look very much like that anti-pattern. The one in read_index_from() probably should be replaced with git_pathdup() as well. Sorry no patches (I'm very slow these days).

-- 
Duy