Re: [RFC] TODO in read-cache.c

From: Duy Nguyen <pclouds@gmail.com>
Date: Apr 6, 2019, 12:03 UTC
Message-ID: <CACsJy8AnXawOgC0eWKpSF7iGXAvPdP9=SZX1HePRABVdkiKs8g@mail.gmail.com>
In-Reply-To: <CAMknYEPS68VEkUbNxeKQvVDGjzVpBXKNAi3uA04pLwN9k4ZTfA@mail.gmail.com>

On Sat, Apr 6, 2019 at 6:42 PM Kapil Jain <jkapil.cs@gmail.com> wrote:

Show 14 quoted lines

>
> i found some TODO tasks inside `read-cache.c` in `read_index_from()`
> function. which says:
>
> /*
> * TODO trace2: replace "the_repository" with the actual repo instance
> that is associated with the given "istate".
> */
>
> this same TODO occurs at 4 other places in the same file.
>
> Will it be ok, if i complete this TODO by modifying the trace2's
> function signatures to accept `struct repository`
> and change the calls to those functions accordingly ?

trace2 API can already take 'struct repository' (the_repository is a pointer to 'struct repository'). I'm pretty sure the purpose is to _not_ pass the_repository (because it implies the default repo, which is not always true). Which means you read-cache.c's functions need to take 'struct repository *' as an argument and let the caller decide what repo they want to use.

In some cases, it will be simple. For example, if you have a look at repo_read_index(), it already knows what repo it handles, so you can just extend read_index_from() to take 'struct repository *' and pass 'repo' to it.

Be careful though, repository and istate does not have one-to-one relationship (I'll leave it to you to find out why). So you cannot replace

 return read_index_from(repo->index, repo->index_file, repo->gitdir);

in that function with

 return read_index_from(repo);

and make read_index_from() use 'repo->index'. It will have to be

 return read_index_from(repo, repo->index, repo->index_file);

-- 
Duy