From 78db6ea9dc1a872f9d07a36fe7aec700a5c963b9 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 Feb 2012 03:18:29 -0500 Subject: grep: make locking flag global The low-level grep code traditionally didn't care about threading, as it doesn't do any threading itself and didn't call out to other non-thread-safe code. That changed with 0579f91 (grep: enable threading with -p and -W using lazy attribute lookup, 2011-12-12), which pushed the lookup of funcname attributes (which is not thread-safe) into the low-level grep code. As a result, the low-level code learned about a new global "grep_attr_mutex" to serialize access to the attribute code. A multi-threaded caller (e.g., builtin/grep.c) is expected to initialize the mutex and set "use_threads" in the grep_opt structure. The low-level code only uses the lock if use_threads is set. However, putting the use_threads flag into the grep_opt struct is not the most logical place. Whether threading is in use is not something that matters for each call to grep_buffer, but is instead global to the whole program (i.e., if any thread is doing multi-threaded grep, every other thread, even if it thinks it is doing its own single-threaded grep, would need to use the locking). In practice, this distinction isn't a problem for us, because the only user of multi-threaded grep is "git-grep", which does nothing except call grep. This patch turns the opt->use_threads flag into a global flag. More important than the nit-picking semantic argument above is that this means that the locking functions don't need to actually have access to a grep_opt to know whether to lock. Which in turn can make adding new locks simpler, as we don't need to pass around a grep_opt. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- grep.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) (limited to 'grep.c') diff --git a/grep.c b/grep.c index 486230b511..7a67c2ff6f 100644 --- a/grep.c +++ b/grep.c @@ -807,26 +807,28 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol, } #ifndef NO_PTHREADS +int grep_use_locks; + /* * This lock protects access to the gitattributes machinery, which is * not thread-safe. */ pthread_mutex_t grep_attr_mutex; -static inline void grep_attr_lock(struct grep_opt *opt) +static inline void grep_attr_lock(void) { - if (opt->use_threads) + if (grep_use_locks) pthread_mutex_lock(&grep_attr_mutex); } -static inline void grep_attr_unlock(struct grep_opt *opt) +static inline void grep_attr_unlock(void) { - if (opt->use_threads) + if (grep_use_locks) pthread_mutex_unlock(&grep_attr_mutex); } #else -#define grep_attr_lock(opt) -#define grep_attr_unlock(opt) +#define grep_attr_lock() +#define grep_attr_unlock() #endif static int match_funcname(struct grep_opt *opt, const char *name, char *bol, char *eol) @@ -834,9 +836,9 @@ static int match_funcname(struct grep_opt *opt, const char *name, char *bol, cha xdemitconf_t *xecfg = opt->priv; if (xecfg && !xecfg->find_func) { struct userdiff_driver *drv; - grep_attr_lock(opt); + grep_attr_lock(); drv = userdiff_find_by_path(name); - grep_attr_unlock(opt); + grep_attr_unlock(); if (drv && drv->funcname.pattern) { const struct userdiff_funcname *pe = &drv->funcname; xdiff_set_find_func(xecfg, pe->pattern, pe->cflags); -- cgit v1.2.3 From b3aeb285d0ac1dcb4d578a61a68e08646f96501f Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 Feb 2012 03:18:41 -0500 Subject: grep: move sha1-reading mutex into low-level code The multi-threaded git-grep code needs to serialize access to the thread-unsafe read_sha1_file call. It does this with a mutex that is local to builtin/grep.c. Let's instead push this down into grep.c, where it can be used by both builtin/grep.c and grep.c. This will let us safely teach the low-level grep.c code tricks that involve reading from the object db. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/grep.c | 29 ++++++----------------------- grep.c | 6 ++++++ grep.h | 17 +++++++++++++++++ 3 files changed, 29 insertions(+), 23 deletions(-) (limited to 'grep.c') diff --git a/builtin/grep.c b/builtin/grep.c index 5b881870f6..aeb361663a 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -85,21 +85,6 @@ static inline void grep_unlock(void) pthread_mutex_unlock(&grep_mutex); } -/* Used to serialize calls to read_sha1_file. */ -static pthread_mutex_t read_sha1_mutex; - -static inline void read_sha1_lock(void) -{ - if (use_threads) - pthread_mutex_lock(&read_sha1_mutex); -} - -static inline void read_sha1_unlock(void) -{ - if (use_threads) - pthread_mutex_unlock(&read_sha1_mutex); -} - /* Signalled when a new work_item is added to todo. */ static pthread_cond_t cond_add; @@ -254,7 +239,7 @@ static void start_threads(struct grep_opt *opt) int i; pthread_mutex_init(&grep_mutex, NULL); - pthread_mutex_init(&read_sha1_mutex, NULL); + pthread_mutex_init(&grep_read_mutex, NULL); pthread_mutex_init(&grep_attr_mutex, NULL); pthread_cond_init(&cond_add, NULL); pthread_cond_init(&cond_write, NULL); @@ -303,7 +288,7 @@ static int wait_all(void) } pthread_mutex_destroy(&grep_mutex); - pthread_mutex_destroy(&read_sha1_mutex); + pthread_mutex_destroy(&grep_read_mutex); pthread_mutex_destroy(&grep_attr_mutex); pthread_cond_destroy(&cond_add); pthread_cond_destroy(&cond_write); @@ -313,8 +298,6 @@ static int wait_all(void) return hit; } #else /* !NO_PTHREADS */ -#define read_sha1_lock() -#define read_sha1_unlock() static int wait_all(void) { @@ -376,9 +359,9 @@ static void *lock_and_read_sha1_file(const unsigned char *sha1, enum object_type { void *data; - read_sha1_lock(); + grep_read_lock(); data = read_sha1_file(sha1, type, size); - read_sha1_unlock(); + grep_read_unlock(); return data; } @@ -617,10 +600,10 @@ static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec, struct strbuf base; int hit, len; - read_sha1_lock(); + grep_read_lock(); data = read_object_with_reference(obj->sha1, tree_type, &size, NULL); - read_sha1_unlock(); + grep_read_unlock(); if (!data) die(_("unable to read tree (%s)"), sha1_to_hex(obj->sha1)); diff --git a/grep.c b/grep.c index 7a67c2ff6f..db58a29c2b 100644 --- a/grep.c +++ b/grep.c @@ -826,6 +826,12 @@ static inline void grep_attr_unlock(void) if (grep_use_locks) pthread_mutex_unlock(&grep_attr_mutex); } + +/* + * Same as git_attr_mutex, but protecting the thread-unsafe object db access. + */ +pthread_mutex_t grep_read_mutex; + #else #define grep_attr_lock() #define grep_attr_unlock() diff --git a/grep.h b/grep.h index 3653bb333c..4f1b0251b0 100644 --- a/grep.h +++ b/grep.h @@ -139,6 +139,23 @@ extern int grep_threads_ok(const struct grep_opt *opt); */ extern int grep_use_locks; extern pthread_mutex_t grep_attr_mutex; +extern pthread_mutex_t grep_read_mutex; + +static inline void grep_read_lock(void) +{ + if (grep_use_locks) + pthread_mutex_lock(&grep_read_mutex); +} + +static inline void grep_read_unlock(void) +{ + if (grep_use_locks) + pthread_mutex_unlock(&grep_read_mutex); +} + +#else +#define grep_read_lock() +#define grep_read_unlock() #endif #endif -- cgit v1.2.3 From e1327023ea22c3bf57e7d28596da356043f073fc Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 Feb 2012 03:19:28 -0500 Subject: grep: refactor the concept of "grep source" into an object The main interface to the low-level grep code is grep_buffer, which takes a pointer to a buffer and a size. This is convenient and flexible (we use it to grep commit bodies, files on disk, and blobs by sha1), but it makes it hard to pass extra information about what we are grepping (either for correctness, like overriding binary auto-detection, or for optimizations, like lazily loading blob contents). Instead, let's encapsulate the idea of a "grep source", including the buffer, its size, and where the data is coming from. This is similar to the diff_filespec structure used by the diff code (unsurprising, since future patches will implement some of the same optimizations found there). The diffstat is slightly scarier than the actual patch content. Most of the modified lines are simply replacing access to raw variables with their counterparts that are now in a "struct grep_source". Most of the added lines were taken from builtin/grep.c, which partially abstracted the idea of grep sources (for file vs sha1 sources). Instead of dropping the now-redundant code, this patch leaves builtin/grep.c using the traditional grep_buffer interface (which now wraps the grep_source interface). That makes it easy to test that there is no change of behavior (yet). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- grep.c | 198 ++++++++++++++++++++++++++++++++++++++++++++++++++++++----------- grep.h | 22 ++++++++ 2 files changed, 186 insertions(+), 34 deletions(-) (limited to 'grep.c') diff --git a/grep.c b/grep.c index db58a29c2b..8204ca25e5 100644 --- a/grep.c +++ b/grep.c @@ -837,13 +837,13 @@ pthread_mutex_t grep_read_mutex; #define grep_attr_unlock() #endif -static int match_funcname(struct grep_opt *opt, const char *name, char *bol, char *eol) +static int match_funcname(struct grep_opt *opt, struct grep_source *gs, char *bol, char *eol) { xdemitconf_t *xecfg = opt->priv; if (xecfg && !xecfg->find_func) { struct userdiff_driver *drv; grep_attr_lock(); - drv = userdiff_find_by_path(name); + drv = userdiff_find_by_path(gs->name); grep_attr_unlock(); if (drv && drv->funcname.pattern) { const struct userdiff_funcname *pe = &drv->funcname; @@ -866,33 +866,33 @@ static int match_funcname(struct grep_opt *opt, const char *name, char *bol, cha return 0; } -static void show_funcname_line(struct grep_opt *opt, const char *name, - char *buf, char *bol, unsigned lno) +static void show_funcname_line(struct grep_opt *opt, struct grep_source *gs, + char *bol, unsigned lno) { - while (bol > buf) { + while (bol > gs->buf) { char *eol = --bol; - while (bol > buf && bol[-1] != '\n') + while (bol > gs->buf && bol[-1] != '\n') bol--; lno--; if (lno <= opt->last_shown) break; - if (match_funcname(opt, name, bol, eol)) { - show_line(opt, bol, eol, name, lno, '='); + if (match_funcname(opt, gs, bol, eol)) { + show_line(opt, bol, eol, gs->name, lno, '='); break; } } } -static void show_pre_context(struct grep_opt *opt, const char *name, char *buf, +static void show_pre_context(struct grep_opt *opt, struct grep_source *gs, char *bol, char *end, unsigned lno) { unsigned cur = lno, from = 1, funcname_lno = 0; int funcname_needed = !!opt->funcname; - if (opt->funcbody && !match_funcname(opt, name, bol, end)) + if (opt->funcbody && !match_funcname(opt, gs, bol, end)) funcname_needed = 2; if (opt->pre_context < lno) @@ -901,14 +901,14 @@ static void show_pre_context(struct grep_opt *opt, const char *name, char *buf, from = opt->last_shown + 1; /* Rewind. */ - while (bol > buf && + while (bol > gs->buf && cur > (funcname_needed == 2 ? opt->last_shown + 1 : from)) { char *eol = --bol; - while (bol > buf && bol[-1] != '\n') + while (bol > gs->buf && bol[-1] != '\n') bol--; cur--; - if (funcname_needed && match_funcname(opt, name, bol, eol)) { + if (funcname_needed && match_funcname(opt, gs, bol, eol)) { funcname_lno = cur; funcname_needed = 0; } @@ -916,7 +916,7 @@ static void show_pre_context(struct grep_opt *opt, const char *name, char *buf, /* We need to look even further back to find a function signature. */ if (opt->funcname && funcname_needed) - show_funcname_line(opt, name, buf, bol, cur); + show_funcname_line(opt, gs, bol, cur); /* Back forward. */ while (cur < lno) { @@ -924,7 +924,7 @@ static void show_pre_context(struct grep_opt *opt, const char *name, char *buf, while (*eol != '\n') eol++; - show_line(opt, bol, eol, name, cur, sign); + show_line(opt, bol, eol, gs->name, cur, sign); bol = eol + 1; cur++; } @@ -991,11 +991,10 @@ static void std_output(struct grep_opt *opt, const void *buf, size_t size) fwrite(buf, size, 1, stdout); } -static int grep_buffer_1(struct grep_opt *opt, const char *name, - char *buf, unsigned long size, int collect_hits) +static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int collect_hits) { - char *bol = buf; - unsigned long left = size; + char *bol; + unsigned long left; unsigned lno = 1; unsigned last_hit = 0; int binary_match_only = 0; @@ -1023,13 +1022,16 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name, } opt->last_shown = 0; + if (grep_source_load(gs) < 0) + return 0; + switch (opt->binary) { case GREP_BINARY_DEFAULT: - if (buffer_is_binary(buf, size)) + if (buffer_is_binary(gs->buf, gs->size)) binary_match_only = 1; break; case GREP_BINARY_NOMATCH: - if (buffer_is_binary(buf, size)) + if (buffer_is_binary(gs->buf, gs->size)) return 0; /* Assume unmatch */ break; case GREP_BINARY_TEXT: @@ -1043,6 +1045,8 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name, try_lookahead = should_lookahead(opt); + bol = gs->buf; + left = gs->size; while (left) { char *eol, ch; int hit; @@ -1091,14 +1095,14 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name, if (opt->status_only) return 1; if (opt->name_only) { - show_name(opt, name); + show_name(opt, gs->name); return 1; } if (opt->count) goto next_line; if (binary_match_only) { opt->output(opt, "Binary file ", 12); - output_color(opt, name, strlen(name), + output_color(opt, gs->name, strlen(gs->name), opt->color_filename); opt->output(opt, " matches\n", 9); return 1; @@ -1107,23 +1111,23 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name, * pre-context lines, we would need to show them. */ if (opt->pre_context || opt->funcbody) - show_pre_context(opt, name, buf, bol, eol, lno); + show_pre_context(opt, gs, bol, eol, lno); else if (opt->funcname) - show_funcname_line(opt, name, buf, bol, lno); - show_line(opt, bol, eol, name, lno, ':'); + show_funcname_line(opt, gs, bol, lno); + show_line(opt, bol, eol, gs->name, lno, ':'); last_hit = lno; if (opt->funcbody) show_function = 1; goto next_line; } - if (show_function && match_funcname(opt, name, bol, eol)) + if (show_function && match_funcname(opt, gs, bol, eol)) show_function = 0; if (show_function || (last_hit && lno <= last_hit + opt->post_context)) { /* If the last hit is within the post context, * we need to show this line. */ - show_line(opt, bol, eol, name, lno, '-'); + show_line(opt, bol, eol, gs->name, lno, '-'); } next_line: @@ -1141,7 +1145,7 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name, return 0; if (opt->unmatch_name_only) { /* We did not see any hit, so we want to show this */ - show_name(opt, name); + show_name(opt, gs->name); return 1; } @@ -1155,7 +1159,7 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name, */ if (opt->count && count) { char buf[32]; - output_color(opt, name, strlen(name), opt->color_filename); + output_color(opt, gs->name, strlen(gs->name), opt->color_filename); output_sep(opt, ':'); snprintf(buf, sizeof(buf), "%u\n", count); opt->output(opt, buf, strlen(buf)); @@ -1190,23 +1194,149 @@ static int chk_hit_marker(struct grep_expr *x) } } -int grep_buffer(struct grep_opt *opt, const char *name, char *buf, unsigned long size) +int grep_source(struct grep_opt *opt, struct grep_source *gs) { /* * we do not have to do the two-pass grep when we do not check * buffer-wide "all-match". */ if (!opt->all_match) - return grep_buffer_1(opt, name, buf, size, 0); + return grep_source_1(opt, gs, 0); /* Otherwise the toplevel "or" terms hit a bit differently. * We first clear hit markers from them. */ clr_hit_marker(opt->pattern_expression); - grep_buffer_1(opt, name, buf, size, 1); + grep_source_1(opt, gs, 1); if (!chk_hit_marker(opt->pattern_expression)) return 0; - return grep_buffer_1(opt, name, buf, size, 0); + return grep_source_1(opt, gs, 0); +} + +int grep_buffer(struct grep_opt *opt, const char *name, char *buf, unsigned long size) +{ + struct grep_source gs; + int r; + + grep_source_init(&gs, GREP_SOURCE_BUF, name, NULL); + gs.buf = buf; + gs.size = size; + + r = grep_source(opt, &gs); + + grep_source_clear(&gs); + return r; +} + +void grep_source_init(struct grep_source *gs, enum grep_source_type type, + const char *name, const void *identifier) +{ + gs->type = type; + gs->name = name ? xstrdup(name) : NULL; + gs->buf = NULL; + gs->size = 0; + + switch (type) { + case GREP_SOURCE_FILE: + gs->identifier = xstrdup(identifier); + break; + case GREP_SOURCE_SHA1: + gs->identifier = xmalloc(20); + memcpy(gs->identifier, identifier, 20); + break; + case GREP_SOURCE_BUF: + gs->identifier = NULL; + } +} + +void grep_source_clear(struct grep_source *gs) +{ + free(gs->name); + gs->name = NULL; + free(gs->identifier); + gs->identifier = NULL; + grep_source_clear_data(gs); +} + +void grep_source_clear_data(struct grep_source *gs) +{ + switch (gs->type) { + case GREP_SOURCE_FILE: + case GREP_SOURCE_SHA1: + free(gs->buf); + gs->buf = NULL; + gs->size = 0; + break; + case GREP_SOURCE_BUF: + /* leave user-provided buf intact */ + break; + } +} + +static int grep_source_load_sha1(struct grep_source *gs) +{ + enum object_type type; + + grep_read_lock(); + gs->buf = read_sha1_file(gs->identifier, &type, &gs->size); + grep_read_unlock(); + + if (!gs->buf) + return error(_("'%s': unable to read %s"), + gs->name, + sha1_to_hex(gs->identifier)); + return 0; +} + +static int grep_source_load_file(struct grep_source *gs) +{ + const char *filename = gs->identifier; + struct stat st; + char *data; + size_t size; + int i; + + if (lstat(filename, &st) < 0) { + err_ret: + if (errno != ENOENT) + error(_("'%s': %s"), filename, strerror(errno)); + return -1; + } + if (!S_ISREG(st.st_mode)) + return -1; + size = xsize_t(st.st_size); + i = open(filename, O_RDONLY); + if (i < 0) + goto err_ret; + data = xmalloc(size + 1); + if (st.st_size != read_in_full(i, data, size)) { + error(_("'%s': short read %s"), filename, strerror(errno)); + close(i); + free(data); + return -1; + } + close(i); + data[size] = 0; + + gs->buf = data; + gs->size = size; + return 0; +} + +int grep_source_load(struct grep_source *gs) +{ + if (gs->buf) + return 0; + + switch (gs->type) { + case GREP_SOURCE_FILE: + return grep_source_load_file(gs); + case GREP_SOURCE_SHA1: + return grep_source_load_sha1(gs); + case GREP_SOURCE_BUF: + return gs->buf ? 0 : -1; + } + die("BUG: invalid grep_source type"); } diff --git a/grep.h b/grep.h index 4f1b0251b0..e386ca4c73 100644 --- a/grep.h +++ b/grep.h @@ -129,6 +129,28 @@ extern void compile_grep_patterns(struct grep_opt *opt); extern void free_grep_patterns(struct grep_opt *opt); extern int grep_buffer(struct grep_opt *opt, const char *name, char *buf, unsigned long size); +struct grep_source { + char *name; + + enum grep_source_type { + GREP_SOURCE_SHA1, + GREP_SOURCE_FILE, + GREP_SOURCE_BUF, + } type; + void *identifier; + + char *buf; + unsigned long size; +}; + +void grep_source_init(struct grep_source *gs, enum grep_source_type type, + const char *name, const void *identifier); +int grep_source_load(struct grep_source *gs); +void grep_source_clear_data(struct grep_source *gs); +void grep_source_clear(struct grep_source *gs); + +int grep_source(struct grep_opt *opt, struct grep_source *gs); + extern struct grep_opt *grep_opt_dup(const struct grep_opt *opt); extern int grep_threads_ok(const struct grep_opt *opt); -- cgit v1.2.3 From c876d6da88d9bf1f3377d4eed66fc7705e31c30e Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 Feb 2012 03:20:10 -0500 Subject: grep: drop grep_buffer's "name" parameter Before the grep_source interface existed, grep_buffer was used by two types of callers: 1. Ones which pulled a file into a buffer, and then wanted to supply the file's name for the output (i.e., git grep). 2. Ones which really just wanted to grep a buffer (i.e., git log --grep). Callers in set (1) should now be using grep_source. Callers in set (2) always pass NULL for the "name" parameter of grep_buffer. We can therefore get rid of this now-useless parameter. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- grep.c | 4 ++-- grep.h | 2 +- revision.c | 1 - 3 files changed, 3 insertions(+), 4 deletions(-) (limited to 'grep.c') diff --git a/grep.c b/grep.c index 8204ca25e5..2a3fe7ce6f 100644 --- a/grep.c +++ b/grep.c @@ -1215,12 +1215,12 @@ int grep_source(struct grep_opt *opt, struct grep_source *gs) return grep_source_1(opt, gs, 0); } -int grep_buffer(struct grep_opt *opt, const char *name, char *buf, unsigned long size) +int grep_buffer(struct grep_opt *opt, char *buf, unsigned long size) { struct grep_source gs; int r; - grep_source_init(&gs, GREP_SOURCE_BUF, name, NULL); + grep_source_init(&gs, GREP_SOURCE_BUF, NULL, NULL); gs.buf = buf; gs.size = size; diff --git a/grep.h b/grep.h index e386ca4c73..8bf3001417 100644 --- a/grep.h +++ b/grep.h @@ -127,7 +127,7 @@ extern void append_grep_pattern(struct grep_opt *opt, const char *pat, const cha extern void append_header_grep_pattern(struct grep_opt *, enum grep_header_field, const char *); extern void compile_grep_patterns(struct grep_opt *opt); extern void free_grep_patterns(struct grep_opt *opt); -extern int grep_buffer(struct grep_opt *opt, const char *name, char *buf, unsigned long size); +extern int grep_buffer(struct grep_opt *opt, char *buf, unsigned long size); struct grep_source { char *name; diff --git a/revision.c b/revision.c index 064e351084..89d3a8fafb 100644 --- a/revision.c +++ b/revision.c @@ -2128,7 +2128,6 @@ static int commit_match(struct commit *commit, struct rev_info *opt) if (!opt->grep_filter.pattern_list && !opt->grep_filter.header_list) return 1; return grep_buffer(&opt->grep_filter, - NULL, /* we say nothing, not even filename */ commit->buffer, strlen(commit->buffer)); } -- cgit v1.2.3 From 94ad9d9e075d3f7802cf56641ebb342d43fb46e3 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 Feb 2012 03:20:43 -0500 Subject: grep: cache userdiff_driver in grep_source Right now, grep only uses the userdiff_driver for one thing: looking up funcname patterns for "-p" and "-W". As new uses for userdiff drivers are added to the grep code, we want to minimize attribute lookups, which can be expensive. It might seem at first that this would also optimize multiple lookups when the funcname pattern for a file is needed multiple times. However, the compiled funcname pattern is already cached in struct grep_opt's "priv" member, so multiple lookups are already suppressed. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- grep.c | 22 ++++++++++++++++------ grep.h | 4 ++++ 2 files changed, 20 insertions(+), 6 deletions(-) (limited to 'grep.c') diff --git a/grep.c b/grep.c index 2a3fe7ce6f..bb1856985b 100644 --- a/grep.c +++ b/grep.c @@ -841,12 +841,9 @@ static int match_funcname(struct grep_opt *opt, struct grep_source *gs, char *bo { xdemitconf_t *xecfg = opt->priv; if (xecfg && !xecfg->find_func) { - struct userdiff_driver *drv; - grep_attr_lock(); - drv = userdiff_find_by_path(gs->name); - grep_attr_unlock(); - if (drv && drv->funcname.pattern) { - const struct userdiff_funcname *pe = &drv->funcname; + grep_source_load_driver(gs); + if (gs->driver->funcname.pattern) { + const struct userdiff_funcname *pe = &gs->driver->funcname; xdiff_set_find_func(xecfg, pe->pattern, pe->cflags); } else { xecfg = opt->priv = NULL; @@ -1237,6 +1234,7 @@ void grep_source_init(struct grep_source *gs, enum grep_source_type type, gs->name = name ? xstrdup(name) : NULL; gs->buf = NULL; gs->size = 0; + gs->driver = NULL; switch (type) { case GREP_SOURCE_FILE: @@ -1340,3 +1338,15 @@ int grep_source_load(struct grep_source *gs) } die("BUG: invalid grep_source type"); } + +void grep_source_load_driver(struct grep_source *gs) +{ + if (gs->driver) + return; + + grep_attr_lock(); + gs->driver = userdiff_find_by_path(gs->name); + if (!gs->driver) + gs->driver = userdiff_find_by_name("default"); + grep_attr_unlock(); +} diff --git a/grep.h b/grep.h index 8bf3001417..73b28c2df7 100644 --- a/grep.h +++ b/grep.h @@ -9,6 +9,7 @@ typedef int pcre_extra; #endif #include "kwset.h" #include "thread-utils.h" +#include "userdiff.h" enum grep_pat_token { GREP_PATTERN, @@ -141,6 +142,8 @@ struct grep_source { char *buf; unsigned long size; + + struct userdiff_driver *driver; }; void grep_source_init(struct grep_source *gs, enum grep_source_type type, @@ -148,6 +151,7 @@ void grep_source_init(struct grep_source *gs, enum grep_source_type type, int grep_source_load(struct grep_source *gs); void grep_source_clear_data(struct grep_source *gs); void grep_source_clear(struct grep_source *gs); +void grep_source_load_driver(struct grep_source *gs); int grep_source(struct grep_opt *opt, struct grep_source *gs); -- cgit v1.2.3 From 41b59bfcb16abb738e5c95c95fb462e717d47d4d Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 Feb 2012 03:21:02 -0500 Subject: grep: respect diff attributes for binary-ness There is currently no way for users to tell git-grep that a particular path is or is not a binary file; instead, grep always relies on its auto-detection (or the user specifying "-a" to treat all binary-looking files like text). This patch teaches git-grep to use the same attribute lookup that is used by git-diff. We could add a new "grep" flag, but that is unnecessarily complex and unlikely to be useful. Despite the name, the "-diff" attribute (or "diff=foo" and the associated diff.foo.binary config option) are really about describing the contents of the path. It's simply historical that diff was the only thing that cared about these attributes in the past. And if this simple approach turns out to be insufficient, we still have a backwards-compatible path forward: we can add a separate "grep" attribute, and fall back to respecting "diff" if it is unset. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- grep.c | 16 ++++++++++++++-- grep.h | 1 + t/t7008-grep-binary.sh | 24 ++++++++++++++++++++++++ 3 files changed, 39 insertions(+), 2 deletions(-) (limited to 'grep.c') diff --git a/grep.c b/grep.c index bb1856985b..a50d161721 100644 --- a/grep.c +++ b/grep.c @@ -1024,11 +1024,11 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle switch (opt->binary) { case GREP_BINARY_DEFAULT: - if (buffer_is_binary(gs->buf, gs->size)) + if (grep_source_is_binary(gs)) binary_match_only = 1; break; case GREP_BINARY_NOMATCH: - if (buffer_is_binary(gs->buf, gs->size)) + if (grep_source_is_binary(gs)) return 0; /* Assume unmatch */ break; case GREP_BINARY_TEXT: @@ -1350,3 +1350,15 @@ void grep_source_load_driver(struct grep_source *gs) gs->driver = userdiff_find_by_name("default"); grep_attr_unlock(); } + +int grep_source_is_binary(struct grep_source *gs) +{ + grep_source_load_driver(gs); + if (gs->driver->binary != -1) + return gs->driver->binary; + + if (!grep_source_load(gs)) + return buffer_is_binary(gs->buf, gs->size); + + return 0; +} diff --git a/grep.h b/grep.h index 73b28c2df7..36e49d8255 100644 --- a/grep.h +++ b/grep.h @@ -152,6 +152,7 @@ int grep_source_load(struct grep_source *gs); void grep_source_clear_data(struct grep_source *gs); void grep_source_clear(struct grep_source *gs); void grep_source_load_driver(struct grep_source *gs); +int grep_source_is_binary(struct grep_source *gs); int grep_source(struct grep_opt *opt, struct grep_source *gs); diff --git a/t/t7008-grep-binary.sh b/t/t7008-grep-binary.sh index 917a264eea..fd6410fc71 100755 --- a/t/t7008-grep-binary.sh +++ b/t/t7008-grep-binary.sh @@ -99,4 +99,28 @@ test_expect_success 'git grep yx a' " test_must_fail git grep -f f a " +test_expect_success 'grep respects binary diff attribute' ' + echo text >t && + git add t && + echo t:text >expect && + git grep text t >actual && + test_cmp expect actual && + echo "t -diff" >.gitattributes && + echo "Binary file t matches" >expect && + git grep text t >actual && + test_cmp expect actual +' + +test_expect_success 'grep respects not-binary diff attribute' ' + echo binQary | q_to_nul >b && + git add b && + echo "Binary file b matches" >expect && + git grep bin b >actual && + test_cmp expect actual && + echo "b diff" >.gitattributes && + echo "b:binQary" >expect && + git grep bin b | nul_to_q >actual && + test_cmp expect actual +' + test_done -- cgit v1.2.3 From 08265798e1ff6abc1b0aaff31c1471f83bd51425 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 Feb 2012 03:21:11 -0500 Subject: grep: load file data after checking binary-ness Usually we load each file to grep into memory, check whether it's binary, and then either grep it (the default) or not (if "-I" was given). In the "-I" case, we can skip loading the file entirely if it is marked as binary via gitattributes. On my giant 3-gigabyte media repository, doing "git grep -I foo" went from: real 0m0.712s user 0m0.044s sys 0m4.780s to: real 0m0.026s user 0m0.016s sys 0m0.020s Obviously this is an extreme example. The repo is almost entirely binary files, and you can see that we spent all of our time asking the kernel to read() the data. However, with a cold disk cache, even avoiding a few binary files can have an impact. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- grep.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'grep.c') diff --git a/grep.c b/grep.c index a50d161721..3821400966 100644 --- a/grep.c +++ b/grep.c @@ -1019,9 +1019,6 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle } opt->last_shown = 0; - if (grep_source_load(gs) < 0) - return 0; - switch (opt->binary) { case GREP_BINARY_DEFAULT: if (grep_source_is_binary(gs)) @@ -1042,6 +1039,9 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle try_lookahead = should_lookahead(opt); + if (grep_source_load(gs) < 0) + return 0; + bol = gs->buf; left = gs->size; while (left) { -- cgit v1.2.3 From fba4f1259db8e5dc47b1c8e6e344c61f7eb4a9df Mon Sep 17 00:00:00 2001 From: Michał Kiedrowicz Date: Sat, 25 Feb 2012 10:24:28 +0100 Subject: grep -P: Fix matching ^ and $ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When "git grep" is run with -P/--perl-regexp, it doesn't match ^ and $ at the beginning/end of the line. This is because PCRE normally matches ^ and $ at the beginning/end of the whole text, not for each line, and "git grep" passes a large chunk of text (possibly containing many lines) to pcre_exec() and then splits the text into lines. This makes "git grep -P" behave differently from "git grep -E" and also from "grep -P" and "pcregrep": $ cat file a b $ git grep --no-index -P '^ ' file $ git grep --no-index -E '^ ' file file: b $ grep -c -P '^ ' file b $ pcregrep -c '^ ' file b Reported-by: Zbigniew Jędrzejewski-Szmek Signed-off-by: Michał Kiedrowicz Signed-off-by: Junio C Hamano --- grep.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'grep.c') diff --git a/grep.c b/grep.c index b29d09c7f6..9c5e1cd950 100644 --- a/grep.c +++ b/grep.c @@ -79,7 +79,7 @@ static void compile_pcre_regexp(struct grep_pat *p, const struct grep_opt *opt) { const char *error; int erroffset; - int options = 0; + int options = PCRE_MULTILINE; if (opt->ignore_case) options |= PCRE_CASELESS; -- cgit v1.2.3