Martin Ågren <martin.ag...@gmail.com> writes:

> `newfd` starts out negative. If we then take the lock, `newfd` will
> become non-negative. We later check for exactly that property before
> calling `write_locked_index()`. That is, we are simply using `newfd` as
> a boolean to keep track of whether we took the lock or not. (We always
> use `newfd` and `lock_file` together, so they really are mirroring each
> other.)
>
> Drop `newfd` and check with `is_lock_file_locked()` instead. While at
> it, move the `static struct lock_file` into `cmd_checkout_index()` and
> make it non-static. It is only used in this function, and after
> 076aa2cbd (tempfile: auto-allocate tempfiles on heap, 2017-09-05), we
> can have lockfiles on the stack.
>
> Signed-off-by: Martin Ågren <martin.ag...@gmail.com>
> ---
> v2: New patch.

Well reasoned and explained.  Thanks.

>
>  builtin/checkout-index.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/builtin/checkout-index.c b/builtin/checkout-index.c
> index 39c8be05d..b0e78b819 100644
> --- a/builtin/checkout-index.c
> +++ b/builtin/checkout-index.c
> @@ -129,8 +129,6 @@ static const char * const builtin_checkout_index_usage[] 
> = {
>       NULL
>  };
>  
> -static struct lock_file lock_file;
> -
>  static int option_parse_stage(const struct option *opt,
>                             const char *arg, int unset)
>  {
> @@ -150,7 +148,7 @@ static int option_parse_stage(const struct option *opt,
>  int cmd_checkout_index(int argc, const char **argv, const char *prefix)
>  {
>       int i;
> -     int newfd = -1;
> +     struct lock_file lock_file = LOCK_INIT;
>       int all = 0;
>       int read_from_stdin = 0;
>       int prefix_length;
> @@ -206,7 +204,7 @@ int cmd_checkout_index(int argc, const char **argv, const 
> char *prefix)
>       if (index_opt && !state.base_dir_len && !to_tempfile) {
>               state.refresh_cache = 1;
>               state.istate = &the_index;
> -             newfd = hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);
> +             hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);
>       }
>  
>       /* Check out named files first */
> @@ -251,7 +249,7 @@ int cmd_checkout_index(int argc, const char **argv, const 
> char *prefix)
>       if (all)
>               checkout_all(prefix, prefix_length);
>  
> -     if (0 <= newfd &&
> +     if (is_lock_file_locked(&lock_file) &&
>           write_locked_index(&the_index, &lock_file, COMMIT_LOCK))
>               die("Unable to write new index file");
>       return 0;

Reply via email to