Re: [PATCH 49/83] builtin/apply: move 'lock_file' global into 'struct apply_state'

From: Christian Couder <christian.couder@gmail.com>
Date: Apr 30, 2016, 19:39 UTC
Message-ID: <CAP8UFD1bDj2bnRSpo3Gp5B4Du6fNGSUfXv6dLL6Cs4KsJTw5Zg@mail.gmail.com>
In-Reply-To: <CAPig+cRSe8oOjo2h6SuJQyD+he_Q7zHfF4TivZ0amhAu4HLQ+g@mail.gmail.com>

On Mon, Apr 25, 2016 at 9:50 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:

Show 8 quoted lines

>> @@ -4515,8 +4521,6 @@ static int write_out_results(struct apply_state *state, struct patch *list)
>>         return errs;
>>  }
>>
>> -static struct lock_file lock_file;
>
> Does the static lock_file in build_fake_ancestor() deserve the same
> sort of treatment? (I haven't traced the code enough to answer this.)

Maybe yes we could do the same thing for this static lock_file, but this can be done later, and it could be a bit involved, so I prefer to not touch that for now.

We are using the lock_file like this in build_fake_ancestor():

    hold_lock_file_for_update(&lock, filename, LOCK_DIE_ON_ERROR);
    if (write_locked_index(&result, &lock, COMMIT_LOCK))
        return error("Could not write temporary index to %s", filename);

so it looks like it is safe to call build_fake_ancestor() many times as long as it is not called by different threads.