From: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
To: git@vger.kernel.org
Cc: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Subject: [PATCH/RFC/BUG] unpack-trees.c: do not use "the_index"
Date: Fri, 1 Jun 2018 18:11:53 +0200 [thread overview]
Message-ID: <20180601161153.15192-1-pclouds@gmail.com> (raw)
unpack-trees code works on multiple indexes specified in
unpack_trees_options. Although they normally all refer to the_index at
the call site, that is the caller's business. unpack-trees.c should
not make any assumption about that and should use the correct index
field in unpack_trees_options.
This patch is actually confusing because sometimes the function
parameter is also named "the_index" while some other times "the_index"
is the global variable because the function just does not have a
parameter of the same name! The only subtle difference is that the
function parameter is a pointer while the global one is not.
This is more of a bug report than an actual fix because I'm not sure
if "o->src_index" is always the correct answer instead of "the_index"
here. But this is very similar to 7db118303a (unpack_trees: fix
breakage when o->src_index != o->dst_index - 2018-04-23) and could
potentially break things again...
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
unpack-trees.c | 45 ++++++++++++++++++++++++++-------------------
1 file changed, 26 insertions(+), 19 deletions(-)
diff --git a/unpack-trees.c b/unpack-trees.c
index 3a85a02a77..114496cfc2 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -18,6 +18,9 @@
#include "fsmonitor.h"
#include "fetch-object.h"
+/* Do not use the_index here, you probably want o->src_index */
+#define the_index the_index_should_not_be_used here
+
/*
* Error messages expected by scripts out of plumbing commands such as
* read-tree. Non-scripted Porcelain is not required to use these messages
@@ -1085,13 +1088,15 @@ static int unpack_callback(int n, unsigned long mask, unsigned long dirmask, str
return mask;
}
-static int clear_ce_flags_1(struct cache_entry **cache, int nr,
+static int clear_ce_flags_1(struct index_state *istate,
+ struct cache_entry **cache, int nr,
struct strbuf *prefix,
int select_mask, int clear_mask,
struct exclude_list *el, int defval);
/* Whole directory matching */
-static int clear_ce_flags_dir(struct cache_entry **cache, int nr,
+static int clear_ce_flags_dir(struct index_state *istate,
+ struct cache_entry **cache, int nr,
struct strbuf *prefix,
char *basename,
int select_mask, int clear_mask,
@@ -1100,7 +1105,7 @@ static int clear_ce_flags_dir(struct cache_entry **cache, int nr,
struct cache_entry **cache_end;
int dtype = DT_DIR;
int ret = is_excluded_from_list(prefix->buf, prefix->len,
- basename, &dtype, el, &the_index);
+ basename, &dtype, el, istate);
int rc;
strbuf_addch(prefix, '/');
@@ -1122,7 +1127,7 @@ static int clear_ce_flags_dir(struct cache_entry **cache, int nr,
* calling clear_ce_flags_1(). That function will call
* the expensive is_excluded_from_list() on every entry.
*/
- rc = clear_ce_flags_1(cache, cache_end - cache,
+ rc = clear_ce_flags_1(istate, cache, cache_end - cache,
prefix,
select_mask, clear_mask,
el, ret);
@@ -1145,7 +1150,8 @@ static int clear_ce_flags_dir(struct cache_entry **cache, int nr,
* cache[0]->name[0..(prefix_len-1)]
* Top level path has prefix_len zero.
*/
-static int clear_ce_flags_1(struct cache_entry **cache, int nr,
+static int clear_ce_flags_1(struct index_state *istate,
+ struct cache_entry **cache, int nr,
struct strbuf *prefix,
int select_mask, int clear_mask,
struct exclude_list *el, int defval)
@@ -1179,7 +1185,7 @@ static int clear_ce_flags_1(struct cache_entry **cache, int nr,
len = slash - name;
strbuf_add(prefix, name, len);
- processed = clear_ce_flags_dir(cache, cache_end - cache,
+ processed = clear_ce_flags_dir(istate, cache, cache_end - cache,
prefix,
prefix->buf + prefix->len - len,
select_mask, clear_mask,
@@ -1193,7 +1199,7 @@ static int clear_ce_flags_1(struct cache_entry **cache, int nr,
}
strbuf_addch(prefix, '/');
- cache += clear_ce_flags_1(cache, cache_end - cache,
+ cache += clear_ce_flags_1(istate, cache, cache_end - cache,
prefix,
select_mask, clear_mask, el, defval);
strbuf_setlen(prefix, prefix->len - len - 1);
@@ -1203,7 +1209,7 @@ static int clear_ce_flags_1(struct cache_entry **cache, int nr,
/* Non-directory */
dtype = ce_to_dtype(ce);
ret = is_excluded_from_list(ce->name, ce_namelen(ce),
- name, &dtype, el, &the_index);
+ name, &dtype, el, istate);
if (ret < 0)
ret = defval;
if (ret > 0)
@@ -1213,15 +1219,17 @@ static int clear_ce_flags_1(struct cache_entry **cache, int nr,
return nr - (cache_end - cache);
}
-static int clear_ce_flags(struct cache_entry **cache, int nr,
- int select_mask, int clear_mask,
- struct exclude_list *el)
+static int clear_ce_flags(struct index_state *istate,
+ int select_mask, int clear_mask,
+ struct exclude_list *el)
{
static struct strbuf prefix = STRBUF_INIT;
strbuf_reset(&prefix);
- return clear_ce_flags_1(cache, nr,
+ return clear_ce_flags_1(istate,
+ istate->cache,
+ istate->cache_nr,
&prefix,
select_mask, clear_mask,
el, 0);
@@ -1231,7 +1239,7 @@ static int clear_ce_flags(struct cache_entry **cache, int nr,
* Set/Clear CE_NEW_SKIP_WORKTREE according to $GIT_DIR/info/sparse-checkout
*/
static void mark_new_skip_worktree(struct exclude_list *el,
- struct index_state *the_index,
+ struct index_state *istate,
int select_flag, int skip_wt_flag)
{
int i;
@@ -1240,8 +1248,8 @@ static void mark_new_skip_worktree(struct exclude_list *el,
* 1. Pretend the narrowest worktree: only unmerged entries
* are checked out
*/
- for (i = 0; i < the_index->cache_nr; i++) {
- struct cache_entry *ce = the_index->cache[i];
+ for (i = 0; i < istate->cache_nr; i++) {
+ struct cache_entry *ce = istate->cache[i];
if (select_flag && !(ce->ce_flags & select_flag))
continue;
@@ -1256,8 +1264,7 @@ static void mark_new_skip_worktree(struct exclude_list *el,
* 2. Widen worktree according to sparse-checkout file.
* Matched entries will have skip_wt_flag cleared (i.e. "in")
*/
- clear_ce_flags(the_index->cache, the_index->cache_nr,
- select_flag, skip_wt_flag, el);
+ clear_ce_flags(istate, select_flag, skip_wt_flag, el);
}
static int verify_absent(const struct cache_entry *,
@@ -1636,7 +1643,7 @@ static int verify_clean_subdirectory(const struct cache_entry *ce,
memset(&d, 0, sizeof(d));
if (o->dir)
d.exclude_per_dir = o->dir->exclude_per_dir;
- i = read_directory(&d, &the_index, pathbuf, namelen+1, NULL);
+ i = read_directory(&d, o->src_index, pathbuf, namelen+1, NULL);
if (i)
return o->gently ? -1 :
add_rejected_path(o, ERROR_NOT_UPTODATE_DIR, ce->name);
@@ -1678,7 +1685,7 @@ static int check_ok_to_remove(const char *name, int len, int dtype,
return 0;
if (o->dir &&
- is_excluded(o->dir, &the_index, name, &dtype))
+ is_excluded(o->dir, o->src_index, name, &dtype))
/*
* ce->name is explicitly excluded, so it is Ok to
* overwrite it.
--
2.18.0.rc0.309.g77c7720784
next reply other threads:[~2018-06-01 16:12 UTC|newest]
Thread overview: 92+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-06-01 16:11 Nguyễn Thái Ngọc Duy [this message]
2018-06-01 18:34 ` [PATCH/RFC/BUG] unpack-trees.c: do not use "the_index" Elijah Newren
2018-06-01 18:51 ` Stefan Beller
2018-06-02 5:01 ` Duy Nguyen
2018-06-02 5:03 ` Duy Nguyen
2018-06-03 4:58 ` Elijah Newren
2018-06-04 17:33 ` Brandon Williams
2018-06-05 15:43 ` [PATCH 0/6] Fix "the_index" usage in unpack-trees.c Nguyễn Thái Ngọc Duy
2018-06-05 15:43 ` [PATCH 1/6] unpack-trees: remove 'extern' on function declaration Nguyễn Thái Ngọc Duy
2018-06-05 15:43 ` [PATCH 2/6] unpack-trees: add a note about path invalidation Nguyễn Thái Ngọc Duy
2018-06-05 15:43 ` [PATCH 3/6] unpack-trees: don't shadow global var the_index Nguyễn Thái Ngọc Duy
2018-06-05 17:11 ` Ramsay Jones
2018-06-05 15:43 ` [PATCH 4/6] unpack-tress: convert clear_ce_flags* to avoid the_index Nguyễn Thái Ngọc Duy
2018-06-05 17:37 ` Ramsay Jones
2018-06-05 15:43 ` [PATCH 5/6] unpack-trees: avoid the_index in verify_absent() Nguyễn Thái Ngọc Duy
2018-06-05 15:43 ` [PATCH 6/6] Forbid "the_index" in dir.c and unpack-trees.c Nguyễn Thái Ngọc Duy
2018-06-05 16:58 ` Brandon Williams
2018-06-06 4:57 ` Duy Nguyen
2018-06-06 5:02 ` [PATCH v2 0/5] Fix "the_index" usage in unpack-trees.c Nguyễn Thái Ngọc Duy
2018-06-06 5:02 ` [PATCH v2 1/5] unpack-trees: remove 'extern' on function declaration Nguyễn Thái Ngọc Duy
2018-06-06 5:02 ` [PATCH v2 2/5] unpack-trees: add a note about path invalidation Nguyễn Thái Ngọc Duy
2018-06-06 5:02 ` [PATCH v2 3/5] unpack-trees: don't shadow global var the_index Nguyễn Thái Ngọc Duy
2018-06-06 5:02 ` [PATCH v2 4/5] unpack-tress: convert clear_ce_flags* to avoid the_index Nguyễn Thái Ngọc Duy
2018-06-06 5:02 ` [PATCH v2 5/5] unpack-trees: avoid the_index in verify_absent() Nguyễn Thái Ngọc Duy
2018-06-06 7:39 ` [PATCH v3 00/20] Fix incorrect use of the_index Nguyễn Thái Ngọc Duy
2018-06-06 7:39 ` [PATCH v3 01/20] unpack-trees: remove 'extern' on function declaration Nguyễn Thái Ngọc Duy
2018-06-06 7:39 ` [PATCH v3 02/20] unpack-trees: add a note about path invalidation Nguyễn Thái Ngọc Duy
2018-06-06 7:39 ` [PATCH v3 03/20] unpack-trees: don't shadow global var the_index Nguyễn Thái Ngọc Duy
2018-06-06 7:39 ` [PATCH v3 04/20] unpack-tress: convert clear_ce_flags* to avoid the_index Nguyễn Thái Ngọc Duy
2018-06-06 7:39 ` [PATCH v3 05/20] unpack-trees: avoid the_index in verify_absent() Nguyễn Thái Ngọc Duy
2018-06-06 7:39 ` [PATCH v3 06/20] attr.h: drop extern from function declaration Nguyễn Thái Ngọc Duy
2018-06-06 7:39 ` [PATCH v3 07/20] attr: remove an implicit dependency on the_index Nguyễn Thái Ngọc Duy
2018-06-06 13:35 ` Ramsay Jones
2018-06-06 16:50 ` Brandon Williams
2018-06-06 16:58 ` Duy Nguyen
2018-06-06 17:02 ` Brandon Williams
2018-06-06 7:39 ` [PATCH v3 08/20] convert.h: drop 'extern' from function declaration Nguyễn Thái Ngọc Duy
2018-06-06 7:39 ` [PATCH v3 09/20] convert.c: remove an implicit dependency on the_index Nguyễn Thái Ngọc Duy
2018-06-06 7:39 ` [PATCH v3 10/20] dir.c: remove an implicit dependency on the_index in pathspec code Nguyễn Thái Ngọc Duy
2018-06-06 7:39 ` [PATCH v3 11/20] ls-files: correct index argument to get_convert_attr_ascii() Nguyễn Thái Ngọc Duy
2018-06-06 7:39 ` [PATCH v3 12/20] pathspec.c: use the right index instead of the_index Nguyễn Thái Ngọc Duy
2018-06-06 7:39 ` [PATCH v3 13/20] submodule.c: " Nguyễn Thái Ngọc Duy
2018-06-06 7:39 ` [PATCH v3 14/20] entry.c: " Nguyễn Thái Ngọc Duy
2018-06-06 7:39 ` [PATCH v3 15/20] attr: remove index from git_attr_set_direction() Nguyễn Thái Ngọc Duy
2018-06-06 16:58 ` Brandon Williams
2018-06-06 7:39 ` [PATCH v3 16/20] preload-index.c: use the right index instead of the_index Nguyễn Thái Ngọc Duy
2018-06-06 7:39 ` [PATCH v3 17/20] cache.c: remove an implicit dependency on the_index Nguyễn Thái Ngọc Duy
2018-06-06 17:00 ` Brandon Williams
2018-06-06 7:39 ` [PATCH v3 18/20] resolve-undo.c: use the right index instead of the_index Nguyễn Thái Ngọc Duy
2018-06-06 7:39 ` [PATCH v3 19/20] grep: " Nguyễn Thái Ngọc Duy
2018-06-06 7:39 ` [PATCH v3 20/20] cache.h: make the_index part of "compatibility macros" Nguyễn Thái Ngọc Duy
2018-06-06 16:49 ` [PATCH v4 00/23] Fix incorrect use of the_index Nguyễn Thái Ngọc Duy
2018-06-06 16:49 ` [PATCH v4 01/23] unpack-trees: remove 'extern' on function declaration Nguyễn Thái Ngọc Duy
2018-06-06 16:49 ` [PATCH v4 02/23] unpack-trees: add a note about path invalidation Nguyễn Thái Ngọc Duy
2018-06-06 16:49 ` [PATCH v4 03/23] unpack-trees: don't shadow global var the_index Nguyễn Thái Ngọc Duy
2018-06-06 16:49 ` [PATCH v4 04/23] unpack-tress: convert clear_ce_flags* to avoid the_index Nguyễn Thái Ngọc Duy
2018-06-07 7:41 ` Elijah Newren
2018-06-07 15:11 ` Duy Nguyen
2018-06-08 15:58 ` Duy Nguyen
2018-06-06 16:49 ` [PATCH v4 05/23] unpack-trees: avoid the_index in verify_absent() Nguyễn Thái Ngọc Duy
2018-06-06 17:02 ` [PATCH v4 06/23] attr.h: drop extern from function declaration Nguyễn Thái Ngọc Duy
2018-06-06 17:02 ` [PATCH v4 07/23] attr: remove an implicit dependency on the_index Nguyễn Thái Ngọc Duy
2018-06-06 17:02 ` [PATCH v4 08/23] convert.h: drop 'extern' from function declaration Nguyễn Thái Ngọc Duy
2018-06-06 17:02 ` [PATCH v4 09/23] convert.c: remove an implicit dependency on the_index Nguyễn Thái Ngọc Duy
2018-06-06 17:02 ` [PATCH v4 10/23] dir.c: remove an implicit dependency on the_index in pathspec code Nguyễn Thái Ngọc Duy
2018-06-06 17:02 ` [PATCH v4 11/23] ls-files: correct index argument to get_convert_attr_ascii() Nguyễn Thái Ngọc Duy
2018-06-06 17:02 ` [PATCH v4 12/23] pathspec.c: use the right index instead of the_index Nguyễn Thái Ngọc Duy
2018-06-06 17:02 ` [PATCH v4 13/23] submodule.c: " Nguyễn Thái Ngọc Duy
2018-06-06 17:02 ` [PATCH v4 14/23] entry.c: " Nguyễn Thái Ngọc Duy
2018-06-06 17:02 ` [PATCH v4 15/23] attr: remove index from git_attr_set_direction() Nguyễn Thái Ngọc Duy
2018-06-09 17:57 ` Elijah Newren
2018-06-06 17:02 ` [PATCH v4 16/23] preload-index.c: use the right index instead of the_index Nguyễn Thái Ngọc Duy
2018-06-06 17:02 ` [PATCH v4 17/23] read-cache.c: remove an implicit dependency on the_index Nguyễn Thái Ngọc Duy
2018-06-09 18:10 ` Elijah Newren
2018-06-09 18:45 ` Duy Nguyen
2018-06-09 19:48 ` Elijah Newren
2018-06-06 17:02 ` [PATCH v4 18/23] apply.c: use the right index instead of the_index Nguyễn Thái Ngọc Duy
2018-06-06 17:02 ` [PATCH v4 19/23] difftool: " Nguyễn Thái Ngọc Duy
2018-06-06 17:02 ` [PATCH v4 20/23] checkout: avoid the_index when possible Nguyễn Thái Ngọc Duy
2018-06-06 17:02 ` [PATCH v4 21/23] resolve-undo.c: use the right index instead of the_index Nguyễn Thái Ngọc Duy
2018-06-06 17:02 ` [PATCH v4 22/23] grep: " Nguyễn Thái Ngọc Duy
2018-06-06 17:02 ` [PATCH v4 23/23] cache.h: make the_index part of "compatibility macros" Nguyễn Thái Ngọc Duy
2018-06-07 7:44 ` [PATCH v4 00/23] Fix incorrect use of the_index Elijah Newren
2018-06-09 19:58 ` Elijah Newren
2018-06-11 16:05 ` Duy Nguyen
2018-06-11 16:11 ` Elijah Newren
2018-06-11 16:24 ` Duy Nguyen
2018-06-11 16:44 ` Elijah Newren
2018-06-11 16:49 ` Duy Nguyen
2018-06-14 17:18 ` Duy Nguyen
2018-06-14 20:57 ` Elijah Newren
2018-06-11 18:20 ` Duy Nguyen
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=20180601161153.15192-1-pclouds@gmail.com \
--to=pclouds@gmail.com \
--cc=git@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).