{"thread":{"id":"66254","subject":"[PATCH] dir: find common prefix among positive pathspecs","startedAt":"2026-09-02T13:11:43Z","lastAt":"2026-09-16T16:07:30Z","messageCount":29,"participants":["Yannik Tausch","Junio C Hamano","Elijah Newren"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"551750","messageId":"AA085B7A-F528-458A-8AA9-7664480997AE@ytausch.de","threadId":"66254","inReplyTo":null,"subject":"[PATCH] dir: find common prefix among positive pathspecs","fromName":"Yannik Tausch","fromEmail":"dev@ytausch.de","sentAt":"2026-09-02T13:04:09Z","receivedAt":"2026-09-02T13:11:43Z","isPatch":true,"body":"common_prefix_len() skips exclude pathspec items, but uses n == 0 to\nidentify the initial item and items[0] as the comparison source. When\nan exclude item comes first, the function returns zero even when all\npositive pathspecs share a directory.\n\nTrack the first positive item explicitly. Return its match and the\ncommon prefix length together so that common_prefix() and\nfill_directory() use the correct string. Add a unit test with an\nunrelated exclude before two positive pathspecs that share a directory.\n\nSigned-off-by: Yannik Tausch \n---\n\nThis patch is based on https://lore.kernel.org/git/0CA8678D-0540-4A2E-B314-B9BEB04E2BF5@ytausch.de/T/#u.\n\n dir.c | 51 +++++++++++++++++++++++++++-----------------\n t/unit-tests/u-dir.c | 28 ++++++++++++++++++++++++\n 2 files changed, 60 insertions(+), 19 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 7072715389..441c1795a1 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -212,9 +212,19 @@ static int fnmatch_icase_mem(const char *pattern, int patternlen,\n \treturn match_status;\n }\n \n-static size_t common_prefix_len(const struct pathspec *pathspec)\n+struct pathspec_prefix {\n+\tconst char *match;\n+\tsize_t len;\n+};\n+\n+/*\n+ * Find the common prefix of positive pathspec items. The returned match\n+ * points into the first positive item and is not NUL-terminated at len.\n+ */\n+static struct pathspec_prefix find_common_prefix(const struct pathspec *pathspec)\n {\n-\tint n;\n+\tstruct pathspec_prefix prefix = { 0 };\n+\tint n, first = -1;\n \tsize_t max = 0;\n \n \t/*\n@@ -237,44 +247,47 @@ static size_t common_prefix_len(const struct pathspec *pathspec)\n \t\tsize_t i = 0, len = 0, item_len;\n \t\tif (pathspec->items[n].magic & PATHSPEC_EXCLUDE)\n \t\t\tcontinue;\n+\t\tif (first < 0)\n+\t\t\tfirst = n;\n \t\tif (pathspec->items[n].magic & PATHSPEC_ICASE)\n \t\t\titem_len = pathspec->items[n].prefix;\n \t\telse\n \t\t\titem_len = pathspec->items[n].nowildcard_len;\n-\t\twhile (i < item_len && (n == 0 || i < max)) {\n+\t\twhile (i < item_len && (n == first || i < max)) {\n \t\t\tchar c = pathspec->items[n].match[i];\n-\t\t\tif (c != pathspec->items[0].match[i])\n+\t\t\tif (c != pathspec->items[first].match[i])\n \t\t\t\tbreak;\n \t\t\tif (c == '/')\n \t\t\t\tlen = i + 1;\n \t\t\ti++;\n \t\t}\n-\t\tif (n == 0 || len < max) {\n+\t\tif (n == first || len < max) {\n \t\t\tmax = len;\n \t\t\tif (!max)\n \t\t\t\tbreak;\n \t\t}\n \t}\n-\treturn max;\n+\tprefix.match = first < 0 ? NULL : pathspec->items[first].match;\n+\tprefix.len = max;\n+\treturn prefix;\n }\n \n /*\n- * Returns a copy of the longest leading path common among all\n+ * Returns a copy of the longest leading path common among all positive\n * pathspecs.\n */\n char *common_prefix(const struct pathspec *pathspec)\n {\n-\tunsigned long len = common_prefix_len(pathspec);\n+\tstruct pathspec_prefix prefix = find_common_prefix(pathspec);\n \n-\treturn len ? xmemdupz(pathspec->items[0].match, len) : NULL;\n+\treturn prefix.len ? xmemdupz(prefix.match, prefix.len) : NULL;\n }\n \n int fill_directory(struct dir_struct *dir,\n \t\t struct index_state *istate,\n \t\t const struct pathspec *pathspec)\n {\n-\tconst char *prefix;\n-\tsize_t prefix_len;\n+\tstruct pathspec_prefix prefix;\n \n \tunsigned exclusive_flags = DIR_SHOW_IGNORED | DIR_SHOW_IGNORED_TOO;\n \tif ((dir->flags & exclusive_flags) == exclusive_flags)\n@@ -284,13 +297,13 @@ int fill_directory(struct dir_struct *dir,\n \t * Calculate common prefix for the pathspec, and\n \t * use that to optimize the directory walk\n \t */\n-\tprefix_len = common_prefix_len(pathspec);\n-\tprefix = prefix_len ? pathspec->items[0].match : \"\";\n+\tprefix = find_common_prefix(pathspec);\n \n \t/* Read the directory and prune it */\n-\tread_directory(dir, istate, prefix, prefix_len, pathspec);\n+\tread_directory(dir, istate, prefix.len ? prefix.match : \"\",\n+\t\t prefix.len, pathspec);\n \n-\treturn prefix_len;\n+\treturn prefix.len;\n }\n \n int within_depth(const char *name, int namelen,\n@@ -394,7 +407,7 @@ static int match_pathspec_item(struct index_state *istate,\n \n \t/*\n \t * The normal call pattern is:\n-\t * 1. prefix = common_prefix_len(ps);\n+\t * 1. prefix = find_common_prefix(ps).len;\n \t * 2. prune something, or fill_directory\n \t * 3. match_pathspec()\n \t *\n@@ -411,11 +424,11 @@ static int match_pathspec_item(struct index_state *istate,\n \t * prefix part when :(icase) is involved. We do exact\n \t * comparison ourselves.\n \t *\n-\t * Normally the caller (common_prefix_len() in fact) does\n+\t * Normally the caller (find_common_prefix() in fact) does\n \t * _exact_ matching on name[-prefix+1..-1] and we do not need\n \t * to check that part. Be defensive and check it anyway, in\n-\t * case common_prefix_len is changed, or a new caller is\n-\t * introduced that does not use common_prefix_len.\n+\t * case find_common_prefix() is changed, or a new caller is\n+\t * introduced that does not use find_common_prefix().\n \t *\n \t * If the penalty turns out too high when prefix is really\n \t * long, maybe change it to\ndiff --git a/t/unit-tests/u-dir.c b/t/unit-tests/u-dir.c\nindex 2d0adaa39e..8b558e0391 100644\n--- a/t/unit-tests/u-dir.c\n+++ b/t/unit-tests/u-dir.c\n@@ -45,3 +45,31 @@ void test_dir__within_depth(void)\n \n \n }\n+\n+void test_dir__common_prefix_skips_excluded_pathspecs(void)\n+{\n+\tstruct pathspec_item items[] = {\n+\t\t{\n+\t\t\t.match = \"unrelated/path\",\n+\t\t\t.magic = PATHSPEC_EXCLUDE,\n+\t\t\t.nowildcard_len = 14,\n+\t\t},\n+\t\t{\n+\t\t\t.match = \"foo/bar\",\n+\t\t\t.nowildcard_len = 7,\n+\t\t},\n+\t\t{\n+\t\t\t.match = \"foo/baz\",\n+\t\t\t.nowildcard_len = 7,\n+\t\t},\n+\t};\n+\tstruct pathspec pathspec = {\n+\t\t.nr = ARRAY_SIZE(items),\n+\t\t.magic = PATHSPEC_EXCLUDE,\n+\t\t.items = items,\n+\t};\n+\tchar *prefix = common_prefix(&pathspec);\n+\n+\tcl_assert_equal_s(prefix, \"foo/\");\n+\tfree(prefix);\n+}\n\nbase-commit: 1630431f326e15fcde608827b5ff38422528eb59\nprerequisite-patch-id: 256750f07ff447732869d1aadde2f1050e7bb169\n-- \n2.55.0"},{"id":"551790","messageId":"xmqqecfbk2eb.fsf@gitster.g","threadId":"66254","inReplyTo":"AA085B7A-F528-458A-8AA9-7664480997AE@ytausch.de","subject":"Re: [PATCH] dir: find common prefix among positive pathspecs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-09-02T17:07:40Z","receivedAt":"2026-09-02T17:07:43Z","isPatch":true,"body":"Yannik Tausch writes:\n\n> common_prefix_len() skips exclude pathspec items, but uses n == 0 to\n> identify the initial item and items[0] as the comparison source. When\n> an exclude item comes first, the function returns zero even when all\n> positive pathspecs share a directory.\n>\n> Track the first positive item explicitly. Return its match and the\n> common prefix length together so that common_prefix() and\n> fill_directory() use the correct string. Add a unit test with an\n> unrelated exclude before two positive pathspecs that share a directory.\n>\n> Signed-off-by: Yannik Tausch \n> ---\n>\n> This patch is based on https://lore.kernel.org/git/0CA8678D-0540-4A2E-B314-B9BEB04E2BF5@ytausch.de/T/#u.\n\nI am not sure what you mean. Do you mean that the other one should\nhave been marked as [PATCH 1/2] and this one [PATCH 2/2]? The way\nwe use the phrase \"based on\" does not exactly match that situation.\nIt is more like \"This patch applies on top of the other one\", or\n\"This patch depends on the other one.\"\n\n> -static size_t common_prefix_len(const struct pathspec *pathspec)\n> +struct pathspec_prefix {\n> +\tconst char *match;\n> +\tsize_t len;\n> +};\n> +\n> +/*\n> + * Find the common prefix of positive pathspec items. The returned match\n> + * points into the first positive item and is not NUL-terminated at len.\n> + */\n> +static struct pathspec_prefix find_common_prefix(const struct pathspec *pathspec)\n\nOur norm in C is not to pass structures by value either as parameter\nof as return value, unless there is a very good reason to do so.\n\nSince we can easily use\n\n\tconst char *common_prefix(const sturct pathspec *pathspec, size_t *len);\n\nto return .match and store the length in *len when we return, we\ncannot say that this case has a very good reason to use a structure\npassed by value.\n\nActually, I have a feeling that we do not want find_common_prefix()\nhelper. Instead perhaps\n\n static size_t common_prefix_len(const struct pathspec *pathspec,\n\t\t\t\t const char **matched_prefix)\n\nmay be an alternative that is easier to work with. Because the\nexisting callers assume that pathspec->items[0].match is where they\ncan grab the common prefix from, they should look like\n\n\tlen = common_prefix_len(pathspec);\n\t... use the first len bytes of pathspec->items[0].match[] ...\n\nThey want to be told to do this instead now:\n\n\tconst char *common_prefix;\n\n\tlen = common_prefix_len(pathspec, &common_prefix);\n\t... use the first len bytes of common_prefix[] ...\n\nIn \"use the first len bytes\" logic they already have, they know not\nto memdup when len == 0 (and ignore pathspec->items[0].match[] in\nthat case), and they know they need to memdup if they want to have\ntheir own copies, etc., so the changes to them can be kept to the\nminimum.\n\n> +\tprefix.match = first < 0 ? NULL : pathspec->items[first].match;\n> +\tprefix.len = max;\n> +\treturn prefix;\n\nSo instead of these three lines, your return sequence would become\n\n\t*matched_prefix = first < 0 ? NULL : pathspec->items[first].match;\n\treturn max;\n\nIf there is no positive element in the given pathspec (by the way,\n\"pathspec\" refers to the whole set, and each element in it may be\neither positive or negative, so \"positive pathspec(s)\" is a\nmisnomer), the loop never touches first or max, so when the loop\nexits, we won't have \"match\" and \"len\" is 0. Your changes in the\nloop to avoid assuming [0] is positive element all look correct.\n\n"},{"id":"551847","messageId":"81EC0E28-13E7-4D10-BD07-3601124CBD77@ytausch.de","threadId":"66254","inReplyTo":"xmqqecfbk2eb.fsf@gitster.g","subject":"Re: [PATCH] dir: find common prefix among positive pathspecs","fromName":"Yannik Tausch","fromEmail":"dev@ytausch.de","sentAt":"2026-09-03T09:59:12Z","receivedAt":"2026-09-03T09:59:28Z","isPatch":true,"body":"Hi,\n\n> Junio C Hamano writes:\n\n> I am not sure what you mean. Do you mean that the other one should\n> have been marked as [PATCH 1/2] and this one [PATCH 2/2]? The way\n> we use the phrase \"based on\" does not exactly match that situation.\n> It is more like \"This patch applies on top of the other one\", or\n> \"This patch depends on the other one.\"\n\nI wanted to indicate that this patch depends on the other one, but they can reviewed\nindependently. This is because the other patch eliminates a bug that leads to wrong \ninput data for the code segments I change in this one.\n\nRe-reading your contribution docs, I understand that this might indeed be better\nsubmitted as a patch series. I will resubmit as patch series v2.\n\n>> -static size_t common_prefix_len(const struct pathspec *pathspec)\n>> +struct pathspec_prefix {\n>> + const char *match;\n>> + size_t len;\n>> +};\n>> +\n>> +/*\n>> + * Find the common prefix of positive pathspec items. The returned match\n>> + * points into the first positive item and is not NUL-terminated at len.\n>> + */\n>> +static struct pathspec_prefix find_common_prefix(const struct pathspec *pathspec)\n> \n> Our norm in C is not to pass structures by value either as parameter\n> of as return value, unless there is a very good reason to do so.\n> \n> Since we can easily use\n> \n> const char *common_prefix(const sturct pathspec *pathspec, size_t *len);\n> \n> to return .match and store the length in *len when we return, we\n> cannot say that this case has a very good reason to use a structure\n> passed by value.\n\nFair if that’s your convention, note that in other languages I usually write, - I’m probably\ntelling you nothing new - we usually prefer clear separation of input and output values,\nwhich is, IMO, cleaner when returning a struct and makes this version more readable.\n\n> Actually, I have a feeling that we do not want find_common_prefix()\n> helper. Instead perhaps\n> \n> static size_t common_prefix_len(const struct pathspec *pathspec,\n> const char **matched_prefix)\n> \n> may be an alternative that is easier to work with. Because the\n> existing callers assume that pathspec->items[0].match is where they\n> can grab the common prefix from, they should look like\n> \n> len = common_prefix_len(pathspec);\n> ... use the first len bytes of pathspec->items[0].match[] ...\n> \n> They want to be told to do this instead now:\n> \n> const char *common_prefix;\n> \n> len = common_prefix_len(pathspec, &common_prefix);\n> ... use the first len bytes of common_prefix[] ...\n> \n> In \"use the first len bytes\" logic they already have, they know not\n> to memdup when len == 0 (and ignore pathspec->items[0].match[] in\n> that case), and they know they need to memdup if they want to have\n> their own copies, etc., so the changes to them can be kept to the\n> minimum.\n> \n>> + prefix.match = first < 0 ? NULL : pathspec->items[first].match;\n>> + prefix.len = max;\n>> + return prefix;\n> \n> So instead of these three lines, your return sequence would become\n> \n> *matched_prefix = first < 0 ? NULL : pathspec->items[first].match;\n> return max;\n> \n> If there is no positive element in the given pathspec (by the way,\n> \"pathspec\" refers to the whole set, and each element in it may be\n> either positive or negative, so \"positive pathspec(s)\" is a\n> misnomer), the loop never touches first or max, so when the loop\n> exits, we won't have \"match\" and \"len\" is 0. Your changes in the\n> loop to avoid assuming [0] is positive element all look correct.\n\nI addressed all your comments and will follow up with v2.\n\nYannik\n\n"},{"id":"551848","messageId":"886A25E6-8854-4AF6-BF0B-CFB57B673026@ytausch.de","threadId":"66254","inReplyTo":"81EC0E28-13E7-4D10-BD07-3601124CBD77@ytausch.de","subject":"[PATCH v2 0/2] dir: fix pathspec prefixes with exclusions","fromName":"Yannik Tausch","fromEmail":"dev@ytausch.de","sentAt":"2026-09-03T10:02:30Z","receivedAt":"2026-09-03T10:02:48Z","isPatch":true,"body":"Pathspec prefix optimization must account for exclude items separately.\nThe prefix is derived from non-exclude items, so applying it while matching\nan exclude item can compare the wrong portions of the paths. Conversely, an\nexclude item at the start of the pathspec currently prevents finding a common\nprefix among the remaining items.\n\nThe first patch matches exclude items against the full pathname. The second\npatch finds the common prefix starting with the first non-exclude item and\nreturns both the prefix length and the string from which it was derived.\n\nChanges since v1:\n\n* Send the changes as a two-patch series in dependency order.\n* Return the matched prefix through an output parameter instead of returning\n a structure by value.\n* Use \"non-exclude pathspec item\" terminology and consistent variable names.\n\nYannik Tausch (2):\n dir: do not apply prefix to negative pathspecs\n dir: find common prefix among non-exclude pathspec items\n\n dir.c | 39 +++++++++++++++++++++----------------\n t/t6132-pathspec-exclude.sh | 9 +++++++++\n t/unit-tests/u-dir.c | 28 ++++++++++++++++++++++++++\n 3 files changed, 59 insertions(+), 17 deletions(-)\n\nRange-diff against v1:\n1: c8a2f1e22e = 1: c8a2f1e22e dir: do not apply prefix to negative pathspecs\n2: 5a179872c1 ! 2: d0e08fdb96 dir: find common prefix among positive pathspecs\n @@ Metadata\n Author: Yannik Tausch \n \n ## Commit message ##\n - dir: find common prefix among positive pathspecs\n + dir: find common prefix among non-exclude pathspec items\n \n common_prefix_len() skips exclude pathspec items, but uses n == 0 to\n identify the initial item and items[0] as the comparison source. When\n an exclude item comes first, the function returns zero even when all\n - positive pathspecs share a directory.\n + remaining items share a directory.\n \n - Track the first positive item explicitly. Return its match and the\n - common prefix length together so that common_prefix() and\n - fill_directory() use the correct string. Add a unit test with an\n - unrelated exclude before two positive pathspecs that share a directory.\n + Track the first non-exclude item explicitly. Return its match through\n + an output parameter so that common_prefix() and fill_directory() use\n + the correct string. Add a unit test with an unrelated exclude item\n + before two non-exclude items that share a directory.\n \n Signed-off-by: Yannik Tausch \n \n @@ dir.c: static int fnmatch_icase_mem(const char *pattern, int patternlen,\n }\n \n -static size_t common_prefix_len(const struct pathspec *pathspec)\n -+struct pathspec_prefix {\n -+\tconst char *match;\n -+\tsize_t len;\n -+};\n -+\n -+/*\n -+ * Find the common prefix of positive pathspec items. The returned match\n -+ * points into the first positive item and is not NUL-terminated at len.\n -+ */\n -+static struct pathspec_prefix find_common_prefix(const struct pathspec *pathspec)\n ++static size_t common_prefix_len(const struct pathspec *pathspec,\n ++\t\t\t\tconst char **matched_prefix)\n {\n -\tint n;\n -+\tstruct pathspec_prefix prefix = { 0 };\n +\tint n, first = -1;\n \tsize_t max = 0;\n \n @@ dir.c: static size_t common_prefix_len(const struct pathspec *pathspec)\n \t\t\t\tbreak;\n \t\t}\n \t}\n --\treturn max;\n -+\tprefix.match = first < 0 ? NULL : pathspec->items[first].match;\n -+\tprefix.len = max;\n -+\treturn prefix;\n ++\t*matched_prefix = first < 0 ? NULL : pathspec->items[first].match;\n + \treturn max;\n }\n \n /*\n - * Returns a copy of the longest leading path common among all\n -+ * Returns a copy of the longest leading path common among all positive\n - * pathspecs.\n +- * pathspecs.\n ++ * Returns a copy of the longest leading path common among all pathspec\n ++ * items that are not excluded.\n */\n char *common_prefix(const struct pathspec *pathspec)\n {\n -\tunsigned long len = common_prefix_len(pathspec);\n -+\tstruct pathspec_prefix prefix = find_common_prefix(pathspec);\n ++\tconst char *matched_prefix;\n ++\tsize_t len = common_prefix_len(pathspec, &matched_prefix);\n \n -\treturn len ? xmemdupz(pathspec->items[0].match, len) : NULL;\n -+\treturn prefix.len ? xmemdupz(prefix.match, prefix.len) : NULL;\n ++\treturn len ? xmemdupz(matched_prefix, len) : NULL;\n }\n \n int fill_directory(struct dir_struct *dir,\n @@ dir.c: static size_t common_prefix_len(const struct pathspec *pathspec)\n \t\t const struct pathspec *pathspec)\n {\n -\tconst char *prefix;\n --\tsize_t prefix_len;\n -+\tstruct pathspec_prefix prefix;\n ++\tconst char *matched_prefix;\n + \tsize_t prefix_len;\n \n \tunsigned exclusive_flags = DIR_SHOW_IGNORED | DIR_SHOW_IGNORED_TOO;\n - \tif ((dir->flags & exclusive_flags) == exclusive_flags)\n @@ dir.c: int fill_directory(struct dir_struct *dir,\n \t * Calculate common prefix for the pathspec, and\n \t * use that to optimize the directory walk\n \t */\n -\tprefix_len = common_prefix_len(pathspec);\n -\tprefix = prefix_len ? pathspec->items[0].match : \"\";\n -+\tprefix = find_common_prefix(pathspec);\n ++\tprefix_len = common_prefix_len(pathspec, &matched_prefix);\n \n \t/* Read the directory and prune it */\n -\tread_directory(dir, istate, prefix, prefix_len, pathspec);\n -+\tread_directory(dir, istate, prefix.len ? prefix.match : \"\",\n -+\t\t prefix.len, pathspec);\n ++\tread_directory(dir, istate, prefix_len ? matched_prefix : \"\",\n ++\t\t prefix_len, pathspec);\n \n --\treturn prefix_len;\n -+\treturn prefix.len;\n + \treturn prefix_len;\n }\n - \n - int within_depth(const char *name, int namelen,\n @@ dir.c: static int match_pathspec_item(struct index_state *istate,\n \n \t/*\n \t * The normal call pattern is:\n -\t * 1. prefix = common_prefix_len(ps);\n -+\t * 1. prefix = find_common_prefix(ps).len;\n ++\t * 1. prefix = common_prefix_len(ps, &matched_prefix);\n \t * 2. prune something, or fill_directory\n \t * 3. match_pathspec()\n \t *\n @@ dir.c: static int match_pathspec_item(struct index_state *istate,\n - \t * prefix part when :(icase) is involved. We do exact\n - \t * comparison ourselves.\n - \t *\n --\t * Normally the caller (common_prefix_len() in fact) does\n -+\t * Normally the caller (find_common_prefix() in fact) does\n + \t * Normally the caller (common_prefix_len() in fact) does\n \t * _exact_ matching on name[-prefix+1..-1] and we do not need\n \t * to check that part. Be defensive and check it anyway, in\n -\t * case common_prefix_len is changed, or a new caller is\n -\t * introduced that does not use common_prefix_len.\n -+\t * case find_common_prefix() is changed, or a new caller is\n -+\t * introduced that does not use find_common_prefix().\n ++\t * case common_prefix_len() is changed, or a new caller is\n ++\t * introduced that does not use common_prefix_len().\n \t *\n \t * If the penalty turns out too high when prefix is really\n \t * long, maybe change it to\n @@ t/unit-tests/u-dir.c: void test_dir__within_depth(void)\n \n }\n +\n -+void test_dir__common_prefix_skips_excluded_pathspecs(void)\n ++void test_dir__common_prefix_skips_excluded_pathspec_items(void)\n +{\n +\tstruct pathspec_item items[] = {\n +\t\t{\n-- \n2.55.0\n\n"},{"id":"551850","messageId":"0617001F-13BB-4548-A10A-89877977CFB5@ytausch.de","threadId":"66254","inReplyTo":"886A25E6-8854-4AF6-BF0B-CFB57B673026@ytausch.de","subject":"[PATCH v2 1/2] dir: do not apply prefix to negative pathspecs","fromName":"Yannik Tausch","fromEmail":"dev@ytausch.de","sentAt":"2026-09-03T10:03:32Z","receivedAt":"2026-09-03T10:03:51Z","isPatch":true,"body":"common_prefix_len() derives the common prefix solely from positive\npathspecs, skipping those marked with PATHSPEC_EXCLUDE. However,\nmatch_pathspec_with_flags() also passes that prefix when matching the\nnegative pathspecs.\n\nA negative pathspec may be shorter than the prefix. In that case,\nmatch_pathspec_item() advances item->match beyond its allocation and\nsubtracts the prefix from item->len, producing a negative matchlen. It\nthen dereferences the out-of-bounds pointer. If the resulting byte is\nnot NUL, matchlen is converted to size_t when passed to ps_strncmp(),\nwhich may cause a much larger out-of-bounds read.\n\nThe problem can be reproduced with AddressSanitizer:\n\n make SANITIZE=address CFLAGS=\"-g -O0\" git\n git init test &&\n cd test &&\n DIR=$(printf \"a%.0s\" {1..150}) &&\n mkdir -p \"$DIR\" &&\n touch \"$DIR/f.txt\" &&\n git add -A &&\n git commit -m test &&\n ../git ls-files -- \"$DIR/\" \":(exclude)xy\"\n\nThis reports a heap-buffer-overflow. Without AddressSanitizer, the\noutput may depend on the contents of memory following the negative\npathspec.\n\nFix the bug by using a zero prefix when matching negative pathspecs.\nAdd a regression test that combines a positive pathspec with a longer\ncommon prefix and a shorter, unrelated negative pathspec.\n\nSigned-off-by: Yannik Tausch \n---\n dir.c | 2 +-\n t/t6132-pathspec-exclude.sh | 9 +++++++++\n 2 files changed, 10 insertions(+), 1 deletion(-)\n\ndiff --git a/dir.c b/dir.c\nindex 95d8a1cce9..7072715389 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -593,7 +593,7 @@ static int match_pathspec_with_flags(struct index_state *istate,\n \tif (!(ps->magic & PATHSPEC_EXCLUDE) || !positive)\n \t\treturn positive;\n \tnegative = do_match_pathspec(istate, ps, name, namelen,\n-\t\t\t\t prefix, seen,\n+\t\t\t\t 0, seen,\n \t\t\t\t flags | DO_MATCH_EXCLUDE);\n \treturn negative ? 0 : positive;\n }\ndiff --git a/t/t6132-pathspec-exclude.sh b/t/t6132-pathspec-exclude.sh\nindex 9fdafeb1e9..ad919cc739 100755\n--- a/t/t6132-pathspec-exclude.sh\n+++ b/t/t6132-pathspec-exclude.sh\n@@ -183,6 +183,15 @@ EOF\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'negative pathspec shorter than positive pathspec prefix' '\n+\tgit ls-files -- sub/sub/ \":(exclude)sub2\" >actual &&\n+\tcat <<-\\EOF >expect &&\n+\tsub/sub/file\n+\tsub/sub/sub/file\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'multiple exclusions' '\n \tgit ls-files -- \":^*/file2\" \":^sub2\" >actual &&\n \tcat <<-\\EOF >expect &&\n-- \n2.55.0\n\n"},{"id":"551853","messageId":"27FF785F-F5D5-44EC-93C2-5BD67BD99147@ytausch.de","threadId":"66254","inReplyTo":"886A25E6-8854-4AF6-BF0B-CFB57B673026@ytausch.de","subject":"[PATCH v2 2/2] dir: find common prefix among non-exclude pathspec items","fromName":"Yannik Tausch","fromEmail":"dev@ytausch.de","sentAt":"2026-09-03T10:04:51Z","receivedAt":"2026-09-03T10:05:08Z","isPatch":true,"body":"common_prefix_len() skips exclude pathspec items, but uses n == 0 to\nidentify the initial item and items[0] as the comparison source. When\nan exclude item comes first, the function returns zero even when all\nremaining items share a directory.\n\nTrack the first non-exclude item explicitly. Return its match through\nan output parameter so that common_prefix() and fill_directory() use\nthe correct string. Add a unit test with an unrelated exclude item\nbefore two non-exclude items that share a directory.\n\nSigned-off-by: Yannik Tausch \n---\n dir.c | 37 +++++++++++++++++++++----------------\n t/unit-tests/u-dir.c | 28 ++++++++++++++++++++++++++++\n 2 files changed, 49 insertions(+), 16 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 7072715389..d896e7be4b 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -212,9 +212,10 @@ static int fnmatch_icase_mem(const char *pattern, int patternlen,\n \treturn match_status;\n }\n \n-static size_t common_prefix_len(const struct pathspec *pathspec)\n+static size_t common_prefix_len(const struct pathspec *pathspec,\n+\t\t\t\tconst char **matched_prefix)\n {\n-\tint n;\n+\tint n, first = -1;\n \tsize_t max = 0;\n \n \t/*\n@@ -237,43 +238,47 @@ static size_t common_prefix_len(const struct pathspec *pathspec)\n \t\tsize_t i = 0, len = 0, item_len;\n \t\tif (pathspec->items[n].magic & PATHSPEC_EXCLUDE)\n \t\t\tcontinue;\n+\t\tif (first < 0)\n+\t\t\tfirst = n;\n \t\tif (pathspec->items[n].magic & PATHSPEC_ICASE)\n \t\t\titem_len = pathspec->items[n].prefix;\n \t\telse\n \t\t\titem_len = pathspec->items[n].nowildcard_len;\n-\t\twhile (i < item_len && (n == 0 || i < max)) {\n+\t\twhile (i < item_len && (n == first || i < max)) {\n \t\t\tchar c = pathspec->items[n].match[i];\n-\t\t\tif (c != pathspec->items[0].match[i])\n+\t\t\tif (c != pathspec->items[first].match[i])\n \t\t\t\tbreak;\n \t\t\tif (c == '/')\n \t\t\t\tlen = i + 1;\n \t\t\ti++;\n \t\t}\n-\t\tif (n == 0 || len < max) {\n+\t\tif (n == first || len < max) {\n \t\t\tmax = len;\n \t\t\tif (!max)\n \t\t\t\tbreak;\n \t\t}\n \t}\n+\t*matched_prefix = first < 0 ? NULL : pathspec->items[first].match;\n \treturn max;\n }\n \n /*\n- * Returns a copy of the longest leading path common among all\n- * pathspecs.\n+ * Returns a copy of the longest leading path common among all pathspec\n+ * items that are not excluded.\n */\n char *common_prefix(const struct pathspec *pathspec)\n {\n-\tunsigned long len = common_prefix_len(pathspec);\n+\tconst char *matched_prefix;\n+\tsize_t len = common_prefix_len(pathspec, &matched_prefix);\n \n-\treturn len ? xmemdupz(pathspec->items[0].match, len) : NULL;\n+\treturn len ? xmemdupz(matched_prefix, len) : NULL;\n }\n \n int fill_directory(struct dir_struct *dir,\n \t\t struct index_state *istate,\n \t\t const struct pathspec *pathspec)\n {\n-\tconst char *prefix;\n+\tconst char *matched_prefix;\n \tsize_t prefix_len;\n \n \tunsigned exclusive_flags = DIR_SHOW_IGNORED | DIR_SHOW_IGNORED_TOO;\n@@ -284,11 +289,11 @@ int fill_directory(struct dir_struct *dir,\n \t * Calculate common prefix for the pathspec, and\n \t * use that to optimize the directory walk\n \t */\n-\tprefix_len = common_prefix_len(pathspec);\n-\tprefix = prefix_len ? pathspec->items[0].match : \"\";\n+\tprefix_len = common_prefix_len(pathspec, &matched_prefix);\n \n \t/* Read the directory and prune it */\n-\tread_directory(dir, istate, prefix, prefix_len, pathspec);\n+\tread_directory(dir, istate, prefix_len ? matched_prefix : \"\",\n+\t\t prefix_len, pathspec);\n \n \treturn prefix_len;\n }\n@@ -394,7 +399,7 @@ static int match_pathspec_item(struct index_state *istate,\n \n \t/*\n \t * The normal call pattern is:\n-\t * 1. prefix = common_prefix_len(ps);\n+\t * 1. prefix = common_prefix_len(ps, &matched_prefix);\n \t * 2. prune something, or fill_directory\n \t * 3. match_pathspec()\n \t *\n@@ -414,8 +419,8 @@ static int match_pathspec_item(struct index_state *istate,\n \t * Normally the caller (common_prefix_len() in fact) does\n \t * _exact_ matching on name[-prefix+1..-1] and we do not need\n \t * to check that part. Be defensive and check it anyway, in\n-\t * case common_prefix_len is changed, or a new caller is\n-\t * introduced that does not use common_prefix_len.\n+\t * case common_prefix_len() is changed, or a new caller is\n+\t * introduced that does not use common_prefix_len().\n \t *\n \t * If the penalty turns out too high when prefix is really\n \t * long, maybe change it to\ndiff --git a/t/unit-tests/u-dir.c b/t/unit-tests/u-dir.c\nindex 2d0adaa39e..a3442c3d3c 100644\n--- a/t/unit-tests/u-dir.c\n+++ b/t/unit-tests/u-dir.c\n@@ -45,3 +45,31 @@ void test_dir__within_depth(void)\n \n \n }\n+\n+void test_dir__common_prefix_skips_excluded_pathspec_items(void)\n+{\n+\tstruct pathspec_item items[] = {\n+\t\t{\n+\t\t\t.match = \"unrelated/path\",\n+\t\t\t.magic = PATHSPEC_EXCLUDE,\n+\t\t\t.nowildcard_len = 14,\n+\t\t},\n+\t\t{\n+\t\t\t.match = \"foo/bar\",\n+\t\t\t.nowildcard_len = 7,\n+\t\t},\n+\t\t{\n+\t\t\t.match = \"foo/baz\",\n+\t\t\t.nowildcard_len = 7,\n+\t\t},\n+\t};\n+\tstruct pathspec pathspec = {\n+\t\t.nr = ARRAY_SIZE(items),\n+\t\t.magic = PATHSPEC_EXCLUDE,\n+\t\t.items = items,\n+\t};\n+\tchar *prefix = common_prefix(&pathspec);\n+\n+\tcl_assert_equal_s(prefix, \"foo/\");\n+\tfree(prefix);\n+}\n-- \n2.55.0\n\n"},{"id":"551856","messageId":"777C1706-60E8-4AAE-9EAB-E509567FABF6@ytausch.de","threadId":"66254","inReplyTo":"CY5PR17MB6144A1A7BF2E101FE26A6A85B1B62@CY5PR17MB6144.namprd17.prod.outlook.com","subject":"Re: [PATCH v2 2/2] dir: find common prefix among non-exclude pathspec items","fromName":"Yannik Tausch","fromEmail":"dev@ytausch.de","sentAt":"2026-09-03T11:49:19Z","receivedAt":"2026-09-03T11:49:45Z","isPatch":true,"body":"Hi,\n\n> Darik P wrote:\n> \n> 940-842-9147\n\ncould you clarify what these numbers refer to?\n\nNote that, as indicated in the v1 patch (https://lore.kernel.org/git/0CA8678D-0540-4A2E-B314-B9BEB04E2BF5@ytausch.de/), I already discussed this matter with the git security mailing list. Not sure if it might be related to it.\n\nYannik"},{"id":"551889","messageId":"A4F31FE9-901E-46EE-B4B5-DDE0FDA8F4EE@ytausch.de","threadId":"66254","inReplyTo":"886A25E6-8854-4AF6-BF0B-CFB57B673026@ytausch.de","subject":"Re: [PATCH v2 0/2] dir: fix pathspec prefixes with exclusions","fromName":"Yannik Tausch","fromEmail":"dev@ytausch.de","sentAt":"2026-09-03T18:06:06Z","receivedAt":"2026-09-03T18:06:22Z","isPatch":true,"body":"I just found [1], which is related to this patch series. I didn’t review the discussion in detail yet, will follow up.\n\n[1]: https://lore.kernel.org/git/xmqqv78qw3hc.fsf@gitster.g/T/#t"},{"id":"551890","messageId":"xmqq4ig6cihc.fsf@gitster.g","threadId":"66254","inReplyTo":"27FF785F-F5D5-44EC-93C2-5BD67BD99147@ytausch.de","subject":"Re: [PATCH v2 2/2] dir: find common prefix among non-exclude pathspec items","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-09-03T18:11:59Z","receivedAt":"2026-09-03T18:12:06Z","isPatch":true,"body":"Yannik Tausch writes:\n\n> +void test_dir__common_prefix_skips_excluded_pathspec_items(void)\n> +{\n> +\tstruct pathspec_item items[] = {\n> +\t\t{\n> +\t\t\t.match = \"unrelated/path\",\n> +\t\t\t.magic = PATHSPEC_EXCLUDE,\n> +\t\t\t.nowildcard_len = 14,\n> +\t\t},\n\nThis unfortunately triggers\n\nt/unit-tests/u-dir.c: In function 'test_dir__common_prefix_skips_excluded_pathspec_items':\nt/unit-tests/u-dir.c:53:34: error: initialization discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers]\n 53 | .match = \"unrelated/path\",\n | ^~~~~~~~~~~~~~~~\n\nOther than that, looking good.\n"},{"id":"551891","messageId":"xmqqy0dib3ue.fsf_-_@gitster.g","threadId":"66254","inReplyTo":"xmqq4ig6cihc.fsf@gitster.g","subject":"pathspec: match and original in pathspec_item are const","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-09-03T18:13:29Z","receivedAt":"2026-09-03T18:13:32Z","isPatch":false,"body":"Junio C Hamano writes:\n\n> This unfortunately triggers\n>\n> t/unit-tests/u-dir.c: In function 'test_dir__common_prefix_skips_excluded_pathspec_items':\n> t/unit-tests/u-dir.c:53:34: error: initialization discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers]\n> 53 | .match = \"unrelated/path\",\n> | ^~~~~~~~~~~~~~~~\n>\n> Other than that, looking good.\n\nWe may want a preparatory patch before this step.\n\n----- >8 -----\nSubject: pathspec: match and original in pathspec_item are const\n\nNo existing code modifies these two strings in pathspec elements\nafter they are created via these two pointers. Declare them as\n\"const char *\" to stress on this fact and cast away constness from\nthe code that frees these two strings.\n\nSigned-off-by: Junio C Hamano \n---\n pathspec.c | 4 ++--\n pathspec.h | 4 ++--\n 2 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git c/pathspec.c w/pathspec.c\nindex f78b22709c..06b7065372 100644\n--- c/pathspec.c\n+++ w/pathspec.c\n@@ -749,8 +749,8 @@ void clear_pathspec(struct pathspec *pathspec)\n \tint i, j;\n \n \tfor (i = 0; i < pathspec->nr; i++) {\n-\t\tfree(pathspec->items[i].match);\n-\t\tfree(pathspec->items[i].original);\n+\t\tfree((void *)pathspec->items[i].match);\n+\t\tfree((void *)pathspec->items[i].original);\n \n \t\tfor (j = 0; j < pathspec->items[i].attr_match_nr; j++)\n \t\t\tfree(pathspec->items[i].attr_match[j].value);\ndiff --git c/pathspec.h w/pathspec.h\nindex 5e3a6f1fe7..fc1b9465ad 100644\n--- c/pathspec.h\n+++ w/pathspec.h\n@@ -35,8 +35,8 @@ struct pathspec {\n \tunsigned magic;\n \tint max_depth;\n \tstruct pathspec_item {\n-\t\tchar *match;\n-\t\tchar *original;\n+\t\tconst char *match;\n+\t\tconst char *original;\n \t\tunsigned magic;\n \t\tint len, prefix;\n \t\tint nowildcard_len;\n"},{"id":"551895","messageId":"4439BA70-2C03-499D-B3CE-E43700C0A8DA@ytausch.de","threadId":"66254","inReplyTo":"xmqqy0dib3ue.fsf_-_@gitster.g","subject":"Re: pathspec: match and original in pathspec_item are const","fromName":"Yannik Tausch","fromEmail":"dev@ytausch.de","sentAt":"2026-09-03T18:37:57Z","receivedAt":"2026-09-03T18:38:12Z","isPatch":false,"body":"> I just found [1], which is related to this patch series. I didn’t review the discussion in detail yet, will follow up.\n\nI read this as you came to the same conclusion as me independently discovering the same issue in July. Perfect! I hope it’s fine that I took over the fix that way.\n\n> Junio C Hamano writes:\n> \n>> This unfortunately triggers\n>> \n>> t/unit-tests/u-dir.c: In function 'test_dir__common_prefix_skips_excluded_pathspec_items':\n>> t/unit-tests/u-dir.c:53:34: error: initialization discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers]\n>> 53 | .match = \"unrelated/path\",\n>> | ^~~~~~~~~~~~~~~~\n>> \n>> Other than that, looking good.\n> \n> We may want a preparatory patch before this step.\n\nThanks, I will include your preparatory patch in v3. \n\n"},{"id":"551897","messageId":"887D6D84-F76E-4DCB-9633-CD78DA02BCC5@ytausch.de","threadId":"66254","inReplyTo":"886A25E6-8854-4AF6-BF0B-CFB57B673026@ytausch.de","subject":"[PATCH v3 0/3] dir: fix pathspec prefixes with exclusions","fromName":"Yannik Tausch","fromEmail":"dev@ytausch.de","sentAt":"2026-09-03T18:43:05Z","receivedAt":"2026-09-03T18:43:22Z","isPatch":true,"body":"Pathspec prefix optimization must account for exclude items separately.\nThe prefix is derived from non-exclude items, so applying it while matching\nan exclude item can compare the wrong portions of the paths. Conversely, an\nexclude item at the start of the pathspec currently prevents finding a common\nprefix among the remaining items.\n\nThe first patch, authored by Junio, marks the immutable strings in a pathspec\nitem as const. The second patch matches exclude items against the full\npathname. The third patch finds the common prefix starting with the first\nnon-exclude item and returns both the prefix length and the string from which\nit was derived.\n\nChanges since v2:\n\n* Add Junio's preparatory const-correctness patch, which also fixes the unit\n test build with DEVELOPER=1.\n* Keep the two pathspec prefix fixes unchanged.\n\nJunio C Hamano (1):\n pathspec: match and original in pathspec_item are const\n\nYannik Tausch (2):\n dir: do not apply prefix to negative pathspecs\n dir: find common prefix among non-exclude pathspec items\n\n dir.c | 39 +++++++++++++++++++++----------------\n pathspec.c | 4 ++--\n pathspec.h | 4 ++--\n t/t6132-pathspec-exclude.sh | 9 +++++++++\n t/unit-tests/u-dir.c | 28 ++++++++++++++++++++++++++\n 5 files changed, 63 insertions(+), 21 deletions(-)\n\nRange-diff against v2:\n-: ---------- > 1: a257ce081e pathspec: match and original in pathspec_item are const\n1: c8a2f1e22e = 2: 16c6df5080 dir: do not apply prefix to negative pathspecs\n2: d0e08fdb96 = 3: b05b77f399 dir: find common prefix among non-exclude pathspec items\n-- \n2.55.0\n\n"},{"id":"551898","messageId":"D071AFC9-5727-4445-AB71-39ABB0C77C44@ytausch.de","threadId":"66254","inReplyTo":"887D6D84-F76E-4DCB-9633-CD78DA02BCC5@ytausch.de","subject":"[PATCH v3 1/3] pathspec: match and original in pathspec_item are const","fromName":"Yannik Tausch","fromEmail":"dev@ytausch.de","sentAt":"2026-09-03T18:44:24Z","receivedAt":"2026-09-03T18:44:39Z","isPatch":true,"body":"From: Junio C Hamano \n\nNo existing code modifies these two strings in pathspec elements\nafter they are created via these two pointers. Declare them as\n\"const char *\" to stress on this fact and cast away constness from\nthe code that frees these two strings.\n\nSigned-off-by: Junio C Hamano \nSigned-off-by: Yannik Tausch \n---\n pathspec.c | 4 ++--\n pathspec.h | 4 ++--\n 2 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/pathspec.c b/pathspec.c\nindex 281858f21f..41c53ff26e 100644\n--- a/pathspec.c\n+++ b/pathspec.c\n@@ -749,8 +749,8 @@ void clear_pathspec(struct pathspec *pathspec)\n \tint i, j;\n \n \tfor (i = 0; i < pathspec->nr; i++) {\n-\t\tfree(pathspec->items[i].match);\n-\t\tfree(pathspec->items[i].original);\n+\t\tfree((void *)pathspec->items[i].match);\n+\t\tfree((void *)pathspec->items[i].original);\n \n \t\tfor (j = 0; j < pathspec->items[i].attr_match_nr; j++)\n \t\t\tfree(pathspec->items[i].attr_match[j].value);\ndiff --git a/pathspec.h b/pathspec.h\nindex 5e3a6f1fe7..fc1b9465ad 100644\n--- a/pathspec.h\n+++ b/pathspec.h\n@@ -35,8 +35,8 @@ struct pathspec {\n \tunsigned magic;\n \tint max_depth;\n \tstruct pathspec_item {\n-\t\tchar *match;\n-\t\tchar *original;\n+\t\tconst char *match;\n+\t\tconst char *original;\n \t\tunsigned magic;\n \t\tint len, prefix;\n \t\tint nowildcard_len;\n-- \n2.55.0\n\n"},{"id":"551899","messageId":"A1808378-5CC7-4809-B7D6-2F420306339B@ytausch.de","threadId":"66254","inReplyTo":"887D6D84-F76E-4DCB-9633-CD78DA02BCC5@ytausch.de","subject":"[PATCH v3 2/3] dir: do not apply prefix to negative pathspecs","fromName":"Yannik Tausch","fromEmail":"dev@ytausch.de","sentAt":"2026-09-03T18:45:00Z","receivedAt":"2026-09-03T18:45:15Z","isPatch":true,"body":"common_prefix_len() derives the common prefix solely from positive\npathspecs, skipping those marked with PATHSPEC_EXCLUDE. However,\nmatch_pathspec_with_flags() also passes that prefix when matching the\nnegative pathspecs.\n\nA negative pathspec may be shorter than the prefix. In that case,\nmatch_pathspec_item() advances item->match beyond its allocation and\nsubtracts the prefix from item->len, producing a negative matchlen. It\nthen dereferences the out-of-bounds pointer. If the resulting byte is\nnot NUL, matchlen is converted to size_t when passed to ps_strncmp(),\nwhich may cause a much larger out-of-bounds read.\n\nThe problem can be reproduced with AddressSanitizer:\n\n make SANITIZE=address CFLAGS=\"-g -O0\" git\n git init test &&\n cd test &&\n DIR=$(printf \"a%.0s\" {1..150}) &&\n mkdir -p \"$DIR\" &&\n touch \"$DIR/f.txt\" &&\n git add -A &&\n git commit -m test &&\n ../git ls-files -- \"$DIR/\" \":(exclude)xy\"\n\nThis reports a heap-buffer-overflow. Without AddressSanitizer, the\noutput may depend on the contents of memory following the negative\npathspec.\n\nFix the bug by using a zero prefix when matching negative pathspecs.\nAdd a regression test that combines a positive pathspec with a longer\ncommon prefix and a shorter, unrelated negative pathspec.\n\nSigned-off-by: Yannik Tausch \n---\n dir.c | 2 +-\n t/t6132-pathspec-exclude.sh | 9 +++++++++\n 2 files changed, 10 insertions(+), 1 deletion(-)\n\ndiff --git a/dir.c b/dir.c\nindex 95d8a1cce9..7072715389 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -593,7 +593,7 @@ static int match_pathspec_with_flags(struct index_state *istate,\n \tif (!(ps->magic & PATHSPEC_EXCLUDE) || !positive)\n \t\treturn positive;\n \tnegative = do_match_pathspec(istate, ps, name, namelen,\n-\t\t\t\t prefix, seen,\n+\t\t\t\t 0, seen,\n \t\t\t\t flags | DO_MATCH_EXCLUDE);\n \treturn negative ? 0 : positive;\n }\ndiff --git a/t/t6132-pathspec-exclude.sh b/t/t6132-pathspec-exclude.sh\nindex 9fdafeb1e9..ad919cc739 100755\n--- a/t/t6132-pathspec-exclude.sh\n+++ b/t/t6132-pathspec-exclude.sh\n@@ -183,6 +183,15 @@ EOF\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'negative pathspec shorter than positive pathspec prefix' '\n+\tgit ls-files -- sub/sub/ \":(exclude)sub2\" >actual &&\n+\tcat <<-\\EOF >expect &&\n+\tsub/sub/file\n+\tsub/sub/sub/file\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'multiple exclusions' '\n \tgit ls-files -- \":^*/file2\" \":^sub2\" >actual &&\n \tcat <<-\\EOF >expect &&\n-- \n2.55.0\n\n"},{"id":"551901","messageId":"FD098AF3-8B3C-4581-833A-0170AB2933E4@ytausch.de","threadId":"66254","inReplyTo":"887D6D84-F76E-4DCB-9633-CD78DA02BCC5@ytausch.de","subject":"[PATCH v3 3/3] dir: find common prefix among non-exclude pathspec items","fromName":"Yannik Tausch","fromEmail":"dev@ytausch.de","sentAt":"2026-09-03T18:45:43Z","receivedAt":"2026-09-03T18:45:59Z","isPatch":true,"body":"common_prefix_len() skips exclude pathspec items, but uses n == 0 to\nidentify the initial item and items[0] as the comparison source. When\nan exclude item comes first, the function returns zero even when all\nremaining items share a directory.\n\nTrack the first non-exclude item explicitly. Return its match through\nan output parameter so that common_prefix() and fill_directory() use\nthe correct string. Add a unit test with an unrelated exclude item\nbefore two non-exclude items that share a directory.\n\nSigned-off-by: Yannik Tausch \n---\n dir.c | 37 +++++++++++++++++++++----------------\n t/unit-tests/u-dir.c | 28 ++++++++++++++++++++++++++++\n 2 files changed, 49 insertions(+), 16 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 7072715389..d896e7be4b 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -212,9 +212,10 @@ static int fnmatch_icase_mem(const char *pattern, int patternlen,\n \treturn match_status;\n }\n \n-static size_t common_prefix_len(const struct pathspec *pathspec)\n+static size_t common_prefix_len(const struct pathspec *pathspec,\n+\t\t\t\tconst char **matched_prefix)\n {\n-\tint n;\n+\tint n, first = -1;\n \tsize_t max = 0;\n \n \t/*\n@@ -237,43 +238,47 @@ static size_t common_prefix_len(const struct pathspec *pathspec)\n \t\tsize_t i = 0, len = 0, item_len;\n \t\tif (pathspec->items[n].magic & PATHSPEC_EXCLUDE)\n \t\t\tcontinue;\n+\t\tif (first < 0)\n+\t\t\tfirst = n;\n \t\tif (pathspec->items[n].magic & PATHSPEC_ICASE)\n \t\t\titem_len = pathspec->items[n].prefix;\n \t\telse\n \t\t\titem_len = pathspec->items[n].nowildcard_len;\n-\t\twhile (i < item_len && (n == 0 || i < max)) {\n+\t\twhile (i < item_len && (n == first || i < max)) {\n \t\t\tchar c = pathspec->items[n].match[i];\n-\t\t\tif (c != pathspec->items[0].match[i])\n+\t\t\tif (c != pathspec->items[first].match[i])\n \t\t\t\tbreak;\n \t\t\tif (c == '/')\n \t\t\t\tlen = i + 1;\n \t\t\ti++;\n \t\t}\n-\t\tif (n == 0 || len < max) {\n+\t\tif (n == first || len < max) {\n \t\t\tmax = len;\n \t\t\tif (!max)\n \t\t\t\tbreak;\n \t\t}\n \t}\n+\t*matched_prefix = first < 0 ? NULL : pathspec->items[first].match;\n \treturn max;\n }\n \n /*\n- * Returns a copy of the longest leading path common among all\n- * pathspecs.\n+ * Returns a copy of the longest leading path common among all pathspec\n+ * items that are not excluded.\n */\n char *common_prefix(const struct pathspec *pathspec)\n {\n-\tunsigned long len = common_prefix_len(pathspec);\n+\tconst char *matched_prefix;\n+\tsize_t len = common_prefix_len(pathspec, &matched_prefix);\n \n-\treturn len ? xmemdupz(pathspec->items[0].match, len) : NULL;\n+\treturn len ? xmemdupz(matched_prefix, len) : NULL;\n }\n \n int fill_directory(struct dir_struct *dir,\n \t\t struct index_state *istate,\n \t\t const struct pathspec *pathspec)\n {\n-\tconst char *prefix;\n+\tconst char *matched_prefix;\n \tsize_t prefix_len;\n \n \tunsigned exclusive_flags = DIR_SHOW_IGNORED | DIR_SHOW_IGNORED_TOO;\n@@ -284,11 +289,11 @@ int fill_directory(struct dir_struct *dir,\n \t * Calculate common prefix for the pathspec, and\n \t * use that to optimize the directory walk\n \t */\n-\tprefix_len = common_prefix_len(pathspec);\n-\tprefix = prefix_len ? pathspec->items[0].match : \"\";\n+\tprefix_len = common_prefix_len(pathspec, &matched_prefix);\n \n \t/* Read the directory and prune it */\n-\tread_directory(dir, istate, prefix, prefix_len, pathspec);\n+\tread_directory(dir, istate, prefix_len ? matched_prefix : \"\",\n+\t\t prefix_len, pathspec);\n \n \treturn prefix_len;\n }\n@@ -394,7 +399,7 @@ static int match_pathspec_item(struct index_state *istate,\n \n \t/*\n \t * The normal call pattern is:\n-\t * 1. prefix = common_prefix_len(ps);\n+\t * 1. prefix = common_prefix_len(ps, &matched_prefix);\n \t * 2. prune something, or fill_directory\n \t * 3. match_pathspec()\n \t *\n@@ -414,8 +419,8 @@ static int match_pathspec_item(struct index_state *istate,\n \t * Normally the caller (common_prefix_len() in fact) does\n \t * _exact_ matching on name[-prefix+1..-1] and we do not need\n \t * to check that part. Be defensive and check it anyway, in\n-\t * case common_prefix_len is changed, or a new caller is\n-\t * introduced that does not use common_prefix_len.\n+\t * case common_prefix_len() is changed, or a new caller is\n+\t * introduced that does not use common_prefix_len().\n \t *\n \t * If the penalty turns out too high when prefix is really\n \t * long, maybe change it to\ndiff --git a/t/unit-tests/u-dir.c b/t/unit-tests/u-dir.c\nindex 2d0adaa39e..a3442c3d3c 100644\n--- a/t/unit-tests/u-dir.c\n+++ b/t/unit-tests/u-dir.c\n@@ -45,3 +45,31 @@ void test_dir__within_depth(void)\n \n \n }\n+\n+void test_dir__common_prefix_skips_excluded_pathspec_items(void)\n+{\n+\tstruct pathspec_item items[] = {\n+\t\t{\n+\t\t\t.match = \"unrelated/path\",\n+\t\t\t.magic = PATHSPEC_EXCLUDE,\n+\t\t\t.nowildcard_len = 14,\n+\t\t},\n+\t\t{\n+\t\t\t.match = \"foo/bar\",\n+\t\t\t.nowildcard_len = 7,\n+\t\t},\n+\t\t{\n+\t\t\t.match = \"foo/baz\",\n+\t\t\t.nowildcard_len = 7,\n+\t\t},\n+\t};\n+\tstruct pathspec pathspec = {\n+\t\t.nr = ARRAY_SIZE(items),\n+\t\t.magic = PATHSPEC_EXCLUDE,\n+\t\t.items = items,\n+\t};\n+\tchar *prefix = common_prefix(&pathspec);\n+\n+\tcl_assert_equal_s(prefix, \"foo/\");\n+\tfree(prefix);\n+}\n-- \n2.55.0\n\n"},{"id":"551902","messageId":"xmqqbjaeb22p.fsf@gitster.g","threadId":"66254","inReplyTo":"4439BA70-2C03-499D-B3CE-E43700C0A8DA@ytausch.de","subject":"Re: pathspec: match and original in pathspec_item are const","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-09-03T18:51:42Z","receivedAt":"2026-09-03T18:51:48Z","isPatch":false,"body":"Yannik Tausch writes:\n\n>> I just found [1], which is related to this patch series. I didn’t review the discussion in detail yet, will follow up.\n>\n> I read this as you came to the same conclusion as me independently discovering the same issue in July. Perfect! I hope it’s fine that I took over the fix that way.\n>\n>> Junio C Hamano writes:\n>> \n>>> This unfortunately triggers\n>>> \n>>> t/unit-tests/u-dir.c: In function 'test_dir__common_prefix_skips_excluded_pathspec_items':\n>>> t/unit-tests/u-dir.c:53:34: error: initialization discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers]\n>>> 53 | .match = \"unrelated/path\",\n>>> | ^~~~~~~~~~~~~~~~\n>>> \n>>> Other than that, looking good.\n>> \n>> We may want a preparatory patch before this step.\n>\n> Thanks, I will include your preparatory patch in v3. \n\nThe 'const' patch will be queued separately, and a synthetic base will\nbe prepared for your two-patch series by merging the 'const' patch on\na recent tip of master.\n\nUnless you have other changes, there is no need for you to send a\nthree-patch series. We do not need to take the 'const' patch hostage\nto the 'pathspec' patch.\n\nThanks.\n"},{"id":"551904","messageId":"15ABB1A3-AAA6-4F53-B46C-C92E0B529520@ytausch.de","threadId":"66254","inReplyTo":"xmqqbjaeb22p.fsf@gitster.g","subject":"Re: pathspec: match and original in pathspec_item are const","fromName":"Yannik Tausch","fromEmail":"dev@ytausch.de","sentAt":"2026-09-03T18:57:36Z","receivedAt":"2026-09-03T18:57:52Z","isPatch":false,"body":"\n> The 'const' patch will be queued separately, and a synthetic base will\n> be prepared for your two-patch series by merging the 'const' patch on\n> a recent tip of master.\n> \n> Unless you have other changes, there is no need for you to send a\n> three-patch series. We do not need to take the 'const' patch hostage\n> to the 'pathspec' patch.\n\nOkay, anything I need to do now since I already submitted this as v3?\n"},{"id":"551913","messageId":"xmqqa4py9har.fsf@gitster.g","threadId":"66254","inReplyTo":"15ABB1A3-AAA6-4F53-B46C-C92E0B529520@ytausch.de","subject":"Re: pathspec: match and original in pathspec_item are const","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-09-03T21:05:48Z","receivedAt":"2026-09-03T21:05:51Z","isPatch":false,"body":"Yannik Tausch writes:\n\n>> The 'const' patch will be queued separately, and a synthetic base will\n>> be prepared for your two-patch series by merging the 'const' patch on\n>> a recent tip of master.\n>> \n>> Unless you have other changes, there is no need for you to send a\n>> three-patch series. We do not need to take the 'const' patch hostage\n>> to the 'pathspec' patch.\n>\n> Okay, anything I need to do now since I already submitted this as v3?\n\nIf [v3 2/3] and [v3 3/3] are identical to v2, just telling me to\nignore v3 would be sufficient.\n\nIf you need to make further changes, a two-patch series v4 on top of\nd66ac2af30 (Merge branch 'jc/pathspec-match-const' into\nyt/pathspec-negative-prefix, 2026-09-03) would be great.\n\nThanks.\n\n"},{"id":"551914","messageId":"8D0EAE11-B582-4C0E-9195-486FABF83FE3@ytausch.de","threadId":"66254","inReplyTo":"xmqqa4py9har.fsf@gitster.g","subject":"Re: pathspec: match and original in pathspec_item are const","fromName":"Yannik Tausch","fromEmail":"dev@ytausch.de","sentAt":"2026-09-03T21:13:07Z","receivedAt":"2026-09-03T21:13:23Z","isPatch":false,"body":"> If [v3 2/3] and [v3 3/3] are identical to v2, just telling me to\n> ignore v3 would be sufficient.\n\nPlease ignore v3 then.\n\n> If you need to make further changes, a two-patch series v4 on top of\n> d66ac2af30 (Merge branch 'jc/pathspec-match-const' into\n> yt/pathspec-negative-prefix, 2026-09-03) would be great.\n\nMany thanks, I‘ll use that if further changes become necessary in the review."},{"id":"551924","messageId":"CABPp-BFJo80oE=rtWc0FRNUxVh=6NHZeQmHD2q69VGwDcrHNhw@mail.gmail.com","threadId":"66254","inReplyTo":"0617001F-13BB-4548-A10A-89877977CFB5@ytausch.de","subject":"Re: [PATCH v2 1/2] dir: do not apply prefix to negative pathspecs","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2026-09-04T05:00:57Z","receivedAt":"2026-09-04T05:01:09Z","isPatch":true,"body":"Hi Yannik,\n\nOn Thu, Sep 3, 2026 at 3:23 AM Yannik Tausch wrote:\n>\n> common_prefix_len() derives the common prefix solely from positive\n> pathspecs, skipping those marked with PATHSPEC_EXCLUDE. However,\n> match_pathspec_with_flags() also passes that prefix when matching the\n> negative pathspecs.\n>\n> A negative pathspec may be shorter than the prefix. In that case,\n> match_pathspec_item() advances item->match beyond its allocation and\n> subtracts the prefix from item->len, producing a negative matchlen. It\n> then dereferences the out-of-bounds pointer. If the resulting byte is\n> not NUL, matchlen is converted to size_t when passed to ps_strncmp(),\n> which may cause a much larger out-of-bounds read.\n>\n> The problem can be reproduced with AddressSanitizer:\n>\n> make SANITIZE=address CFLAGS=\"-g -O0\" git\n> git init test &&\n> cd test &&\n> DIR=$(printf \"a%.0s\" {1..150}) &&\n> mkdir -p \"$DIR\" &&\n> touch \"$DIR/f.txt\" &&\n> git add -A &&\n> git commit -m test &&\n> ../git ls-files -- \"$DIR/\" \":(exclude)xy\"\n>\n> This reports a heap-buffer-overflow. Without AddressSanitizer, the\n> output may depend on the contents of memory following the negative\n> pathspec.\n>\n> Fix the bug by using a zero prefix when matching negative pathspecs.\n> Add a regression test that combines a positive pathspec with a longer\n> common prefix and a shorter, unrelated negative pathspec.\n>\n> Signed-off-by: Yannik Tausch \n> ---\n> dir.c | 2 +-\n> t/t6132-pathspec-exclude.sh | 9 +++++++++\n> 2 files changed, 10 insertions(+), 1 deletion(-)\n>\n> diff --git a/dir.c b/dir.c\n> index 95d8a1cce9..7072715389 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -593,7 +593,7 @@ static int match_pathspec_with_flags(struct index_state *istate,\n> if (!(ps->magic & PATHSPEC_EXCLUDE) || !positive)\n> return positive;\n> negative = do_match_pathspec(istate, ps, name, namelen,\n> - prefix, seen,\n> + 0, seen,\n> flags | DO_MATCH_EXCLUDE);\n> return negative ? 0 : positive;\n> }\n> diff --git a/t/t6132-pathspec-exclude.sh b/t/t6132-pathspec-exclude.sh\n> index 9fdafeb1e9..ad919cc739 100755\n> --- a/t/t6132-pathspec-exclude.sh\n> +++ b/t/t6132-pathspec-exclude.sh\n> @@ -183,6 +183,15 @@ EOF\n> test_cmp expect actual\n> '\n>\n> +test_expect_success 'negative pathspec shorter than positive pathspec prefix' '\n> + git ls-files -- sub/sub/ \":(exclude)sub2\" >actual &&\n> + cat <<-\\EOF >expect &&\n> + sub/sub/file\n> + sub/sub/sub/file\n> + EOF\n> + test_cmp expect actual\n> +'\n\nWould it make sense to add a regression case whose failure before this\npatch is deterministic without ASan?\n\nThe test above advances beyond the end of \"sub2\", so its result\ndepends on out-of-bounds memory. I actually saw this test pass\nwithout your fixes, when not run under ASan, which may depend on the\nallocator or build.\n\nAn alternative would be an exclude whose length equals the seven-byte\nprefix, keeping the accesses in bounds:\n\n test_expect_success 'exclude is matched against the full path' '\n git ls-files -- sub/sub/ \":(exclude)zzzzzzz\" >actual &&\n cat <<-\\EOF >expect &&\n sub/sub/file\n sub/sub/sub/file\n EOF\n test_cmp expect actual\n '\n\nBefore this patch, stripping seven bytes points at the exclude\nstring's NUL terminator, which is then treated as matching everything.\nI get no output before the fix, and both expected paths after your\nfix.\n\nI'm not suggesting this as a replacement for your regression test; I\nthink the out-of-bounds case is still useful. I just think this extra\ntestcase might be a nice complement.\n"},{"id":"551925","messageId":"CABPp-BF6hps9DibSV4ghbowkOD-NfEsHYFdLoKab0hCfEi9rgw@mail.gmail.com","threadId":"66254","inReplyTo":"27FF785F-F5D5-44EC-93C2-5BD67BD99147@ytausch.de","subject":"Re: [PATCH v2 2/2] dir: find common prefix among non-exclude pathspec items","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2026-09-04T05:02:06Z","receivedAt":"2026-09-04T05:02:18Z","isPatch":true,"body":"On Thu, Sep 3, 2026 at 3:08 AM Yannik Tausch wrote:\n>\n> common_prefix_len() skips exclude pathspec items, but uses n == 0 to\n> identify the initial item and items[0] as the comparison source. When\n> an exclude item comes first, the function returns zero even when all\n> remaining items share a directory.\n>\n> Track the first non-exclude item explicitly. Return its match through\n> an output parameter so that common_prefix() and fill_directory() use\n> the correct string. Add a unit test with an unrelated exclude item\n> before two non-exclude items that share a directory.\n\nThis to me looked more like what you are changing, and I had a hard\ntime figuring out why you were changing it.\n\nDoes the following alternative correctly capture your intent and change here? :\n\n\ndir: preserve pathspec prefix optimization with leading excludes\n\nDirectory walks use the common directory prefix of non-exclude\npathspec items to avoid scanning unrelated portions of the working\ntree or index. Exclude items only remove paths from that candidate\nset, so they do not need to widen the traversal.\n\nWhen an exclude item is the first pathspec item,\ncommon_prefix_len() fails to establish a comparison base and returns\na zero-length prefix. The result is correct, but git unnecessarily\ntraverses from a broader starting point even when all non-exclude\nitems share a directory.\n\nUse the first non-exclude item as the comparison base and return its\nstring together with the prefix length, allowing callers to start\nfrom the recovered directory prefix. Exclude matching continues to\nuse full paths, so this restores the optimization without changing\nwhich paths are selected. Add a unit test covering an exclude item\nbefore two non-exclude items with a common directory.\n\n\n> Signed-off-by: Yannik Tausch \n> ---\n> dir.c | 37 +++++++++++++++++++++----------------\n> t/unit-tests/u-dir.c | 28 ++++++++++++++++++++++++++++\n> 2 files changed, 49 insertions(+), 16 deletions(-)\n>\n> diff --git a/dir.c b/dir.c\n> index 7072715389..d896e7be4b 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -212,9 +212,10 @@ static int fnmatch_icase_mem(const char *pattern, int patternlen,\n> return match_status;\n> }\n>\n> -static size_t common_prefix_len(const struct pathspec *pathspec)\n> +static size_t common_prefix_len(const struct pathspec *pathspec,\n> + const char **matched_prefix)\n> {\n> - int n;\n> + int n, first = -1;\n> size_t max = 0;\n>\n> /*\n> @@ -237,43 +238,47 @@ static size_t common_prefix_len(const struct pathspec *pathspec)\n> size_t i = 0, len = 0, item_len;\n> if (pathspec->items[n].magic & PATHSPEC_EXCLUDE)\n> continue;\n> + if (first < 0)\n> + first = n;\n> if (pathspec->items[n].magic & PATHSPEC_ICASE)\n> item_len = pathspec->items[n].prefix;\n> else\n> item_len = pathspec->items[n].nowildcard_len;\n> - while (i < item_len && (n == 0 || i < max)) {\n> + while (i < item_len && (n == first || i < max)) {\n> char c = pathspec->items[n].match[i];\n> - if (c != pathspec->items[0].match[i])\n> + if (c != pathspec->items[first].match[i])\n> break;\n> if (c == '/')\n> len = i + 1;\n> i++;\n> }\n> - if (n == 0 || len < max) {\n> + if (n == first || len < max) {\n> max = len;\n> if (!max)\n> break;\n> }\n> }\n> + *matched_prefix = first < 0 ? NULL : pathspec->items[first].match;\n> return max;\n> }\n>\n> /*\n> - * Returns a copy of the longest leading path common among all\n> - * pathspecs.\n> + * Returns a copy of the longest leading path common among all pathspec\n> + * items that are not excluded.\n> */\n> char *common_prefix(const struct pathspec *pathspec)\n> {\n> - unsigned long len = common_prefix_len(pathspec);\n> + const char *matched_prefix;\n> + size_t len = common_prefix_len(pathspec, &matched_prefix);\n>\n> - return len ? xmemdupz(pathspec->items[0].match, len) : NULL;\n> + return len ? xmemdupz(matched_prefix, len) : NULL;\n> }\n>\n> int fill_directory(struct dir_struct *dir,\n> struct index_state *istate,\n> const struct pathspec *pathspec)\n> {\n> - const char *prefix;\n> + const char *matched_prefix;\n> size_t prefix_len;\n>\n> unsigned exclusive_flags = DIR_SHOW_IGNORED | DIR_SHOW_IGNORED_TOO;\n> @@ -284,11 +289,11 @@ int fill_directory(struct dir_struct *dir,\n> * Calculate common prefix for the pathspec, and\n> * use that to optimize the directory walk\n> */\n> - prefix_len = common_prefix_len(pathspec);\n> - prefix = prefix_len ? pathspec->items[0].match : \"\";\n> + prefix_len = common_prefix_len(pathspec, &matched_prefix);\n>\n> /* Read the directory and prune it */\n> - read_directory(dir, istate, prefix, prefix_len, pathspec);\n> + read_directory(dir, istate, prefix_len ? matched_prefix : \"\",\n> + prefix_len, pathspec);\n>\n> return prefix_len;\n> }\n> @@ -394,7 +399,7 @@ static int match_pathspec_item(struct index_state *istate,\n>\n> /*\n> * The normal call pattern is:\n> - * 1. prefix = common_prefix_len(ps);\n> + * 1. prefix = common_prefix_len(ps, &matched_prefix);\n> * 2. prune something, or fill_directory\n> * 3. match_pathspec()\n> *\n> @@ -414,8 +419,8 @@ static int match_pathspec_item(struct index_state *istate,\n> * Normally the caller (common_prefix_len() in fact) does\n> * _exact_ matching on name[-prefix+1..-1] and we do not need\n> * to check that part. Be defensive and check it anyway, in\n> - * case common_prefix_len is changed, or a new caller is\n> - * introduced that does not use common_prefix_len.\n> + * case common_prefix_len() is changed, or a new caller is\n> + * introduced that does not use common_prefix_len().\n> *\n> * If the penalty turns out too high when prefix is really\n> * long, maybe change it to\n> diff --git a/t/unit-tests/u-dir.c b/t/unit-tests/u-dir.c\n> index 2d0adaa39e..a3442c3d3c 100644\n> --- a/t/unit-tests/u-dir.c\n> +++ b/t/unit-tests/u-dir.c\n> @@ -45,3 +45,31 @@ void test_dir__within_depth(void)\n>\n>\n> }\n> +\n> +void test_dir__common_prefix_skips_excluded_pathspec_items(void)\n> +{\n> + struct pathspec_item items[] = {\n> + {\n> + .match = \"unrelated/path\",\n> + .magic = PATHSPEC_EXCLUDE,\n> + .nowildcard_len = 14,\n> + },\n> + {\n> + .match = \"foo/bar\",\n> + .nowildcard_len = 7,\n> + },\n> + {\n> + .match = \"foo/baz\",\n> + .nowildcard_len = 7,\n> + },\n> + };\n> + struct pathspec pathspec = {\n> + .nr = ARRAY_SIZE(items),\n> + .magic = PATHSPEC_EXCLUDE,\n> + .items = items,\n> + };\n> + char *prefix = common_prefix(&pathspec);\n> +\n> + cl_assert_equal_s(prefix, \"foo/\");\n> + free(prefix);\n> +}\n> --\n> 2.55.0\n\nIf my wording above is correct, I think the code looks like it\ncorrectly implements that idea.\n"},{"id":"551965","messageId":"xmqqmrtx6qsk.fsf@gitster.g","threadId":"66254","inReplyTo":"CABPp-BFJo80oE=rtWc0FRNUxVh=6NHZeQmHD2q69VGwDcrHNhw@mail.gmail.com","subject":"Re: [PATCH v2 1/2] dir: do not apply prefix to negative pathspecs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-09-04T14:21:15Z","receivedAt":"2026-09-04T14:21:18Z","isPatch":true,"body":"Elijah Newren writes:\n\n> Hi Yannik,\n>\n> On Thu, Sep 3, 2026 at 3:23 AM Yannik Tausch wrote:\n>>\n>> common_prefix_len() derives the common prefix solely from positive\n>> pathspecs, skipping those marked with PATHSPEC_EXCLUDE. However,\n>> match_pathspec_with_flags() also passes that prefix when matching the\n>> negative pathspecs.\n>>\n>> A negative pathspec may be shorter than the prefix. In that case,\n>> match_pathspec_item() advances item->match beyond its allocation and\n>> subtracts the prefix from item->len, producing a negative matchlen. It\n>> then dereferences the out-of-bounds pointer. If the resulting byte is\n>> not NUL, matchlen is converted to size_t when passed to ps_strncmp(),\n>> which may cause a much larger out-of-bounds read.\n>>\n>> The problem can be reproduced with AddressSanitizer:\n> ...\n> Would it make sense to add a regression case whose failure before this\n> patch is deterministic without ASan?\n\nVery good point.\n\nEven if a negative pathspec were long enough, it would produce an\nincorrect result if you strip the leading part of a negative entry.\n\nWith positive elements \"a/b\" and \"a/c\", and a negative element\n\"x/b\", both paths \"a/b/m\" and \"a/c/n\" should match the pathspec with\nthese three elements, but if you incorrectly use prefix=2 to strip\nthe common prefix computed across positives, i.e., \"a/\", while\ntrying to see if the path \"a/b/m\" matches negative \"x/b\", we'd end\nup trying to see if subpath \"b/m\" (in \"a/b/m\", after 2 leading\nprefix bytes are stripped away) matches subpattern \"b\" (in \"x/b\",\nafter incorrectly stripping 2 leading bytes). Yay, \"b/m\" begins\nwith \"b\" so it matches! Not quite.\n\n $ git init\n $ mkdir -p a/b a/c\n $ >a/b/m >a/c/n\n $ git add a\n $ rungit jch ls-files a/b ':!x/b' a/c\n a/b/m\n a/c/n\n $ rungit master ls-files a/b ':!x/b' a/c\n a/c/n\n\nSo \"if prefix computed across positives is longer than a negative\nelement\" is a special case that may manifest as one extra breakage\n(i.e., logically it is wrong in that it uses incorrectly shortened\npattern and path for negated matching and produce incorrect result,\nbut in addition to that, the negated pattern string points outside\nthe original string, accessing wrong piece of memory), but I tend to\nagree that it is equally if not more important to demonstrate what\nis broken even without that extra breakage.\n\nThanks.\n\n"},{"id":"551986","messageId":"xmqqy0dh3r2k.fsf@gitster.g","threadId":"66254","inReplyTo":"CABPp-BF6hps9DibSV4ghbowkOD-NfEsHYFdLoKab0hCfEi9rgw@mail.gmail.com","subject":"Re: [PATCH v2 2/2] dir: find common prefix among non-exclude pathspec items","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-09-04T16:43:31Z","receivedAt":"2026-09-04T16:43:33Z","isPatch":true,"body":"Elijah Newren writes:\n\n> This to me looked more like what you are changing, and I had a hard\n> time figuring out why you were changing it.\n\nWhile I share this assessment,...\n\n>\n> Does the following alternative correctly capture your intent and change here? :\n>\n>\n> dir: preserve pathspec prefix optimization with leading excludes\n>\n> Directory walks use the common directory prefix of non-exclude\n> pathspec items to avoid scanning unrelated portions of the working\n> tree or index. Exclude items only remove paths from that candidate\n> set, so they do not need to widen the traversal.\n>\n> When an exclude item is the first pathspec item,\n> common_prefix_len() fails to establish a comparison base and returns\n> a zero-length prefix. The result is correct, but git unnecessarily\n> traverses from a broader starting point even when all non-exclude\n> items share a directory.\n\n... I do not think this is true.\n\nWhat happens inside dir.c::fill_directory() is driven only with the\nreturn value of common_prefix_len(), which already ignores and has\nalways ignored the negative pathspec elements.\n\nWhat this [2/2] changes is what string common_prefix() returns. If\nyou have \"!x/b\" \"a/b\" \"a/c\", common_prefix_len() goes over the two\npositive ones \"a/b\" and \"a/c\" and correctly notices that \"a/\" is\ncommon among the positive ones and its length is 2.\n\nThe problem this patch fixes is that common_prefix() used to always\ngrab the first two bytes of the element that happens to be at the\nbeginning of pathspec, so a pathspec (\"!x/b\" \"a/b\" \"a/c\") would have\ngiven you \"!x\" as the common prefix string, which obviously is\nbogus. The common_prefix() is only used in two code paths that are\nquite distant from here. It is clear there is a bug (i.e., the code\nthat wants to be passed \"a/\" in such a case cannot be happy to see\n\"!x\" instead), but it is totally unclear what the end-user visible\neffect of that bug (i.e. what happens when overlay_tree_on_index()\npasses an incorrectly computed common_prefix() when \"git ls-files\"\nis run with \"--with-tree=\" option?).\n\n> Use the first non-exclude item as the comparison base and return its\n> string together with the prefix length, allowing callers to start\n> from the recovered directory prefix. Exclude matching continues to\n> use full paths, so this restores the optimization without changing\n> which paths are selected. Add a unit test covering an exclude item\n> before two non-exclude items with a common directory.\n\nI do not think this is what this patch does. What you are\ndescribing is this bit:\n\n>> -static size_t common_prefix_len(const struct pathspec *pathspec)\n>> ...\n>> size_t i = 0, len = 0, item_len;\n>> if (pathspec->items[n].magic & PATHSPEC_EXCLUDE)\n>> continue;\n\nwhich dates back to the very beginning of negative pathspec elements\nsupport introduced at ef79b1f870 (Support pathspec magic :(exclude)\nand its short form :!, 2013-12-06), I think.\n"},{"id":"551995","messageId":"CABPp-BHviE8uLgh6PE=6MYkz_zTDZfKU9CbHQjJOeLgA=qpUSA@mail.gmail.com","threadId":"66254","inReplyTo":"xmqqy0dh3r2k.fsf@gitster.g","subject":"Re: [PATCH v2 2/2] dir: find common prefix among non-exclude pathspec items","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2026-09-04T19:19:17Z","receivedAt":"2026-09-04T19:19:32Z","isPatch":true,"body":"On Fri, Sep 4, 2026 at 9:43 AM Junio C Hamano wrote:\n>\n> Elijah Newren writes:\n>\n> > This to me looked more like what you are changing, and I had a hard\n> > time figuring out why you were changing it.\n>\n> While I share this assessment,...\n>\n> >\n> > Does the following alternative correctly capture your intent and change here? :\n> >\n> >\n> > dir: preserve pathspec prefix optimization with leading excludes\n> >\n> > Directory walks use the common directory prefix of non-exclude\n> > pathspec items to avoid scanning unrelated portions of the working\n> > tree or index. Exclude items only remove paths from that candidate\n> > set, so they do not need to widen the traversal.\n> >\n> > When an exclude item is the first pathspec item,\n> > common_prefix_len() fails to establish a comparison base and returns\n> > a zero-length prefix. The result is correct, but git unnecessarily\n> > traverses from a broader starting point even when all non-exclude\n> > items share a directory.\n>\n> ... I do not think this is true.\n>\n> What happens inside dir.c::fill_directory() is driven only with the\n> return value of common_prefix_len(), which already ignores and has\n> always ignored the negative pathspec elements.\n>\n> What this [2/2] changes is what string common_prefix() returns. If\n> you have \"!x/b\" \"a/b\" \"a/c\", common_prefix_len() goes over the two\n> positive ones \"a/b\" and \"a/c\" and correctly notices that \"a/\" is\n> common among the positive ones and its length is 2.\n>\n> The problem this patch fixes is that common_prefix() used to always\n> grab the first two bytes of the element that happens to be at the\n> beginning of pathspec, so a pathspec (\"!x/b\" \"a/b\" \"a/c\") would have\n> given you \"!x\" as the common prefix string, which obviously is\n> bogus. The common_prefix() is only used in two code paths that are\n> quite distant from here. It is clear there is a bug (i.e., the code\n> that wants to be passed \"a/\" in such a case cannot be happy to see\n> \"!x\" instead), but it is totally unclear what the end-user visible\n> effect of that bug (i.e. what happens when overlay_tree_on_index()\n> passes an incorrectly computed common_prefix() when \"git ls-files\"\n> is run with \"--with-tree=\" option?).\n\nMaybe I'm misreading the code. Did it always grab the first two bytes\nof the element at the beginning of pathspec, or did it get an empty\nstring? By my reading of the code (copied here for convenience), it\ngot an empty string:\n\n>-static size_t common_prefix_len(const struct pathspec *pathspec)\n>+static size_t common_prefix_len(const struct pathspec *pathspec,\n>+ const char **matched_prefix)\n> {\n>- int n;\n>+ int n, first = -1;\n> size_t max = 0;\n[...]\n> for (n = 0; n < pathspec->nr; n++) {\n> size_t i = 0, len = 0, item_len;\n> if (pathspec->items[n].magic & PATHSPEC_EXCLUDE)\n> continue;\n>+ if (first < 0)\n>+ first = n;\n> if (pathspec->items[n].magic & PATHSPEC_ICASE)\n> item_len = pathspec->items[n].prefix;\n> else\n> item_len = pathspec->items[n].nowildcard_len;\n>- while (i < item_len && (n == 0 || i < max)) {\n>+ while (i < item_len && (n == first || i < max)) {\n> char c = pathspec->items[n].match[i];\n>- if (c != pathspec->items[0].match[i])\n>+ if (c != pathspec->items[first].match[i])\n> break;\n> if (c == '/')\n> len = i + 1;\n> i++;\n> }\n>- if (n == 0 || len < max) {\n>+ if (n == first || len < max) {\n> max = len;\n> if (!max)\n> break;\n> }\n> }\n>+ *matched_prefix = first < 0 ? NULL : pathspec->items[first].match;\n> return max;\n> }\n\nFollowing the preimage, and using your pathspec of (\"!x/b\", \"a/b\", \"a/c\"):\n - when n=0, we hit the PATHSPEC_EXCLUDE case at the top, so max remains 0\n - for each n>0, we fail both sides of the (n==0 || i < max checks),\nso len remains 0. We then fail (n==0 || len < max) checks, so max is\nnot adjusted (though it'd only be adjusted to 0 anyway)\nSo, at the end, max is 0 and we return 0.\n\n>> > Use the first non-exclude item as the comparison base and return its\n> > string together with the prefix length, allowing callers to start\n> > from the recovered directory prefix. Exclude matching continues to\n> > use full paths, so this restores the optimization without changing\n> > which paths are selected. Add a unit test covering an exclude item\n> > before two non-exclude items with a common directory.\n>\n> I do not think this is what this patch does. What you are\n> describing is this bit:\n>\n> >> -static size_t common_prefix_len(const struct pathspec *pathspec)\n> >> ...\n> >> size_t i = 0, len = 0, item_len;\n> >> if (pathspec->items[n].magic & PATHSPEC_EXCLUDE)\n> >> continue;\n>\n> which dates back to the very beginning of negative pathspec elements\n> support introduced at ef79b1f870 (Support pathspec magic :(exclude)\n> and its short form :!, 2013-12-06), I think.\n\nI was trying to describe \"n == first\" vs. \"n == 0\" in the last\nif-check, which allows us to set max to something greater than 0 when\nan excluded pathspec appears first.\n\nHappy to hear if I'm mis-reading or if my previous explanation\nmis-describes this.\n"},{"id":"552034","messageId":"xmqqse3n65gl.fsf@gitster.g","threadId":"66254","inReplyTo":"CABPp-BHviE8uLgh6PE=6MYkz_zTDZfKU9CbHQjJOeLgA=qpUSA@mail.gmail.com","subject":"Re: [PATCH v2 2/2] dir: find common prefix among non-exclude pathspec items","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-09-05T16:14:18Z","receivedAt":"2026-09-05T16:14:21Z","isPatch":true,"body":"Elijah Newren writes:\n\n> Maybe I'm misreading the code. Did it always grab the first two bytes\n> of the element at the beginning of pathspec, or did it get an empty\n> string? By my reading of the code (copied here for convenience), it\n> got an empty string:\n\nNo, I was the one who misread the code. Indeed in the loop, we\nassume all elements in the pathspec share the same prefix we have\nfound to be valid so far (the loop is about shortening what we found\nso far with later elements in the pathspec), and blindly use the\nfirst element, which is wrong.\n\n"},{"id":"552675","messageId":"7CB757FB-1F2D-4EE6-8C31-8C2CD6D42397@ytausch.de","threadId":"66254","inReplyTo":"886A25E6-8854-4AF6-BF0B-CFB57B673026@ytausch.de","subject":"[PATCH v4 0/2] dir: fix pathspec prefixes with exclusions","fromName":"Yannik Tausch","fromEmail":"dev@ytausch.de","sentAt":"2026-09-14T07:24:45Z","receivedAt":"2026-09-14T07:25:01Z","isPatch":true,"body":"Pathspec prefix optimization must account for exclude items separately.\nThe prefix is derived from non-exclude items, so applying it while\nmatching an exclude item can compare the wrong portions of the paths.\nConversely, an exclude item at the start of the pathspec currently\nprevents finding a common prefix among the remaining items.\n\nThe first patch matches exclude items against the full pathname. The\nsecond patch finds the common prefix starting with the first non-exclude\nitem and returns both the prefix length and the string from which it was\nderived.\n\nChanges since v3, which was withdrawn in favor of v2:\n\n* Return to a two-patch series based on d66ac2af30, leaving Junio's\n preparatory const-correctness patch on its separately queued topic.\n* Add the deterministic regression test suggested by Elijah, while\n retaining the shorter-pattern test for the out-of-bounds access.\n* Explain the observable incorrect match in patch 1 and use consistent\n non-exclude/exclude terminology.\n* Reword patch 2 to describe the directory-walk optimization it restores.\n\nYannik Tausch (2):\n dir: do not apply prefix to negative pathspecs\n dir: preserve pathspec prefix optimization with leading excludes\n\n dir.c | 39 +++++++++++++++++++++----------------\n t/t6132-pathspec-exclude.sh | 18 +++++++++++++++++\n t/unit-tests/u-dir.c | 28 ++++++++++++++++++++++++++\n 3 files changed, 68 insertions(+), 17 deletions(-)\n\nRange-diff against v2:\n1: c8a2f1e22e ! 1: adeb7f2fb6 dir: do not apply prefix to negative pathspecs\n @@ Metadata\n ## Commit message ##\n dir: do not apply prefix to negative pathspecs\n \n - common_prefix_len() derives the common prefix solely from positive\n - pathspecs, skipping those marked with PATHSPEC_EXCLUDE. However,\n - match_pathspec_with_flags() also passes that prefix when matching the\n - negative pathspecs.\n + common_prefix_len() derives the common prefix solely from non-exclude\n + pathspec items. However, match_pathspec_with_flags() also passes that\n + prefix when matching exclude items.\n \n - A negative pathspec may be shorter than the prefix. In that case,\n - match_pathspec_item() advances item->match beyond its allocation and\n - subtracts the prefix from item->len, producing a negative matchlen. It\n - then dereferences the out-of-bounds pointer. If the resulting byte is\n - not NUL, matchlen is converted to size_t when passed to ps_strncmp(),\n - which may cause a much larger out-of-bounds read.\n + This can produce incorrect results because that prefix does not\n + necessarily match an exclude item. For example, given non-exclude items\n + \"a/b\" and \"a/c\" and an exclude item \"x/b\", stripping the two-byte\n + prefix from both the pathname \"a/b/m\" and pattern \"x/b\" makes the\n + remaining strings match and incorrectly excludes the pathname.\n \n - The problem can be reproduced with AddressSanitizer:\n + If an exclude item is shorter than the prefix, match_pathspec_item()\n + instead advances item->match beyond its allocation and subtracts the\n + prefix from item->len, producing a negative matchlen. It then\n + dereferences the out-of-bounds pointer. If the resulting byte is not\n + NUL, matchlen is converted to size_t when passed to ps_strncmp(), which\n + may cause a much larger out-of-bounds read.\n +\n + The out-of-bounds access can be reproduced with AddressSanitizer:\n \n make SANITIZE=address CFLAGS=\"-g -O0\" git\n git init test &&\n @@ Commit message\n git commit -m test &&\n ../git ls-files -- \"$DIR/\" \":(exclude)xy\"\n \n - This reports a heap-buffer-overflow. Without AddressSanitizer, the\n - output may depend on the contents of memory following the negative\n - pathspec.\n -\n - Fix the bug by using a zero prefix when matching negative pathspecs.\n - Add a regression test that combines a positive pathspec with a longer\n - common prefix and a shorter, unrelated negative pathspec.\n + Fix the bug by using a zero prefix when matching exclude items. Add\n + regression tests for both the deterministic incorrect match and the\n + shorter exclude item that causes the out-of-bounds access.\n \n Signed-off-by: Yannik Tausch \n \n @@ t/t6132-pathspec-exclude.sh: EOF\n +\tEOF\n +\ttest_cmp expect actual\n +'\n ++\n ++test_expect_success 'exclude is matched against the full path' '\n ++\tgit ls-files -- sub/sub/ \":(exclude)zzzzzzz\" >actual &&\n ++\tcat <<-\\EOF >expect &&\n ++\tsub/sub/file\n ++\tsub/sub/sub/file\n ++\tEOF\n ++\ttest_cmp expect actual\n ++'\n +\n test_expect_success 'multiple exclusions' '\n \tgit ls-files -- \":^*/file2\" \":^sub2\" >actual &&\n2: d0e08fdb96 ! 2: e8f72cab9c dir: find common prefix among non-exclude pathspec items\n @@ Metadata\n Author: Yannik Tausch \n \n ## Commit message ##\n - dir: find common prefix among non-exclude pathspec items\n + dir: preserve pathspec prefix optimization with leading excludes\n \n - common_prefix_len() skips exclude pathspec items, but uses n == 0 to\n - identify the initial item and items[0] as the comparison source. When\n - an exclude item comes first, the function returns zero even when all\n - remaining items share a directory.\n + Directory walks use the common directory prefix of non-exclude\n + pathspec items to avoid scanning unrelated portions of the working\n + tree or index. Exclude items only remove paths from that candidate\n + set, so they do not need to widen the traversal.\n \n - Track the first non-exclude item explicitly. Return its match through\n - an output parameter so that common_prefix() and fill_directory() use\n - the correct string. Add a unit test with an unrelated exclude item\n - before two non-exclude items that share a directory.\n + When an exclude item is the first pathspec item, common_prefix_len()\n + fails to establish a comparison base and returns a zero-length prefix.\n + The result is correct, but Git unnecessarily traverses from a broader\n + starting point even when all non-exclude items share a directory.\n +\n + Use the first non-exclude item as the comparison base and return its\n + string together with the prefix length, allowing callers to start from\n + the recovered directory prefix. Exclude matching continues to use full\n + paths, so this restores the optimization without changing which paths\n + are selected. Add a unit test covering an exclude item before two\n + non-exclude items with a common directory.\n \n Signed-off-by: Yannik Tausch \n \n\nbase-commit: d66ac2af300f33bd9e8558c5645f2a808cc01f89\n-- \n2.55.0\n\n"},{"id":"552676","messageId":"C6BF8D32-470C-4C54-B4BD-CF9B1E0F191F@ytausch.de","threadId":"66254","inReplyTo":"7CB757FB-1F2D-4EE6-8C31-8C2CD6D42397@ytausch.de","subject":"[PATCH v4 1/2] dir: do not apply prefix to negative pathspecs","fromName":"Yannik Tausch","fromEmail":"dev@ytausch.de","sentAt":"2026-09-14T07:25:51Z","receivedAt":"2026-09-14T07:26:06Z","isPatch":true,"body":"common_prefix_len() derives the common prefix solely from non-exclude\npathspec items. However, match_pathspec_with_flags() also passes that\nprefix when matching exclude items.\n\nThis can produce incorrect results because that prefix does not\nnecessarily match an exclude item. For example, given non-exclude items\n\"a/b\" and \"a/c\" and an exclude item \"x/b\", stripping the two-byte\nprefix from both the pathname \"a/b/m\" and pattern \"x/b\" makes the\nremaining strings match and incorrectly excludes the pathname.\n\nIf an exclude item is shorter than the prefix, match_pathspec_item()\ninstead advances item->match beyond its allocation and subtracts the\nprefix from item->len, producing a negative matchlen. It then\ndereferences the out-of-bounds pointer. If the resulting byte is not\nNUL, matchlen is converted to size_t when passed to ps_strncmp(), which\nmay cause a much larger out-of-bounds read.\n\nThe out-of-bounds access can be reproduced with AddressSanitizer:\n\n make SANITIZE=address CFLAGS=\"-g -O0\" git\n git init test &&\n cd test &&\n DIR=$(printf \"a%.0s\" {1..150}) &&\n mkdir -p \"$DIR\" &&\n touch \"$DIR/f.txt\" &&\n git add -A &&\n git commit -m test &&\n ../git ls-files -- \"$DIR/\" \":(exclude)xy\"\n\nFix the bug by using a zero prefix when matching exclude items. Add\nregression tests for both the deterministic incorrect match and the\nshorter exclude item that causes the out-of-bounds access.\n\nSigned-off-by: Yannik Tausch \n---\n dir.c | 2 +-\n t/t6132-pathspec-exclude.sh | 18 ++++++++++++++++++\n 2 files changed, 19 insertions(+), 1 deletion(-)\n\ndiff --git a/dir.c b/dir.c\nindex 32430090dc..5f42c992d3 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -593,7 +593,7 @@ static int match_pathspec_with_flags(struct index_state *istate,\n \tif (!(ps->magic & PATHSPEC_EXCLUDE) || !positive)\n \t\treturn positive;\n \tnegative = do_match_pathspec(istate, ps, name, namelen,\n-\t\t\t\t prefix, seen,\n+\t\t\t\t 0, seen,\n \t\t\t\t flags | DO_MATCH_EXCLUDE);\n \treturn negative ? 0 : positive;\n }\ndiff --git a/t/t6132-pathspec-exclude.sh b/t/t6132-pathspec-exclude.sh\nindex 9fdafeb1e9..e0c3f73ef0 100755\n--- a/t/t6132-pathspec-exclude.sh\n+++ b/t/t6132-pathspec-exclude.sh\n@@ -183,6 +183,24 @@ EOF\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'negative pathspec shorter than positive pathspec prefix' '\n+\tgit ls-files -- sub/sub/ \":(exclude)sub2\" >actual &&\n+\tcat <<-\\EOF >expect &&\n+\tsub/sub/file\n+\tsub/sub/sub/file\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'exclude is matched against the full path' '\n+\tgit ls-files -- sub/sub/ \":(exclude)zzzzzzz\" >actual &&\n+\tcat <<-\\EOF >expect &&\n+\tsub/sub/file\n+\tsub/sub/sub/file\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'multiple exclusions' '\n \tgit ls-files -- \":^*/file2\" \":^sub2\" >actual &&\n \tcat <<-\\EOF >expect &&\n-- \n2.55.0\n\n"},{"id":"552677","messageId":"B90E7FCE-0A5F-4629-A2BE-37F2CAC5F3B1@ytausch.de","threadId":"66254","inReplyTo":"7CB757FB-1F2D-4EE6-8C31-8C2CD6D42397@ytausch.de","subject":"[PATCH v4 2/2] dir: preserve pathspec prefix optimization with leading excludes","fromName":"Yannik Tausch","fromEmail":"dev@ytausch.de","sentAt":"2026-09-14T07:27:05Z","receivedAt":"2026-09-14T07:27:26Z","isPatch":true,"body":"Directory walks use the common directory prefix of non-exclude\npathspec items to avoid scanning unrelated portions of the working\ntree or index. Exclude items only remove paths from that candidate\nset, so they do not need to widen the traversal.\n\nWhen an exclude item is the first pathspec item, common_prefix_len()\nfails to establish a comparison base and returns a zero-length prefix.\nThe result is correct, but Git unnecessarily traverses from a broader\nstarting point even when all non-exclude items share a directory.\n\nUse the first non-exclude item as the comparison base and return its\nstring together with the prefix length, allowing callers to start from\nthe recovered directory prefix. Exclude matching continues to use full\npaths, so this restores the optimization without changing which paths\nare selected. Add a unit test covering an exclude item before two\nnon-exclude items with a common directory.\n\nSigned-off-by: Yannik Tausch \n---\n dir.c | 37 +++++++++++++++++++++----------------\n t/unit-tests/u-dir.c | 28 ++++++++++++++++++++++++++++\n 2 files changed, 49 insertions(+), 16 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 5f42c992d3..abc4a78f31 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -212,9 +212,10 @@ static int fnmatch_icase_mem(const char *pattern, int patternlen,\n \treturn match_status;\n }\n \n-static size_t common_prefix_len(const struct pathspec *pathspec)\n+static size_t common_prefix_len(const struct pathspec *pathspec,\n+\t\t\t\tconst char **matched_prefix)\n {\n-\tint n;\n+\tint n, first = -1;\n \tsize_t max = 0;\n \n \t/*\n@@ -237,43 +238,47 @@ static size_t common_prefix_len(const struct pathspec *pathspec)\n \t\tsize_t i = 0, len = 0, item_len;\n \t\tif (pathspec->items[n].magic & PATHSPEC_EXCLUDE)\n \t\t\tcontinue;\n+\t\tif (first < 0)\n+\t\t\tfirst = n;\n \t\tif (pathspec->items[n].magic & PATHSPEC_ICASE)\n \t\t\titem_len = pathspec->items[n].prefix;\n \t\telse\n \t\t\titem_len = pathspec->items[n].nowildcard_len;\n-\t\twhile (i < item_len && (n == 0 || i < max)) {\n+\t\twhile (i < item_len && (n == first || i < max)) {\n \t\t\tchar c = pathspec->items[n].match[i];\n-\t\t\tif (c != pathspec->items[0].match[i])\n+\t\t\tif (c != pathspec->items[first].match[i])\n \t\t\t\tbreak;\n \t\t\tif (c == '/')\n \t\t\t\tlen = i + 1;\n \t\t\ti++;\n \t\t}\n-\t\tif (n == 0 || len < max) {\n+\t\tif (n == first || len < max) {\n \t\t\tmax = len;\n \t\t\tif (!max)\n \t\t\t\tbreak;\n \t\t}\n \t}\n+\t*matched_prefix = first < 0 ? NULL : pathspec->items[first].match;\n \treturn max;\n }\n \n /*\n- * Returns a copy of the longest leading path common among all\n- * pathspecs.\n+ * Returns a copy of the longest leading path common among all pathspec\n+ * items that are not excluded.\n */\n char *common_prefix(const struct pathspec *pathspec)\n {\n-\tunsigned long len = common_prefix_len(pathspec);\n+\tconst char *matched_prefix;\n+\tsize_t len = common_prefix_len(pathspec, &matched_prefix);\n \n-\treturn len ? xmemdupz(pathspec->items[0].match, len) : NULL;\n+\treturn len ? xmemdupz(matched_prefix, len) : NULL;\n }\n \n int fill_directory(struct dir_struct *dir,\n \t\t struct index_state *istate,\n \t\t const struct pathspec *pathspec)\n {\n-\tconst char *prefix;\n+\tconst char *matched_prefix;\n \tsize_t prefix_len;\n \n \tunsigned exclusive_flags = DIR_SHOW_IGNORED | DIR_SHOW_IGNORED_TOO;\n@@ -284,11 +289,11 @@ int fill_directory(struct dir_struct *dir,\n \t * Calculate common prefix for the pathspec, and\n \t * use that to optimize the directory walk\n \t */\n-\tprefix_len = common_prefix_len(pathspec);\n-\tprefix = prefix_len ? pathspec->items[0].match : \"\";\n+\tprefix_len = common_prefix_len(pathspec, &matched_prefix);\n \n \t/* Read the directory and prune it */\n-\tread_directory(dir, istate, prefix, prefix_len, pathspec);\n+\tread_directory(dir, istate, prefix_len ? matched_prefix : \"\",\n+\t\t prefix_len, pathspec);\n \n \treturn prefix_len;\n }\n@@ -394,7 +399,7 @@ static int match_pathspec_item(struct index_state *istate,\n \n \t/*\n \t * The normal call pattern is:\n-\t * 1. prefix = common_prefix_len(ps);\n+\t * 1. prefix = common_prefix_len(ps, &matched_prefix);\n \t * 2. prune something, or fill_directory\n \t * 3. match_pathspec()\n \t *\n@@ -414,8 +419,8 @@ static int match_pathspec_item(struct index_state *istate,\n \t * Normally the caller (common_prefix_len() in fact) does\n \t * _exact_ matching on name[-prefix+1..-1] and we do not need\n \t * to check that part. Be defensive and check it anyway, in\n-\t * case common_prefix_len is changed, or a new caller is\n-\t * introduced that does not use common_prefix_len.\n+\t * case common_prefix_len() is changed, or a new caller is\n+\t * introduced that does not use common_prefix_len().\n \t *\n \t * If the penalty turns out too high when prefix is really\n \t * long, maybe change it to\ndiff --git a/t/unit-tests/u-dir.c b/t/unit-tests/u-dir.c\nindex 2d0adaa39e..a3442c3d3c 100644\n--- a/t/unit-tests/u-dir.c\n+++ b/t/unit-tests/u-dir.c\n@@ -45,3 +45,31 @@ void test_dir__within_depth(void)\n \n \n }\n+\n+void test_dir__common_prefix_skips_excluded_pathspec_items(void)\n+{\n+\tstruct pathspec_item items[] = {\n+\t\t{\n+\t\t\t.match = \"unrelated/path\",\n+\t\t\t.magic = PATHSPEC_EXCLUDE,\n+\t\t\t.nowildcard_len = 14,\n+\t\t},\n+\t\t{\n+\t\t\t.match = \"foo/bar\",\n+\t\t\t.nowildcard_len = 7,\n+\t\t},\n+\t\t{\n+\t\t\t.match = \"foo/baz\",\n+\t\t\t.nowildcard_len = 7,\n+\t\t},\n+\t};\n+\tstruct pathspec pathspec = {\n+\t\t.nr = ARRAY_SIZE(items),\n+\t\t.magic = PATHSPEC_EXCLUDE,\n+\t\t.items = items,\n+\t};\n+\tchar *prefix = common_prefix(&pathspec);\n+\n+\tcl_assert_equal_s(prefix, \"foo/\");\n+\tfree(prefix);\n+}\n-- \n2.55.0\n\n"},{"id":"552791","messageId":"xmqq5x05xjr4.fsf@gitster.g","threadId":"66254","inReplyTo":"7CB757FB-1F2D-4EE6-8C31-8C2CD6D42397@ytausch.de","subject":"Re: [PATCH v4 0/2] dir: fix pathspec prefixes with exclusions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-09-16T16:07:27Z","receivedAt":"2026-09-16T16:07:30Z","isPatch":true,"body":"Yannik Tausch writes:\n\n> Pathspec prefix optimization must account for exclude items separately.\n> The prefix is derived from non-exclude items, so applying it while\n> matching an exclude item can compare the wrong portions of the paths.\n> Conversely, an exclude item at the start of the pathspec currently\n> prevents finding a common prefix among the remaining items.\n> ...\n> Yannik Tausch (2):\n> dir: do not apply prefix to negative pathspecs\n> dir: preserve pathspec prefix optimization with leading excludes\n\nThnaks. This round looks ready for 'next'.\n\n\n"}]}