git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH/RFC/BUG] unpack-trees.c: do not use "the_index"
@ 2018-06-01 16:11 Nguyễn Thái Ngọc Duy
  2018-06-01 18:34 ` Elijah Newren
  2018-06-05 15:43 ` [PATCH 0/6] Fix "the_index" usage in unpack-trees.c Nguyễn Thái Ngọc Duy
  0 siblings, 2 replies; 92+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-06-01 16:11 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

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


^ permalink raw reply related	[flat|nested] 92+ messages in thread

end of thread, other threads:[~2018-06-14 20:57 UTC | newest]

Thread overview: 92+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-01 16:11 [PATCH/RFC/BUG] unpack-trees.c: do not use "the_index" Nguyễn Thái Ngọc Duy
2018-06-01 18:34 ` 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

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).