Re: [PATCH 2/6] dir.c::match_basename(): pay attention to the length of string parameters
On Thu, Mar 28, 2013 at 05:47:28PM -0400, Jeff King wrote: From: Junio C Hamano gits...@pobox.com The function takes two counted strings (basename, basenamelen and pattern, patternlen) as parameters, together with prefix (the length of the prefix in pattern that is to be matched literally without globbing against the basename) and EXC_* flags that tells it how to match the pattern against the basename. However, it did not pay attention to the length of these counted strings. Update them to do the following: * When the entire pattern is to be matched literally, the pattern matches the basename only when the lengths of them are the same, and they match up to that length. Hrm. Though the tip of this series passes all tests, this one actually breaks bisectability. What happens is that the existing code passes: path=foo/ pathlen=4 pattern=foo/ patternlen=3 match_basename is happy to compare foo/ to foo/ and realize they're equal. With this change, we compare foo to foo/ and do not match. It isn't until the later patch where you start passing pathlen=3 that it works again. I wonder if it is worth reordering the series to put the path_matches fix first. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/6] dir.c::match_basename(): pay attention to the length of string parameters
On Thu, Mar 28, 2013 at 06:40:27PM -0400, Jeff King wrote: On Thu, Mar 28, 2013 at 05:47:28PM -0400, Jeff King wrote: From: Junio C Hamano gits...@pobox.com The function takes two counted strings (basename, basenamelen and pattern, patternlen) as parameters, together with prefix (the length of the prefix in pattern that is to be matched literally without globbing against the basename) and EXC_* flags that tells it how to match the pattern against the basename. However, it did not pay attention to the length of these counted strings. Update them to do the following: * When the entire pattern is to be matched literally, the pattern matches the basename only when the lengths of them are the same, and they match up to that length. Hrm. Though the tip of this series passes all tests, this one actually breaks bisectability. What happens is that the existing code passes: Ugh. That is a problem, but this series does _not_ pass all tests. I think I failed to run the complete test suite on the correct tip. My match_pathspec fix breaks at least t1011. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/6] dir.c::match_basename(): pay attention to the length of string parameters
Jeff King p...@peff.net writes: On Thu, Mar 28, 2013 at 06:40:27PM -0400, Jeff King wrote: On Thu, Mar 28, 2013 at 05:47:28PM -0400, Jeff King wrote: From: Junio C Hamano gits...@pobox.com The function takes two counted strings (basename, basenamelen and pattern, patternlen) as parameters, together with prefix (the length of the prefix in pattern that is to be matched literally without globbing against the basename) and EXC_* flags that tells it how to match the pattern against the basename. However, it did not pay attention to the length of these counted strings. Update them to do the following: * When the entire pattern is to be matched literally, the pattern matches the basename only when the lengths of them are the same, and they match up to that length. Hrm. Though the tip of this series passes all tests, this one actually breaks bisectability. What happens is that the existing code passes: Ugh. That is a problem, but this series does _not_ pass all tests. I think I failed to run the complete test suite on the correct tip. My match_pathspec fix breaks at least t1011. Yeah, the tip of 'jch' (slightly ahead of 'next' that I use myself) has 0003, 1011 and 3001 broken X-. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/6] dir.c::match_basename(): pay attention to the length of string parameters
On Fri, Mar 29, 2013 at 5:49 AM, Jeff King p...@peff.net wrote: My match_pathspec fix breaks at least t1011. Haven't looked closely at the series, but I suspect you need this http://article.gmane.org/gmane.comp.version-control.git/219008 -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/6] dir.c::match_basename(): pay attention to the length of string parameters
On Fri, Mar 29, 2013 at 4:47 AM, Jeff King p...@peff.net wrote:
+static int fnmatch_icase_mem(const char *pattern, int patternlen,
+const char *string, int stringlen,
+int flags)
+{
+ int match_status;
+ struct strbuf pat_buf = STRBUF_INIT;
+ struct strbuf str_buf = STRBUF_INIT;
+ const char *use_pat = pattern;
+ const char *use_str = string;
+
+ if (pattern[patternlen]) {
+ strbuf_add(pat_buf, pattern, patternlen);
+ use_pat = pat_buf.buf;
+ }
+ if (string[stringlen]) {
+ strbuf_add(str_buf, string, stringlen);
+ use_str = str_buf.buf;
+ }
+
+ match_status = fnmatch_icase(use_pat, use_str, 0);
You should pass flags in here instead of 0.
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/6] dir.c::match_basename(): pay attention to the length of string parameters
On Fri, Mar 29, 2013 at 08:25:00AM +0700, Nguyen Thai Ngoc Duy wrote:
On Fri, Mar 29, 2013 at 4:47 AM, Jeff King p...@peff.net wrote:
+static int fnmatch_icase_mem(const char *pattern, int patternlen,
+const char *string, int stringlen,
+int flags)
+{
+ int match_status;
+ struct strbuf pat_buf = STRBUF_INIT;
+ struct strbuf str_buf = STRBUF_INIT;
+ const char *use_pat = pattern;
+ const char *use_str = string;
+
+ if (pattern[patternlen]) {
+ strbuf_add(pat_buf, pattern, patternlen);
+ use_pat = pat_buf.buf;
+ }
+ if (string[stringlen]) {
+ strbuf_add(str_buf, string, stringlen);
+ use_str = str_buf.buf;
+ }
+
+ match_status = fnmatch_icase(use_pat, use_str, 0);
You should pass flags in here instead of 0.
Eek, yeah, that's obviously wrong. Thanks for catching it. Fixing that
clears up all of the test failures outside of t5002.
And if you move patch 5 (special case paths that end with a slash)
into position 2, it cleans up the mid-series failures of t5002, making
the series clean for later bisecting.
Thanks for looking it over.
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
