git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 2/4] dir.c::match_basename(): pay attention to the length of string parameters

From
Jeff King <peff@peff.net>
Date
Mar 26, 2013, 18:55 UTC
Message-ID
<20130326185559.GB26462@sigill.intra.peff.net>
In-Reply-To
<1364323171-20299-3-git-send-email-gitster@pobox.com>
On Tue, Mar 26, 2013 at 11:39:29AM -0700, Junio C Hamano wrote:
Show 21 quoted lines
> 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.
> 
>  * When the pattern is "*" followed by a string to be matched
>    literally, make sure that the basenamelen is equal or longer than
>    the "literal" part of the pattern, and the tail of the basename
>    string matches that literal part.
> 
>  * Otherwise, make sure we use only the counted part of the strings
>    when calling fnmatch_icase().  Because these counted strings are
>    full strings most of the time, avoid unnecessary allocation.

I think this is OK, with the intention that we would eventually drop the allocations from your third bullet point in favor of using a byte-counted version of fnmatch (i.e., nwildmatch). But until then we're going to see a performance drop.

The pattern is usually going to be NUL-terminated at the length counter, but every time we feed a directory, it's going to run into this allocation. And we do it once for _every_ directory against _every_ wildcard gitignore pattern. So I think it is probably going to be measurable. I guess we can try measuring it on something like WebKit, which has plenty of both directories and gitattributes.

-Peff
Previous: Junio C HamanoNext: Jeff King
Message 13 of 66 in “[regression?] trailing slash required in .gitattributes”
  1. Jeff KingMar 19, 2013
  2. Junio C HamanoMar 19, 2013
  3. Jeff KingMar 19, 2013
  4. Jeff KingMar 22, 2013
  5. Junio C HamanoMar 22, 2013
  6. Jeff KingMar 23, 2013
  7. Junio C HamanoMar 24, 2013
  8. 0/4 attribute regression fix for maint-1.8.1 and upwardJunio C Hamano, Mar 26, 2013
  9. 1/4 attr.c::path_matches(): the basename is part of the pathnameJunio C Hamano, Mar 26, 2013
  10. Jeff KingMar 26, 2013
  11. Duy NguyenMar 27, 2013
  12. 2/4 dir.c::match_basename(): pay attention to the length of string parametersJunio C Hamano, Mar 26, 2013
  13. Jeff KingMar 26, 2013
  14. Jeff KingMar 26, 2013
  15. Junio C HamanoMar 26, 2013
  16. Jeff KingMar 26, 2013
  17. Junio C HamanoMar 26, 2013
  18. Jeff KingMar 27, 2013
  19. 3/4 attr.c::path_matches(): special case paths that end with a slashJunio C Hamano, Mar 26, 2013
  20. Jeff KingMar 26, 2013
  21. Jeff KingMar 26, 2013
  22. Duy NguyenMar 27, 2013
  23. Jeff KingMar 28, 2013
  24. 4/4 make sure a pattern without trailing slash matches a directoryJunio C Hamano, Mar 26, 2013
  25. Jeff KingMar 26, 2013
  26. Duy NguyenMar 27, 2013
  27. Junio C HamanoMar 27, 2013
  28. Duy NguyenMar 27, 2013
  29. 0/6 attribute regression fix for maint-1.8.1 and upwardJeff King, Mar 28, 2013
  30. 1/6 attr.c::path_matches(): the basename is part of the pathnameJeff King, Mar 28, 2013
  31. 2/6 dir.c::match_basename(): pay attention to the length of string parametersJeff King, Mar 28, 2013
  32. Jeff KingMar 28, 2013
  33. Jeff KingMar 28, 2013
  34. Junio C HamanoMar 28, 2013
  35. Duy NguyenMar 28, 2013
  36. Duy NguyenMar 29, 2013
  37. Jeff KingMar 29, 2013
  38. Junio C HamanoMar 29, 2013
  39. 3/6 dir.c::match_pathname(): adjust patternlen when shifting patternJeff King, Mar 28, 2013
  40. 4/6 dir.c::match_pathname(): pay attention to the length of string parametersJeff King, Mar 28, 2013
  41. Junio C HamanoMar 28, 2013
  42. Duy NguyenMar 29, 2013
  43. Duy NguyenMar 29, 2013
  44. Torsten BögershausenMar 29, 2013
  45. Duy NguyenMar 29, 2013
  46. Jeff KingMar 29, 2013
  47. Duy NguyenMar 29, 2013
  48. Junio C HamanoMar 29, 2013
  49. Jeff KingMar 29, 2013
  50. Junio C HamanoMar 29, 2013
  51. Jeff KingMar 29, 2013
  52. Duy NguyenMar 30, 2013
  53. 5/6 attr.c::path_matches(): special case paths that end with a slashJeff King, Mar 28, 2013
  54. 6/6 t: check that a pattern without trailing slash matches a directoryJeff King, Mar 28, 2013
  55. Eric SunshineMar 28, 2013
  56. Jeff KingMar 28, 2013
  57. Duy NguyenMar 23, 2013
  58. Duy NguyenMar 23, 2013
  59. 0/4 attr directory matching regressionNguyễn Thái Ngọc Duy, Mar 25, 2013
  60. 1/4 wildmatch: do not require "text" to be NUL-terminatedNguyễn Thái Ngọc Duy, Mar 25, 2013
  61. 2/4 attr.c: fix pattern{,len} inconsistency in struct match_attrNguyễn Thái Ngọc Duy, Mar 25, 2013
  62. 3/4 dir.c: make match_{base,path}name respect {basename,path}lenNguyễn Thái Ngọc Duy, Mar 25, 2013
  63. 4/4 attr.c: fix matching "subdir" without the trailing slashNguyễn Thái Ngọc Duy, Mar 25, 2013
  64. Duy NguyenMar 25, 2013
  65. Duy NguyenMar 25, 2013
  66. Junio C HamanoMar 26, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.