Re: [PATCH 1/5] object.c: get_max_object_index and get_indexed_object accept 'r' parameter

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Feb 12, 2020, 21:13 UTC
Message-ID: <CAPig+cQhh8pNppSsGGk62-VoHsOUv0aNTxHtDWY+_pousR+KMg@mail.gmail.com>
In-Reply-To: <20200212202210.GC4364@syl.local>

On Wed, Feb 12, 2020 at 3:22 PM Taylor Blau <me@ttaylorr.com> wrote:

Show 11 quoted lines

> On Wed, Feb 12, 2020 at 07:19:07PM +0000, Parth Gala via GitGitGadget wrote:
> > diff --git a/builtin/fsck.c b/builtin/fsck.c
> > @@ -375,6 +375,7 @@ static void check_object(struct object *obj)
> >  static void check_connectivity(void)
> >  {
> >       int i, max;
> > +     struct repository *r = the_repository;
>
> Is there a reason that you assign use/assign 'r' here? [...]
> but I'm not sure that it's necessary here. Could you instead pass
> 'the_repository' directly to the functions that now require it?

One benefit of doing it this way is that future patches will be much less noisy which convert these callers to also work with any 'repository' object. For instance, after the current patch, we have:

    static void check_connectivity(void)
    {
        struct repository *r = the_repository;
        max = get_max_object_index(r);
        ...

A future patch which converts check_connectivity() to work with any repository will then require very little change -- just adding an 'r' argument to the function declaration, and dropping the line which declares 'r':

    static void check_connectivity(struct repository *r)
    {
        max = get_max_object_index(r);
        ...

So, I'm fine with this patch series' approach of declaring a variable 'r' like this.