summaryrefslogtreecommitdiff
path: root/grep.c
Commit message (Collapse)AuthorAgeFilesLines
* Merge branch 'rs/grep-expr-cleanup'Junio C Hamano2022-02-051-34/+36
|\ | | | | | | | | | | | | | | | | | | Code clean-up. * rs/grep-expr-cleanup: grep: use grep_and_expr() in compile_pattern_and() grep: extract grep_binexp() from grep_or_expr() grep: use grep_not_expr() in compile_pattern_not() grep: use grep_or_expr() in compile_pattern_or()
| * grep: use grep_and_expr() in compile_pattern_and()Taylor Blau2022-01-061-6/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | In a similar spirit as a previous commit, use the new `grep_and_expr()` to construct the AND node in `compile_pattern_and()`. Unlike the aforementioned previous commit, this is not about code duplication, since this is the only spot in grep.c where an AND node is constructed. Rather, this is about visual consistency with the other `compile_pattern_xyz()` functions. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * grep: extract grep_binexp() from grep_or_expr()Taylor Blau2022-01-061-2/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | When constructing an OR node, the grep.c code uses `grep_or_expr()` to make a node, assign its kind, and set its left and right children. The same is not done for AND nodes. Prepare to introduce a new `grep_and_expr()` function which will share code with the existing implementation of `grep_or_expr()` by introducing a new function which compiles either kind of binary expression, and reimplement `grep_or_expr()` in terms of it. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * grep: use grep_not_expr() in compile_pattern_not()René Scharfe2022-01-061-13/+11
| | | | | | | | | | | | | | | | Move the definition of grep_not_expr() up and use this function in compile_pattern_not() to simplify the code and reduce duplication. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * grep: use grep_or_expr() in compile_pattern_or()René Scharfe2022-01-061-15/+11
| | | | | | | | | | | | | | | | Move the definition of grep_or_expr() up and use this function in compile_pattern_or() to reduce code duplication. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'lh/use-gnu-color-in-grep'Junio C Hamano2022-01-101-3/+3
|\ \ | |/ |/| | | | | | | | | | | The color palette used by "git grep" has been updated to match that of GNU grep. * lh/use-gnu-color-in-grep: grep: align default colors with GNU grep ones
| * grep: align default colors with GNU grep onesLénaïc Huard2022-01-051-3/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | git-grep shares a lot of options with the standard grep tool. Like GNU grep, it has coloring options to highlight the matching text. And like it, it has options to customize the various colored parts. This patch updates the default git-grep colors to make them match the GNU grep default ones [1]. It was possible to get the same result by setting the various `color.grep.<slot>` options, but this patch makes `git grep --color` share the same color scheme as `grep --color` by default without any user configuration. [1] https://www.man7.org/linux/man-pages/man1/grep.1.html#ENVIRONMENT Signed-off-by: Lénaïc Huard <lenaic@lhuard.fr> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'rs/pcre2-utf'Junio C Hamano2022-01-051-2/+2
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | "git grep --perl-regexp" failed to match UTF-8 characters with wildcard when the pattern consists only of ASCII letters, which has been corrected. * rs/pcre2-utf: grep/pcre2: factor out literal variable grep/pcre2: use PCRE2_UTF even with ASCII patterns
| * | grep/pcre2: factor out literal variableRené Scharfe2021-12-201-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | Patterns that contain no wildcards and don't have to be case-folded are literal. Give this condition a name to increase the readability of the boolean expression for enabling the option PCRE2_UTF. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | grep/pcre2: use PCRE2_UTF even with ASCII patternsRené Scharfe2021-12-201-1/+1
| |/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | compile_pcre2_pattern() currently uses the option PCRE2_UTF only for patterns with non-ASCII characters. Patterns with ASCII wildcards can match non-ASCII strings, though. Without that option PCRE2 mishandles UTF-8 input, though -- it matches parts of multi-byte characters. Fix that by using PCRE2_UTF even for ASCII-only patterns. This is a remake of the reverted ae39ba431a (grep/pcre2: fix an edge case concerning ascii patterns and UTF-8 data, 2021-10-15). The change to the condition and the test are simplified and more targeted. Original-patch-by: Hamza Mahfooz <someguy@effective-light.com> Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | log: let --invert-grep only invert --grepRené Scharfe2021-12-171-3/+19
|/ | | | | | | | | | | | | | | | | | | | | The option --invert-grep is documented to filter out commits whose messages match the --grep filters. However, it also affects the header matches (--author, --committer), which is not intended. Move the handling of that option to grep.c, as only the code there can distinguish between matches in the header from those in the message body. If --invert-grep is given then enable extended expressions (not the regex type, we just need git grep's --not to work), negate the body patterns and check if any of them match by piggy-backing on the collect_hits mechanism of grep_source_1(). Collecting the matches in struct grep_opt is a bit iffy, but with "last_shown" we have a precedent for writing state information to that struct. Reported-by: Dotan Cohen <dotancohen@gmail.com> Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Revert "grep/pcre2: fix an edge case concerning ascii patterns and UTF-8 data"Junio C Hamano2021-11-191-4/+2
| | | | | | | | This reverts commit ae39ba431ab861548eb60b4bd2e1d8b8813db76f, as it breaks "grep" when looking for a string in non UTF-8 haystack, when linked with certain versions of PCREv2 library. Signed-off-by: Junio C Hamano <gitster@pobox.com>
* grep/pcre2: fix an edge case concerning ascii patterns and UTF-8 dataHamza Mahfooz2021-10-151-2/+4
| | | | | | | | | | | | | | | If we attempt to grep non-ascii log message text with an ascii pattern, we run into the following issue: $ git log --color --author='.var.*Bjar' -1 origin/master | grep ^Author grep: (standard input): binary file matches So, to fix this teach the grep code to use PCRE2_UTF, as long as the log output is encoded in UTF-8. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Hamza Mahfooz <someguy@effective-light.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* grep: refactor next_match() and match_one_pattern() for external useHamza Mahfooz2021-09-291-30/+49
| | | | | | | | | | | | | | | | | These changes are made in preparation of, the colorization support for the "git log" subcommands that, rely on regex functionality (i.e. "--author", "--committer" and "--grep"). These changes are necessary primarily because match_one_pattern() expects header lines to be prefixed, however, in pretty, the prefixes are stripped from the lines because the name-email pairs need to go through additional parsing, before they can be printed and because next_match() doesn't handle the case of "ctx == GREP_CONTEXT_HEAD" at all. So, teach next_match() how to handle the new case and move match_one_pattern()'s core logic to headerless_match_one_pattern() while preserving match_one_pattern()'s uses that depend on the additional processing. Signed-off-by: Hamza Mahfooz <someguy@effective-light.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* grep: store grep_source buffer as constJeff King2021-09-221-3/+6
| | | | | | | | | | | | | | | | | | | | | | | | Our grep_buffer() function takes a non-const buffer, which is confusing: we don't take ownership of nor write to the buffer. This mostly comes from the fact that the underlying grep_source struct in which we store the buffer uses non-const pointer. The memory pointed to by the struct is sometimes owned by us (for FILE or OID sources), and sometimes not (for BUF sources). Let's store it as const, which lets us err on the side of caution (i.e., the compiler will warn us if any of our code writes to or tries to free it). As a result, we must annotate the one place where we do free it by casting away the constness. But that's a small price to pay for the extra safety and clarity elsewhere (and indeed, it already had a comment explaining why GREP_SOURCE_BUF _didn't_ free it). And then we can mark grep_buffer() as taking a const buffer. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* grep: mark "haystack" buffers as constJeff King2021-09-221-26/+35
| | | | | | | | | | | | | | | | | When we're grepping in a buffer, we don't need to modify it. So we can take "const char *" buffers, rather than "char *". This can avoid some awkward casts in our callers, and make our expectations more clear (we will not take ownership of the memory, nor will we ever write to it). These spots don't all necessarily have to be converted at the same time, but some of them are dependent on others, because we pass pointers-to-pointers in a few cases. And none of this should change any behavior, since we're just adding "const" qualifiers (and likewise, the compiler will let us know if we missed any spots). So it's relatively low-risk to just do this all at once. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* grep: stop modifying buffer in grep_source_1()Jeff King2021-09-221-4/+1
| | | | | | | | | | | | | | | We find the end of each matching line of a buffer, and then temporarily write a NUL to turn it into a regular C string. But we don't need to do so, because the only thing we do in the interim is pass the line and its length (via an "eol" pointer) to match_line(). And that function should only look at the bytes we passed it, whether it has a terminating NUL or not. We can drop this temporary write in order to simplify the code and make it easier to use const buffers in more of grep.c. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* grep: stop modifying buffer in show_line()Jeff King2021-09-221-3/+0
| | | | | | | | | | | | | | | | | | | | | | When showing lines via grep (or looking for funcnames), we call show_line() on a multi-line buffer. It finds the end of line and marks it with a NUL. However, we don't need to do so, as the resulting line is only used along with its "eol" marker: - we pass both to next_match(), which takes care to look at only the bytes we specified - we pass the line to output_color() without its matching eol marker. However, we do use the "match" struct we got from next_match() to tell it how many bytes to look at (which can never exceed the string we passed it). So we can stop setting and restoring this NUL marker. That makes the code simpler, and will allow us to take a const buffer in a future patch. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* grep: stop modifying buffer in strip_timestampJeff King2021-09-221-10/+3
| | | | | | | | | | | | | | | | | | | | When grepping for headers in commit objects, we receive individual lines (e.g., "author Name <email> 1234 -0000"), and then strip off the timestamp to do our match. We do so by writing a NUL byte over the whitespace separator, and then remembering to restore it later. We had to do it this way when this was added back in a4d7d2c6db (log --author/--committer: really match only with name part, 2008-09-04), because we fed the result directly to regexec(), which expects a NUL-terminated string. But since b7d36ffca0 (regex: use regexec_buf(), 2016-09-21), we have a function which can match part of a buffer. So instead of modifying the string, we can instead just move the "eol" pointer, and the rest of the code will do the right thing. This will let further patches mark more buffers as "const". Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* grep: add repository to OID grep sourcesJonathan Tan2021-09-081-2/+5
| | | | | | | | | | Record the repository whenever an OID grep source is created, and teach the worker threads to explicitly provide the repository when accessing objects. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Reviewed-by: Matheus Tavares <matheus.bernardino@usp.br> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* grep: typesafe versions of grep_source_initJonathan Tan2021-09-081-18/+28
| | | | | | | | | | | | | grep_source_init() can create "struct grep_source" objects and, depending on the value of the type passed, some void-pointer parameters have different meanings. Because one of these types (GREP_SOURCE_OID) will require an additional parameter in a subsequent patch, take the opportunity to increase clarity and type safety by replacing this function with individual functions for each type. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Reviewed-by: Matheus Tavares <matheus.bernardino@usp.br> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'rs/grep-parser-fix'Junio C Hamano2021-07-131-0/+2
|\ | | | | | | | | | | | | | | "git grep --and -e foo" ought to have been diagnosed as an error but instead segfaulted, which has been corrected. * rs/grep-parser-fix: grep: report missing left operand of --and
| * grep: report missing left operand of --andRené Scharfe2021-06-301-0/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Git grep allows combining two patterns with --and. It checks and reports if the second pattern is missing when compiling the expression. A missing first pattern, however, is only reported later at match time. Thus no error is returned if no matching is done, e.g. because no file matches the also given pathspec. When that happens we get an expression tree with an GREP_NODE_AND node and a NULL pointer to the missing left child. free_pattern_expr() tries to dereference it during the cleanup at the end, which results in a segmentation fault. Fix this by verifying the presence of the left operand at expression compilation time. Reported-by: Matthew Hughes <matthewhughes934@gmail.com> Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | hash: provide per-algorithm null OIDsbrian m. carlson2021-04-271-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Up until recently, object IDs did not have an algorithm member, only a hash. Consequently, it was possible to share one null (all-zeros) object ID among all hash algorithms. Now that we're going to be handling objects from multiple hash algorithms, it's important to make sure that all object IDs have a correct algorithm field. Introduce a per-algorithm null OID, and add it to struct hash_algo. Introduce a wrapper function as well, and use it everywhere we used to use the null_oid constant. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'ab/grep-pcre2-allocfix'Junio C Hamano2021-03-221-56/+43
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Updates to memory allocation code around the use of pcre2 library. * ab/grep-pcre2-allocfix: grep/pcre2: move definitions of pcre2_{malloc,free} grep/pcre2: move back to thread-only PCREv2 structures grep/pcre2: actually make pcre2 use custom allocator grep/pcre2: use pcre2_maketables_free() function grep/pcre2: use compile-time PCREv2 version test grep/pcre2: add GREP_PCRE2_DEBUG_MALLOC debug mode grep/pcre2: prepare to add debugging to pcre2_malloc() grep/pcre2: correct reference to grep_init() in comment grep/pcre2: drop needless assignment to NULL grep/pcre2: drop needless assignment + assert() on opt->pcre2
| * | grep/pcre2: move definitions of pcre2_{malloc,free}Ævar Arnfjörð Bjarmason2021-02-171-24/+22
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Move the definitions of the pcre2_{malloc,free} functions above the compile_pcre2_pattern() function they're used in. Before the preceding commit they used to be needed earlier, but now we can move them to be adjacent to the other PCREv2 functions. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | grep/pcre2: move back to thread-only PCREv2 structuresÆvar Arnfjörð Bjarmason2021-02-171-26/+15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Change the setup of the "pcre2_general_context" to happen per-thread in compile_pcre2_pattern() instead of in grep_init(). This change brings it in line with how the rest of the pcre2_* members in the grep_pat structure are set up. As noted in the preceding commit the approach 513f2b0bbd4 (grep: make PCRE2 aware of custom allocator, 2019-10-16) took to allocate the pcre2_general_context seems to have been initially based on a misunderstanding of how PCREv2 memory allocation works. The approach of creating a global context in grep_init() is just added complexity for almost zero gain. On my system it's 24 bytes saved per-thread. For comparison PCREv2 will then go on to allocate at least a kilobyte for its own thread-local state. As noted in 6d423dd542f (grep: don't redundantly compile throwaway patterns under threading, 2017-05-25) the grep code is intentionally not trying to micro-optimize allocations by e.g. sharing some PCREv2 structures globally, while making others thread-local. So let's remove this special case and make all of them thread-local again for simplicity. With this change we could move the pcre2_{malloc,free} functions around to live closer to their current use. I'm not doing that here to keep this change small, that cleanup will be done in a follow-up commit. See also the discussion in 94da9193a6 (grep: add support for PCRE v2, 2017-06-01) about thread safety, and Johannes's comments[1] to the effect that we should be doing what this patch is doing. 1. https://lore.kernel.org/git/nycvar.QRO.7.76.6.1908052120302.46@tvgsbejvaqbjf.bet/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | grep/pcre2: actually make pcre2 use custom allocatorÆvar Arnfjörð Bjarmason2021-02-171-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Continue work started in 513f2b0bbd4 (grep: make PCRE2 aware of custom allocator, 2019-10-16) and make PCREv2 use our pcre2_{malloc,free}(). functions for allocation. We'll now use it for all PCREv2 allocations. The reason 513f2b0bbd4 worked as a bugfix for the USE_NED_ALLOCATOR issue is because it targeted the allocation freed via free(), as opposed to by a pcre2_*free() function. I.e. the pcre2_maketables() and pcre2_maketables_free() pair. For most of the rest we continued allocating with stock malloc() inside PCREv2 itself, but didn't segfault because we'd use its corresponding free(). In a preceding commit of mine I changed the free() to pcre2_maketables_free() on versions of PCREv2 10.34 and newer. So as far as fixing the segfault goes we could revert 513f2b0bbd4. But then we wouldn't use the desired allocator, let's just use it instead. Before this patch we'd on e.g.: grep --threads=1 -iP æ.*var.*xyz Only use pcre2_{malloc,free}() for 2 malloc() calls and 2 corresponding free() calls. Now it's 12 calls to each. This can be observed with the GREP_PCRE2_DEBUG_MALLOC debug mode. Reading the history of how this bug got introduced it wasn't present in Johannes's original patch[1] to fix the issue. My reading of that thread is that the approach the follow-up patches to Johannes's original pursued were based on misunderstanding of how the PCREv2 API works. In particular this part of [2]: "most of the time (like when using UTF-8) the chartable (and therefore the global context) is not needed (even when using alternate allocators)" That's simply not how PCREv2 memory allocation works. It's easy to see how the misunderstanding came about. It's because (as noted above) the issue was noticed because of our use of free() in our own grep.c for freeing the memory allocated by pcre2_maketables(). Thus the misunderstanding that PCREv2's compile context is something only needed for pcre2_maketables(), and e.g. an aborted earlier attempt[3] to only set it up when we ourselves called pcre2_maketables(). That's not what PCREv2's compile context is. To quote PCREv2's documentation: "This context just contains pointers to (and data for) external memory management functions that are called from several places in the PCRE2 library." Thus the failed attempts to go down the route of only creating the general context in cases where we ourselves call pcre2_maketables(), before finally settling on the approach 513f2b0bbd4 took of always creating it, but then mostly not using it. Instead we should always create it, and then pass the general context to those functions that accept it, so that they'll consistently use our preferred memory allocation functions. 1. https://lore.kernel.org/git/3397e6797f872aedd18c6d795f4976e1c579514b.1565005867.git.gitgitgadget@gmail.com/ 2. https://lore.kernel.org/git/CAPUEsphMh_ZqcH3M7PXC9jHTfEdQN3mhTAK2JDkdvKBp53YBoA@mail.gmail.com/ 3. https://lore.kernel.org/git/20190806085014.47776-3-carenas@gmail.com/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | grep/pcre2: use pcre2_maketables_free() functionÆvar Arnfjörð Bjarmason2021-02-171-0/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Make use of the pcre2_maketables_free() function to free the memory allocated by pcre2_maketables(). At first sight it's strange that 10da030ab75 (grep: avoid leak of chartables in PCRE2, 2019-10-16) which added the free() call here doesn't make use of the pcre2_free() the author introduced in the preceding commit in 513f2b0bbd4 (grep: make PCRE2 aware of custom allocator, 2019-10-16). The reason is that at the time the function didn't exist. It was first introduced in PCREv2 version 10.34, released on 2019-11-21. Let's make use of it behind a macro. I don't think this matters for anything to do with custom allocators, but it makes our use of PCREv2 more discoverable. At some distant point in the future we'll be able to drop the version guard, as nobody will be running a version older than 10.34. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | grep/pcre2: use compile-time PCREv2 version testÆvar Arnfjörð Bjarmason2021-02-171-14/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Replace a use of pcre2_config(PCRE2_CONFIG_VERSION, ...) which I added in 95ca1f987ed (grep/pcre2: better support invalid UTF-8 haystacks, 2021-01-24) with the same test done at compile-time. It might be cuter to do this at runtime since we don't have to do the "major >= 11 || (major >= 10 && ...)" test. But in the next commit we'll add another version comparison that absolutely needs to be done at compile-time, so we're better of being consistent across the board. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | grep/pcre2: add GREP_PCRE2_DEBUG_MALLOC debug modeÆvar Arnfjörð Bjarmason2021-02-171-0/+10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add optional printing of PCREv2 allocations to stderr for a developer who manually changes the GREP_PCRE2_DEBUG_MALLOC definition to "1". You need to manually change the definition in the source file similar to the DEBUG_MAILMAP, there's no Makefile knob for this. This will be referenced a subsequent commit, and is generally useful to manually see what's going on with PCREv2 allocations while working on that code. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | grep/pcre2: prepare to add debugging to pcre2_malloc()Ævar Arnfjörð Bjarmason2021-02-171-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Change pcre2_malloc() in a way that'll make it easier for a debugging fprintf() to spew out the allocated pointer. This doesn't introduce any functional change, it just makes a subsequent commit's diff easier to read. Changes code added in 513f2b0bbd4 (grep: make PCRE2 aware of custom allocator, 2019-10-16). Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | grep/pcre2: correct reference to grep_init() in commentÆvar Arnfjörð Bjarmason2021-02-171-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Correct a comment added in 513f2b0bbd4 (grep: make PCRE2 aware of custom allocator, 2019-10-16). This comment was never correct in git.git, but was consistent with an older version of the patch[1]. 1. https://lore.kernel.org/git/20190806163658.66932-3-carenas@gmail.com/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | grep/pcre2: drop needless assignment to NULLÆvar Arnfjörð Bjarmason2021-02-171-2/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Remove a redundant assignment of pcre2_compile_context dating back to my 94da9193a6e (grep: add support for PCRE v2, 2017-06-01). In create_grep_pat() we xcalloc() the "grep_pat" struct, so there's no need to NULL out individual members here. I think this was probably something left over from an earlier development version of mine. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | grep/pcre2: drop needless assignment + assert() on opt->pcre2Ævar Arnfjörð Bjarmason2021-02-171-3/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Drop an assignment added in b65abcafc7a (grep: use PCRE v2 for optimized fixed-string search, 2019-07-01) and the overly cautious assert() I added in 94da9193a6e (grep: add support for PCRE v2, 2017-06-01). There was never a good reason for this, it's just a relic from when I initially wrote the PCREv2 support. We're not going to have confusion about compile_pcre2_pattern() being called when it shouldn't just because we forgot to cargo-cult this opt->pcre2 option. Furthermore the "struct grep_opt" is (mostly) used for the options the user supplied, let's avoid the pattern of needlessly assigning to it. With my recent removal of the PCREv1 backend in 7599730b7e2 (Remove support for v1 of the PCRE library, 2021-01-24) there's even less confusion around what we call where in these codepaths, which is one more reason to remove this. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | use CALLOC_ARRAYRené Scharfe2021-03-131-4/+4
|/ / | | | | | | | | | | | | | | | | Add and apply a semantic patch for converting code that open-codes CALLOC_ARRAY to use it instead. It shortens the code and infers the element size automatically. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'ab/grep-pcre-invalid-utf8'Junio C Hamano2021-02-101-1/+17
|\ \ | | | | | | | | | | | | | | | | | | | | | Update support for invalid UTF-8 in PCRE2. * ab/grep-pcre-invalid-utf8: grep/pcre2: better support invalid UTF-8 haystacks grep/pcre2 tests: don't rely on invalid UTF-8 data test
| * | grep/pcre2: better support invalid UTF-8 haystacksÆvar Arnfjörð Bjarmason2021-01-241-1/+17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Improve the support for invalid UTF-8 haystacks given a non-ASCII needle when using the PCREv2 backend. This is a more complete fix for a bug I started to fix in 870eea8166 (grep: do not enter PCRE2_UTF mode on fixed matching, 2019-07-26), now that PCREv2 has the PCRE2_MATCH_INVALID_UTF mode we can make use of it. This fixes the sort of case described in 8a5999838e (grep: stess test PCRE v2 on invalid UTF-8 data, 2019-07-26), i.e.: - The subject string is non-ASCII (e.g. "ævar") - We're under a is_utf8_locale(), e.g. "en_US.UTF-8", not "C" - We are using --ignore-case, or we're a non-fixed pattern If those conditions were satisfied and we matched found non-valid UTF-8 data PCREv2 might bark on it, in practice this only happened under the JIT backend (turned on by default on most platforms). Ultimately this fixes a "regression" in b65abcafc7 ("grep: use PCRE v2 for optimized fixed-string search", 2019-07-01), I'm putting that in scare-quotes because before then we wouldn't properly support these complex case-folding, locale etc. cases either, it just broke in different ways. There was a bug related to this the PCRE2_NO_START_OPTIMIZE flag fixed in PCREv2 10.36. It can be worked around by setting the PCRE2_NO_START_OPTIMIZE flag. Let's do that in those cases, and add tests for the bug. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | Merge branch 'ab/retire-pcre1'Junio C Hamano2021-02-101-115/+2
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | The support for deprecated PCRE1 library has been dropped. * ab/retire-pcre1: Remove support for v1 of the PCRE library config.mak.uname: remove redundant NO_LIBPCRE1_JIT flag
| * | | Remove support for v1 of the PCRE libraryÆvar Arnfjörð Bjarmason2021-01-231-117/+2
| |/ / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Remove support for using version 1 of the PCRE library. Its use has been discouraged by upstream for a long time, and it's in a bugfix-only state. Anyone who was relying on v1 in particular got a nudge to move to v2 in e6c531b808 (Makefile: make USE_LIBPCRE=YesPlease mean v2, not v1, 2018-03-11), which was first released as part of v2.18.0. With this the LIBPCRE2 test prerequisites is redundant to PCRE. But I'm keeping it for self-documentation purposes, and to avoid conflict with other in-flight PCRE patches. I'm also not changing all of our own "pcre2" names to "pcre", i.e. the inverse of 6d4b5747f0 (grep: change internal *pcre* variable & function names to be *pcre1*, 2017-05-25). I don't see the point, and it makes the history/blame harder to read. Maybe if there's ever a PCRE v3... Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | grep/log: remove hidden --debug and --grep-debug optionsÆvar Arnfjörð Bjarmason2021-01-261-99/+2
|/ / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Remove the hidden "grep --debug" and "log --grep-debug" options added in 17bf35a3c7b (grep: teach --debug option to dump the parse tree, 2012-09-13). At the time these options seem to have been intended to go along with a documentation discussion and to help the author of relevant tests to perform ad-hoc debugging on them[1]. Reasons to want this gone: 1. They were never documented, and the only (rather trivial) use of them in our own codebase for testing is something I removed back in e01b4dab01e (grep: change non-ASCII -i test to stop using --debug, 2017-05-20). 2. Googling around doesn't show any in-the-wild uses I could dig up, and on the Git ML the only mentions after the original discussion seem to have been when they came up in unrelated diff contexts, or that test commit of mine. 3. An exception to that is c581e4a7499 (grep: under --debug, show whether PCRE JIT is enabled, 2019-08-18) where we added the ability to dump out when PCREv2 has the JIT in effect. The combination of that and my earlier b65abcafc7a (grep: use PCRE v2 for optimized fixed-string search, 2019-07-01) means Git prints this out in its most common in-the-wild configuration: $ git log --grep-debug --grep=foo --grep=bar --grep=baz --all-match pcre2_jit_on=1 pcre2_jit_on=1 pcre2_jit_on=1 [all-match] (or pattern_body<body>foo (or pattern_body<body>bar pattern_body<body>baz ) ) $ git grep --debug \( -e foo --and -e bar \) --or -e baz pcre2_jit_on=1 pcre2_jit_on=1 pcre2_jit_on=1 (or (and patternfoo patternbar ) patternbaz ) I.e. for each pattern we're considering for the and/or/--all-match etc. debugging we'll now diligently spew out another identical line saying whether the PCREv2 JIT is on or not. I think that nobody's complained about that rather glaringly obviously bad output says something about how much this is used, i.e. it's not. The need for this debugging aid for the composed grep/log patterns seems to have passed, and the desire to dump the JIT config seems to have been another one-off around the time we had JIT-related issues on the PCREv2 codepath. That the original author of this debugging facility seemingly hasn't noticed the bad output since then[2] is probably some indicator. 1. https://lore.kernel.org/git/cover.1347615361.git.git@drmicha.warpmail.net/ 2. https://lore.kernel.org/git/xmqqk1b8x0ac.fsf@gitster-ct.c.googlers.com/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | grep: copy struct in one fell swoopMartin Ågren2020-11-301-23/+10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We have a `struct grep_opt` with our defaults which we then copy into the caller's struct. Rather than zeroing the target struct and copying each element one by one, just copy everything at once. This leaves the code simpler and more maintainable. We don't have any ownership issues with what we're copying now and can just greedily copy the whole thing. If and when we do need to handle such elements (`char *`?), we must and can handle it appropriately. Make sure to leave a comment to our future selves. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | grep: use designated initializers for `grep_defaults`Martin Ågren2020-11-211-39/+25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In 15fabd1bbd ("builtin/grep.c: make configuration callback more reusable", 2012-10-09), we learned to fill a `static struct grep_opt grep_defaults` which we can use as a blueprint for other such structs. At the time, we didn't consider designated initializers to be widely useable, but these days, we do. (See, e.g., cbc0f81d96 ("strbuf: use designated initializers in STRBUF_INIT", 2017-07-10).) Use designated initializers to let the compiler set up the struct and so that we don't need to remember to call `init_grep_defaults()`. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | grep: don't set up a "default" repo for grepMartin Ågren2020-11-211-2/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | `init_grep_defaults()` fills a `static struct grep_opt grep_defaults`. This struct is then used by `grep_init()` as a blueprint for other such structs. Notably, `grep_init()` takes a `struct repo *` and assigns it into the target struct. As a result, it is unnecessary for us to take a `struct repo *` in `init_grep_defaults()` as well. We assign it into the default struct and never look at it again. And in light of how we return early if we have already set up the default struct, it's not just unnecessary, but is also a bit confusing: If we are called twice and with different repos, is it a bug or a feature that we ignore the second repo? Drop the repo parameter for `init_grep_defaults()`. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | comment: fix spelling mistakes inside commentsSteve Kemp2020-07-291-1/+1
| | | | | | | | | | | | | | | | This commit fixes a couple of minor spelling mistakes inside comments. Signed-off-by: Steve Kemp <steve@steve.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | grep: replace grep_read_mutex by internal obj read lockMatheus Tavares2020-01-171-20/+19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | git-grep uses 'grep_read_mutex' to protect its calls to object reading operations. But these have their own internal lock now, which ensures a better performance (allowing parallel access to more regions). So, let's remove the former and, instead, activate the latter with enable_obj_read_lock(). Sections that are currently protected by 'grep_read_mutex' but are not internally protected by the object reading lock should be surrounded by obj_read_lock() and obj_read_unlock(). These guarantee mutual exclusion with object reading operations, keeping the current behavior and avoiding race conditions. Namely, these places are: In grep.c: - fill_textconv() at fill_textconv_grep(). - userdiff_get_textconv() at grep_source_1(). In builtin/grep.c: - parse_object_or_die() and the submodule functions at grep_submodule(). - deref_tag() and gitmodules_config_oid() at grep_objects(). If these functions become thread-safe, in the future, we might remove the locking and probably get some speedup. Note that some of the submodule functions will already be thread-safe (or close to being thread-safe) with the internal object reading lock. However, as some of them will require additional modifications to be removed from the critical section, this will be done in its own patch. Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | grep: fix race conditions on userdiff callsMatheus Tavares2020-01-171-1/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | git-grep uses an internal grep_read_mutex to protect object reading operations. Similarly, there's a grep_attr_mutex to protect calls to the gitattributes machinery. However, two of the three functions protected by the last mutex may also perform object reading, as seen below: - userdiff_get_textconv() > notes_cache_init() > notes_cache_match_validity() > lookup_commit_reference_gently() > parse_object() > repo_has_object_file() > repo_has_object_file_with_flags() > oid_object_info_extended() - userdiff_find_by_path() > git_check_attr() > collect_some_attrs() > prepare_attr_stack() > read_attr() > read_attr_from_index() > read_blob_data_from_index() > read_object_file() As these calls are not protected by grep_read_mutex, there might be race conditions with other threads performing object reading (e.g. threads calling fill_textconv() at grep.c:fill_textconv_grep()). To prevent that, let's make sure to acquire the lock before both of these calls. Note: this patch might slow down the threaded grep in worktree, for the sake of thread-safeness. However, in the following patches, we should regain performance by replacing grep_read_mutex for an internal object reading lock and allowing parallel inflation during object reading. Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | grep: don't return an expression from pcre2_free()Hans Jerry Illikainen2019-11-301-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, the void pcre2_free() function in grep.c returned free(). While free() itself is void, afaict it's still an expression as per section A.2.3, subsection 6.8.6 (jump-statement) in both C99 [1] and C11 [2]: > return expression Section 6.8.6.4 in C99 [1] and C11 [2] says that: > A return statement with an expression shall not appear in a function > whose return type is void. The consequence of the old behavior was that developer builds with pedantic errors enabled broke Git if PCRE2 was enabled and a smart-enough compiler to detect these errors was used. This commit fixes pedantic builds of Git that enables --with-libpcre. [1] http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf [2] http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1548.pdf Signed-off-by: Hans Jerry Illikainen <hji@dyntopia.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'cb/pcre2-chartables-leakfix'Junio C Hamano2019-10-231-3/+44
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | Leakfix. * cb/pcre2-chartables-leakfix: grep: avoid leak of chartables in PCRE2 grep: make PCRE2 aware of custom allocator grep: make PCRE1 aware of custom allocator
| * | grep: avoid leak of chartables in PCRE2Carlo Marcelo Arenas Belón2019-10-181-3/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 94da9193a6 ("grep: add support for PCRE v2", 2017-06-01) introduced a small memory leak visible with valgrind in t7813. Complete the creation of a PCRE2 specific variable that was missing from the original change and free the generated table just like it is done for PCRE1. Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
View Lorry Controller Status