{"thread":{"id":"59964","subject":"[PATCH 0/2] remove unused \"base-offset\" from tree-walk codepath","startedAt":"2023-07-07T22:21:25Z","lastAt":"2023-07-07T22:21:30Z","messageCount":3,"participants":["Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"479300","messageId":"20230707222116.4129415-1-gitster@pobox.com","threadId":"59964","inReplyTo":null,"subject":"[PATCH 0/2] remove unused \"base-offset\" from tree-walk codepath","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-07-07T22:21:14Z","receivedAt":"2023-07-07T22:21:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Here are a few bonus patches I came up with while working on the\n\"hmm, attr magic does not seem to work with pathspec patterns?\"\ntopic to remove a parameter that is passed around without getting\nany value other than 0 (hence nobody knows if the code is correct\nwhen the parameter is set to non-zero).\n\nIt turned out that they do not interact with the main topic in any\nway, so I made it into a separate and indenendent series.\n\nJunio C Hamano (2):\n  tree-walk: lose base_offset that is never used in\n    tree_entry_interesting\n  tree-walk: drop unused base_offset from do_match()\n\n builtin/grep.c |  2 +-\n list-objects.c |  2 +-\n tree-diff.c    |  2 +-\n tree-walk.c    | 36 ++++++++++++++++++------------------\n tree-walk.h    |  2 +-\n tree.c         |  2 +-\n 6 files changed, 23 insertions(+), 23 deletions(-)\n\n-- \n2.41.0-318-g061c58647e\n\n"},{"id":"479301","messageId":"20230707222116.4129415-2-gitster@pobox.com","threadId":"59964","inReplyTo":"20230707222116.4129415-1-gitster@pobox.com","subject":"[PATCH 1/2] tree-walk: lose base_offset that is never used in tree_entry_interesting","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-07-07T22:21:15Z","receivedAt":"2023-07-07T22:21:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The tree_entry_interesting() function takes base_offset, allowing\nits callers to potentially pass a non-zero number to skip the early\npart of the path string.\n\nThe feature is never exercised and we do not even know what bugs are\nlurking there, as all callers pass 0 to the parameter.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/grep.c | 2 +-\n list-objects.c | 2 +-\n tree-diff.c    | 2 +-\n tree-walk.c    | 5 +++--\n tree-walk.h    | 2 +-\n tree.c         | 2 +-\n 6 files changed, 8 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 22645c6244..5f47317c2f 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -644,7 +644,7 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,\n \t\t\tstrbuf_addstr(&name, base->buf + tn_len);\n \t\t\tmatch = tree_entry_interesting(repo->index,\n \t\t\t\t\t\t       &entry, &name,\n-\t\t\t\t\t\t       0, pathspec);\n+\t\t\t\t\t\t       pathspec);\n \t\t\tstrbuf_setlen(&name, name_base_len);\n \n \t\t\tif (match == all_entries_not_interesting)\ndiff --git a/list-objects.c b/list-objects.c\nindex 672a4cd529..e60a6cd5b4 100644\n--- a/list-objects.c\n+++ b/list-objects.c\n@@ -102,7 +102,7 @@ static void process_tree_contents(struct traversal_context *ctx,\n \twhile (tree_entry(&desc, &entry)) {\n \t\tif (match != all_entries_interesting) {\n \t\t\tmatch = tree_entry_interesting(ctx->revs->repo->index,\n-\t\t\t\t\t\t       &entry, base, 0,\n+\t\t\t\t\t\t       &entry, base,\n \t\t\t\t\t\t       &ctx->revs->diffopt.pathspec);\n \t\t\tif (match == all_entries_not_interesting)\n \t\t\t\tbreak;\ndiff --git a/tree-diff.c b/tree-diff.c\nindex 966946848a..8fc159b86e 100644\n--- a/tree-diff.c\n+++ b/tree-diff.c\n@@ -317,7 +317,7 @@ static void skip_uninteresting(struct tree_desc *t, struct strbuf *base,\n \n \twhile (t->size) {\n \t\tmatch = tree_entry_interesting(opt->repo->index, &t->entry,\n-\t\t\t\t\t       base, 0, &opt->pathspec);\n+\t\t\t\t\t       base, &opt->pathspec);\n \t\tif (match) {\n \t\t\tif (match == all_entries_not_interesting)\n \t\t\t\tt->size = 0;\ndiff --git a/tree-walk.c b/tree-walk.c\nindex 42ed86ef58..1797f2b9c5 100644\n--- a/tree-walk.c\n+++ b/tree-walk.c\n@@ -435,7 +435,7 @@ static inline int prune_traversal(struct index_state *istate,\n \tif (still_interesting < 0)\n \t\treturn still_interesting;\n \treturn tree_entry_interesting(istate, e, base,\n-\t\t\t\t      0, info->pathspec);\n+\t\t\t\t      info->pathspec);\n }\n \n int traverse_trees(struct index_state *istate,\n@@ -1223,10 +1223,11 @@ static enum interesting do_match(struct index_state *istate,\n  */\n enum interesting tree_entry_interesting(struct index_state *istate,\n \t\t\t\t\tconst struct name_entry *entry,\n-\t\t\t\t\tstruct strbuf *base, int base_offset,\n+\t\t\t\t\tstruct strbuf *base,\n \t\t\t\t\tconst struct pathspec *ps)\n {\n \tenum interesting positive, negative;\n+\tconst int base_offset = 0;\n \tpositive = do_match(istate, entry, base, base_offset, ps, 0);\n \n \t/*\ndiff --git a/tree-walk.h b/tree-walk.h\nindex 01a9d8eb44..74cdceb3fe 100644\n--- a/tree-walk.h\n+++ b/tree-walk.h\n@@ -224,7 +224,7 @@ enum interesting {\n \n enum interesting tree_entry_interesting(struct index_state *istate,\n \t\t\t\t\tconst struct name_entry *,\n-\t\t\t\t\tstruct strbuf *, int,\n+\t\t\t\t\tstruct strbuf *,\n \t\t\t\t\tconst struct pathspec *ps);\n \n #endif\ndiff --git a/tree.c b/tree.c\nindex e118914a76..c745462f96 100644\n--- a/tree.c\n+++ b/tree.c\n@@ -32,7 +32,7 @@ int read_tree_at(struct repository *r,\n \twhile (tree_entry(&desc, &entry)) {\n \t\tif (retval != all_entries_interesting) {\n \t\t\tretval = tree_entry_interesting(r->index, &entry,\n-\t\t\t\t\t\t\tbase, 0, pathspec);\n+\t\t\t\t\t\t\tbase, pathspec);\n \t\t\tif (retval == all_entries_not_interesting)\n \t\t\t\tbreak;\n \t\t\tif (retval == entry_not_interesting)\n-- \n2.41.0-318-g061c58647e\n\n"},{"id":"479302","messageId":"20230707222116.4129415-3-gitster@pobox.com","threadId":"59964","inReplyTo":"20230707222116.4129415-1-gitster@pobox.com","subject":"[PATCH 2/2] tree-walk: drop unused base_offset from do_match()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-07-07T22:21:16Z","receivedAt":"2023-07-07T22:21:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The tree-walk.c:do_match() function takes base_offset but just like\ntree_entry_interesting() we dealt with earlier, nobody passes a\nvalue other than 0 in it.  Get rid of the parameter to avoid having\nto worry about potential bugs lurking unexercised.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n tree-walk.c | 33 ++++++++++++++++-----------------\n 1 file changed, 16 insertions(+), 17 deletions(-)\n\ndiff --git a/tree-walk.c b/tree-walk.c\nindex 1797f2b9c5..f48e8c580e 100644\n--- a/tree-walk.c\n+++ b/tree-walk.c\n@@ -1016,17 +1016,17 @@ static int match_wildcard_base(const struct pathspec_item *item,\n /*\n  * Is a tree entry interesting given the pathspec we have?\n  *\n- * Pre-condition: either baselen == base_offset (i.e. empty path)\n+ * Pre-condition: either baselen == 0 (i.e. empty path)\n  * or base[baselen-1] == '/' (i.e. with trailing slash).\n  */\n static enum interesting do_match(struct index_state *istate,\n \t\t\t\t const struct name_entry *entry,\n-\t\t\t\t struct strbuf *base, int base_offset,\n+\t\t\t\t struct strbuf *base,\n \t\t\t\t const struct pathspec *ps,\n \t\t\t\t int exclude)\n {\n \tint i;\n-\tint pathlen, baselen = base->len - base_offset;\n+\tint pathlen, baselen = base->len;\n \tenum interesting never_interesting = ps->has_wildcard ?\n \t\tentry_not_interesting : all_entries_not_interesting;\n \n@@ -1044,7 +1044,7 @@ static enum interesting do_match(struct index_state *istate,\n \t\t    !(ps->magic & PATHSPEC_MAXDEPTH) ||\n \t\t    ps->max_depth == -1)\n \t\t\treturn all_entries_interesting;\n-\t\treturn within_depth(base->buf + base_offset, baselen,\n+\t\treturn within_depth(base->buf, baselen,\n \t\t\t\t    !!S_ISDIR(entry->mode),\n \t\t\t\t    ps->max_depth) ?\n \t\t\tentry_interesting : entry_not_interesting;\n@@ -1055,7 +1055,7 @@ static enum interesting do_match(struct index_state *istate,\n \tfor (i = ps->nr - 1; i >= 0; i--) {\n \t\tconst struct pathspec_item *item = ps->items+i;\n \t\tconst char *match = item->match;\n-\t\tconst char *base_str = base->buf + base_offset;\n+\t\tconst char *base_str = base->buf;\n \t\tint matchlen = item->len, matched = 0;\n \n \t\tif ((!exclude &&   item->magic & PATHSPEC_EXCLUDE) ||\n@@ -1148,9 +1148,9 @@ static enum interesting do_match(struct index_state *istate,\n \n \t\tstrbuf_add(base, entry->path, pathlen);\n \n-\t\tif (!git_fnmatch(item, match, base->buf + base_offset,\n+\t\tif (!git_fnmatch(item, match, base->buf,\n \t\t\t\t item->nowildcard_len)) {\n-\t\t\tstrbuf_setlen(base, base_offset + baselen);\n+\t\t\tstrbuf_setlen(base, baselen);\n \t\t\tgoto interesting;\n \t\t}\n \n@@ -1162,13 +1162,13 @@ static enum interesting do_match(struct index_state *istate,\n \t\t * be performed in the submodule itself.\n \t\t */\n \t\tif (ps->recurse_submodules && S_ISGITLINK(entry->mode) &&\n-\t\t    !ps_strncmp(item, match, base->buf + base_offset,\n+\t\t    !ps_strncmp(item, match, base->buf,\n \t\t\t\titem->nowildcard_len)) {\n-\t\t\tstrbuf_setlen(base, base_offset + baselen);\n+\t\t\tstrbuf_setlen(base, baselen);\n \t\t\tgoto interesting;\n \t\t}\n \n-\t\tstrbuf_setlen(base, base_offset + baselen);\n+\t\tstrbuf_setlen(base, baselen);\n \n \t\t/*\n \t\t * Match all directories. We'll try to match files\n@@ -1204,9 +1204,9 @@ static enum interesting do_match(struct index_state *istate,\n \t\t\t\treturn entry_interesting;\n \n \t\t\tstrbuf_add(base, entry->path, pathlen);\n-\t\t\tret = match_pathspec_attrs(istate, base->buf + base_offset,\n-\t\t\t\t\t\t   base->len - base_offset, item);\n-\t\t\tstrbuf_setlen(base, base_offset + baselen);\n+\t\t\tret = match_pathspec_attrs(istate, base->buf,\n+\t\t\t\t\t\t   base->len, item);\n+\t\t\tstrbuf_setlen(base, baselen);\n \t\t\tif (!ret)\n \t\t\t\tcontinue;\n \t\t}\n@@ -1218,7 +1218,7 @@ static enum interesting do_match(struct index_state *istate,\n /*\n  * Is a tree entry interesting given the pathspec we have?\n  *\n- * Pre-condition: either baselen == base_offset (i.e. empty path)\n+ * Pre-condition: either baselen == 0 (i.e. empty path)\n  * or base[baselen-1] == '/' (i.e. with trailing slash).\n  */\n enum interesting tree_entry_interesting(struct index_state *istate,\n@@ -1227,8 +1227,7 @@ enum interesting tree_entry_interesting(struct index_state *istate,\n \t\t\t\t\tconst struct pathspec *ps)\n {\n \tenum interesting positive, negative;\n-\tconst int base_offset = 0;\n-\tpositive = do_match(istate, entry, base, base_offset, ps, 0);\n+\tpositive = do_match(istate, entry, base, ps, 0);\n \n \t/*\n \t * case | entry | positive | negative | result\n@@ -1265,7 +1264,7 @@ enum interesting tree_entry_interesting(struct index_state *istate,\n \t    positive <= entry_not_interesting) /* #1, #2, #11, #12 */\n \t\treturn positive;\n \n-\tnegative = do_match(istate, entry, base, base_offset, ps, 1);\n+\tnegative = do_match(istate, entry, base, ps, 1);\n \n \t/* #8, #18 */\n \tif (positive == all_entries_interesting &&\n-- \n2.41.0-318-g061c58647e\n\n"}]}