Re: [PATCH 1/2] merge: setup `opts` later in `checkout_fast_forward()`

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Apr 23, 2018 at 10:13 PM, Martin Ågren <martin.agren@xxxxxxxxx> wrote:
> After we initialize the various fields in `opts` but before we actually
> use them, we might return early. Move the initialization further down,
> to immediately before we use `opts`.
>
> This limits the scope of `opts` and will help a subsequent commit fix a
> memory leak without having to worry about those early returns.
>
> Signed-off-by: Martin Ågren <martin.agren@xxxxxxxxx>
> ---
>  merge.c | 32 +++++++++++++++++---------------
>  1 file changed, 17 insertions(+), 15 deletions(-)
>
> diff --git a/merge.c b/merge.c
> index f06a4773d4..f123658e58 100644
> --- a/merge.c
> +++ b/merge.c
> @@ -94,8 +94,24 @@ int checkout_fast_forward(const struct object_id *head,
>                 return -1;
>
>         memset(&trees, 0, sizeof(trees));
> -       memset(&opts, 0, sizeof(opts));
>         memset(&t, 0, sizeof(t));
> +
> +       trees[nr_trees] = parse_tree_indirect(head);
> +       if (!trees[nr_trees++]) {
> +               rollback_lock_file(&lock_file);
> +               return -1;
> +       }
> +       trees[nr_trees] = parse_tree_indirect(remote);
> +       if (!trees[nr_trees++]) {
> +               rollback_lock_file(&lock_file);
> +               return -1;
> +       }
> +       for (i = 0; i < nr_trees; i++) {
> +               parse_tree(trees[i]);
> +               init_tree_desc(t+i, trees[i]->buffer, trees[i]->size);
> +       }
> +
> +       memset(&opts, 0, sizeof(opts));
>         if (overwrite_ignore) {
>                 memset(&dir, 0, sizeof(dir));

I'm guessing the diff algorithm simply found that this was a more
compact representation of the change? It's a bit confusing when your
description indicates you "moved" some code down, but it looks like
you moved code up.

Thanks,
Jake

>                 dir.flags |= DIR_SHOW_IGNORED;
> @@ -112,20 +128,6 @@ int checkout_fast_forward(const struct object_id *head,
>         opts.fn = twoway_merge;
>         setup_unpack_trees_porcelain(&opts, "merge");
>
> -       trees[nr_trees] = parse_tree_indirect(head);
> -       if (!trees[nr_trees++]) {
> -               rollback_lock_file(&lock_file);
> -               return -1;
> -       }
> -       trees[nr_trees] = parse_tree_indirect(remote);
> -       if (!trees[nr_trees++]) {
> -               rollback_lock_file(&lock_file);
> -               return -1;
> -       }
> -       for (i = 0; i < nr_trees; i++) {
> -               parse_tree(trees[i]);
> -               init_tree_desc(t+i, trees[i]->buffer, trees[i]->size);
> -       }
>         if (unpack_trees(nr_trees, t, &opts)) {
>                 rollback_lock_file(&lock_file);
>                 return -1;
> --
> 2.17.0
>




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux