{"thread":{"id":"44787","subject":"[PATCH] unpack-trees: move checkout state into check_updates","startedAt":"2016-12-28T23:31:11Z","lastAt":"2016-12-29T13:53:22Z","messageCount":2,"participants":["Stefan Beller","René Scharfe"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"308477","messageId":"20161228232616.21109-1-sbeller@google.com","threadId":"44787","inReplyTo":null,"subject":"[PATCH] unpack-trees: move checkout state into check_updates","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-12-28T23:26:16Z","receivedAt":"2016-12-28T23:31:11Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The checkout state was introduced via 16da134b1f9\n(read-trees: refactor the unpack_trees() part, 2006-07-30). An attempt to\nrefactor the checkout state was done in b56aa5b268e (unpack-trees: pass\ncheckout state explicitly to check_updates(), 2016-09-13), but we can\ngo even further.\n\nThe `struct checkout state` is not used in unpack_trees apart from\ninitializing it, so move it into the function that makes use of it,\nwhich is `check_updates`.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n unpack-trees.c | 17 ++++++++---------\n 1 file changed, 8 insertions(+), 9 deletions(-)\n\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex ea799d37c5..78703af135 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -224,14 +224,18 @@ static void unlink_entry(const struct cache_entry *ce)\n \tschedule_dir_for_removal(ce->name, ce_namelen(ce));\n }\n \n-static int check_updates(struct unpack_trees_options *o,\n-\t\t\t const struct checkout *state)\n+static int check_updates(struct unpack_trees_options *o)\n {\n \tunsigned cnt = 0, total = 0;\n \tstruct progress *progress = NULL;\n \tstruct index_state *index = &o->result;\n \tint i;\n \tint errs = 0;\n+\tstruct checkout state = CHECKOUT_INIT;\n+\tstate.force = 1;\n+\tstate.quiet = 1;\n+\tstate.refresh_cache = 1;\n+\tstate.istate = &o->result;\n \n \tif (o->update && o->verbose_update) {\n \t\tfor (total = cnt = 0; cnt < index->cache_nr; cnt++) {\n@@ -270,7 +274,7 @@ static int check_updates(struct unpack_trees_options *o,\n \t\t\tdisplay_progress(progress, ++cnt);\n \t\t\tce->ce_flags &= ~CE_UPDATE;\n \t\t\tif (o->update && !o->dry_run) {\n-\t\t\t\terrs |= checkout_entry(ce, state, NULL);\n+\t\t\t\terrs |= checkout_entry(ce, &state, NULL);\n \t\t\t}\n \t\t}\n \t}\n@@ -1100,14 +1104,9 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n \tint i, ret;\n \tstatic struct cache_entry *dfc;\n \tstruct exclude_list el;\n-\tstruct checkout state = CHECKOUT_INIT;\n \n \tif (len > MAX_UNPACK_TREES)\n \t\tdie(\"unpack_trees takes at most %d trees\", MAX_UNPACK_TREES);\n-\tstate.force = 1;\n-\tstate.quiet = 1;\n-\tstate.refresh_cache = 1;\n-\tstate.istate = &o->result;\n \n \tmemset(&el, 0, sizeof(el));\n \tif (!core_apply_sparse_checkout || !o->update)\n@@ -1244,7 +1243,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n \t}\n \n \to->src_index = NULL;\n-\tret = check_updates(o, &state) ? (-2) : 0;\n+\tret = check_updates(o) ? (-2) : 0;\n \tif (o->dst_index) {\n \t\tif (!ret) {\n \t\t\tif (!o->result.cache_tree)\n-- \n2.11.0.rc2.51.g8b63c0e.dirty\n\n"},{"id":"308491","messageId":"fd00a9c2-9c9c-78bd-4700-1333574d0e21@web.de","threadId":"44787","inReplyTo":"20161228232616.21109-1-sbeller@google.com","subject":"Re: [PATCH] unpack-trees: move checkout state into check_updates","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2016-12-29T13:53:05Z","receivedAt":"2016-12-29T13:53:22Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 29.12.2016 um 00:26 schrieb Stefan Beller:\n> The checkout state was introduced via 16da134b1f9\n> (read-trees: refactor the unpack_trees() part, 2006-07-30). An attempt to\n> refactor the checkout state was done in b56aa5b268e (unpack-trees: pass\n> checkout state explicitly to check_updates(), 2016-09-13), but we can\n> go even further.\n>\n> The `struct checkout state` is not used in unpack_trees apart from\n> initializing it, so move it into the function that makes use of it,\n> which is `check_updates`.\n\nThanks for catching this, the result looks nicer.\n\n> Signed-off-by: Stefan Beller <sbeller@google.com>\n> ---\n>  unpack-trees.c | 17 ++++++++---------\n>  1 file changed, 8 insertions(+), 9 deletions(-)\n>\n> diff --git a/unpack-trees.c b/unpack-trees.c\n> index ea799d37c5..78703af135 100644\n> --- a/unpack-trees.c\n> +++ b/unpack-trees.c\n> @@ -224,14 +224,18 @@ static void unlink_entry(const struct cache_entry *ce)\n>  \tschedule_dir_for_removal(ce->name, ce_namelen(ce));\n>  }\n>\n> -static int check_updates(struct unpack_trees_options *o,\n> -\t\t\t const struct checkout *state)\n> +static int check_updates(struct unpack_trees_options *o)\n>  {\n>  \tunsigned cnt = 0, total = 0;\n>  \tstruct progress *progress = NULL;\n>  \tstruct index_state *index = &o->result;\n>  \tint i;\n>  \tint errs = 0;\n> +\tstruct checkout state = CHECKOUT_INIT;\n> +\tstate.force = 1;\n> +\tstate.quiet = 1;\n> +\tstate.refresh_cache = 1;\n> +\tstate.istate = &o->result;\n\nAnd here (as well as in two more places in this function) we could use \n\"index\" instead of \"&o->result\".  Not sure if it's worth a patch, though.\n\n>\n>  \tif (o->update && o->verbose_update) {\n>  \t\tfor (total = cnt = 0; cnt < index->cache_nr; cnt++) {\n> @@ -270,7 +274,7 @@ static int check_updates(struct unpack_trees_options *o,\n>  \t\t\tdisplay_progress(progress, ++cnt);\n>  \t\t\tce->ce_flags &= ~CE_UPDATE;\n>  \t\t\tif (o->update && !o->dry_run) {\n> -\t\t\t\terrs |= checkout_entry(ce, state, NULL);\n> +\t\t\t\terrs |= checkout_entry(ce, &state, NULL);\n>  \t\t\t}\n>  \t\t}\n>  \t}\n> @@ -1100,14 +1104,9 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n>  \tint i, ret;\n>  \tstatic struct cache_entry *dfc;\n>  \tstruct exclude_list el;\n> -\tstruct checkout state = CHECKOUT_INIT;\n>\n>  \tif (len > MAX_UNPACK_TREES)\n>  \t\tdie(\"unpack_trees takes at most %d trees\", MAX_UNPACK_TREES);\n> -\tstate.force = 1;\n> -\tstate.quiet = 1;\n> -\tstate.refresh_cache = 1;\n> -\tstate.istate = &o->result;\n>\n>  \tmemset(&el, 0, sizeof(el));\n>  \tif (!core_apply_sparse_checkout || !o->update)\n> @@ -1244,7 +1243,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n>  \t}\n>\n>  \to->src_index = NULL;\n> -\tret = check_updates(o, &state) ? (-2) : 0;\n> +\tret = check_updates(o) ? (-2) : 0;\n>  \tif (o->dst_index) {\n>  \t\tif (!ret) {\n>  \t\t\tif (!o->result.cache_tree)\n>\n"}]}