From: Junio C Hamano <gitster@pobox•com>
To: "René Scharfe" <l.s.r@web•de>
Cc: Git List <git@vger•kernel.org>
Subject: Re: [PATCH] unpack-trees: pass checkout state explicitly to check_updates()
Date: Tue, 13 Sep 2016 16:25:47 -0700 [thread overview]
Message-ID: <xmqqa8fbtm9w.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <b57982ca-d508-5016-a187-fc3b829f9b0c@web.de> ("René Scharfe"'s message of "Tue, 13 Sep 2016 19:37:06 +0200")
René Scharfe <l.s.r@web•de> writes:
> Add a parameter for the struct checkout variable to check_updates()
> instead of using a static global variable. Passing it explicitly makes
> object ownership and usage more easily apparent. And we get rid of a
> static variable; those can be problematic in library-like code.
> ...
> diff --git a/unpack-trees.c b/unpack-trees.c
> index 11c37fb..74d6dd4 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -218,8 +218,8 @@ static void unlink_entry(const struct cache_entry *ce)
> schedule_dir_for_removal(ce->name, ce_namelen(ce));
> }
>
> -static struct checkout state;
> ...
> @@ -1094,6 +1094,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
> int i, ret;
> static struct cache_entry *dfc;
> struct exclude_list el;
> + struct checkout state;
Does the distinction between this thing in BSS implicitly cleared
and the new one on stack that does not seem to have any
initialization matter?
... goes and looks ...
OK, after this hunk we clear and set up everything in state, so
there is no difference in behaviour. Just we got rid of an
unnecessary file-scope global.
Nice. Thanks.
> if (len > MAX_UNPACK_TREES)
> die("unpack_trees takes at most %d trees", MAX_UNPACK_TREES);
> @@ -1239,7 +1240,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
> }
>
> o->src_index = NULL;
> - ret = check_updates(o) ? (-2) : 0;
> + ret = check_updates(o, &state) ? (-2) : 0;
> if (o->dst_index) {
> if (!ret) {
> if (!o->result.cache_tree)
prev parent reply other threads:[~2016-09-13 23:26 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-13 17:37 [PATCH] unpack-trees: pass checkout state explicitly to check_updates() René Scharfe
2016-09-13 23:25 ` Junio C Hamano [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqa8fbtm9w.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=l.s.r@web$(echo .)de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox