Subject: [PATCH 1/3] grep: generalize header grep code to accept arbitrary headers

From: Nguyễn Thái Ngọc Duy <[email-masked]>
Date: Fri, 28 Sep 2012 14:01:20 +0700

Signed-off-by: Nguyễn Thái Ngọc Duy <[email-masked]>
---
 On Fri, Sep 28, 2012 at 12:09 AM, Junio C Hamano <[email-masked]> wrote:
 >>  enum grep_header_field {
 >>       GREP_HEADER_AUTHOR = 0,
 >> -     GREP_HEADER_COMMITTER
 >> +     GREP_HEADER_COMMITTER,
 >> +     GREP_HEADER_REFLOG,
 >> +     GREP_HEADER_FIELD_MAX
 >>  };
 >> -#define GREP_HEADER_FIELD_MAX (GREP_HEADER_COMMITTER + 1)
 >
 > Please add comment to ensure that FIELD_MAX stays at the end; if you
 > ensure that, the result is much better than the original "we know
 > committer is at the end so add one".

 It's probably even better to remove the enum. Say one day I got drunk
 and decided to add --grep-encoding. This patch makes sure that I could
 submit such a patch even in drunk state.

 grep.c     | 76 +++++++++++++++++++++++++++++++++-----------------------------
 grep.h     | 11 +++------
 revision.c |  8 +++----
 3 files changed, 47 insertions(+), 48 deletions(-)

diff --git a/grep.c b/grep.c
index 898be6e..0c72262 100644
--- a/grep.c
+++ b/grep.c
@@ -10,7 +10,7 @@ static int grep_source_is_binary(struct grep_source *gs);
 static struct grep_pat *create_grep_pat(const char *pat, size_t patlen,
 					const char *origin, int no,
 					enum grep_pat_token t,
-					enum grep_header_field field)
+					const char *header, size_t header_len)
 {
 	struct grep_pat *p = xcalloc(1, sizeof(*p));
 	p->pattern = xmemdupz(pat, patlen);
@@ -18,7 +18,8 @@ static struct grep_pat *create_grep_pat(const char *pat, size_t patlen,
 	p->origin = origin;
 	p->no = no;
 	p->token = t;
-	p->field = field;
+	p->header = header;
+	p->header_len = header_len;
 	return p;
 }
 
@@ -45,7 +46,8 @@ static void do_append_grep_pat(struct grep_pat ***tail, struct grep_pat *p)
 			if (!nl)
 				break;
 			new_pat = create_grep_pat(nl + 1, len - 1, p->origin,
-						  p->no, p->token, p->field);
+						  p->no, p->token,
+						  p->header, p->header_len);
 			new_pat->next = p->next;
 			if (!p->next)
 				*tail = &new_pat->next;
@@ -59,11 +61,13 @@ static void do_append_grep_pat(struct grep_pat ***tail, struct grep_pat *p)
 	}
 }
 
+/* header must not be freed while grep is running */
 void append_header_grep_pattern(struct grep_opt *opt,
-				enum grep_header_field field, const char *pat)
+				const char *header, const char *pat)
 {
 	struct grep_pat *p = create_grep_pat(pat, strlen(pat), "header", 0,
-					     GREP_PATTERN_HEAD, field);
+					     GREP_PATTERN_HEAD,
+					     header, strlen(header));
 	do_append_grep_pat(&opt->header_tail, p);
 }
 
@@ -76,7 +80,7 @@ void append_grep_pattern(struct grep_opt *opt, const char *pat,
 void append_grep_pat(struct grep_opt *opt, const char *pat, size_t patlen,
 		     const char *origin, int no, enum grep_pat_token t)
 {
-	struct grep_pat *p = create_grep_pat(pat, patlen, origin, no, t, 0);
+	struct grep_pat *p = create_grep_pat(pat, patlen, origin, no, t, NULL, 0);
 	do_append_grep_pat(&opt->pattern_tail, p);
 }
 
@@ -92,7 +96,7 @@ struct grep_opt *grep_opt_dup(const struct grep_opt *opt)
 	for(pat = opt->pattern_list; pat != NULL; pat = pat->next)
 	{
 		if(pat->token == GREP_PATTERN_HEAD)
-			append_header_grep_pattern(ret, pat->field,
+			append_header_grep_pattern(ret, pat->header,
 						   pat->pattern);
 		else
 			append_grep_pat(ret, pat->pattern, pat->patternlen,
@@ -359,7 +363,7 @@ static void dump_grep_pat(struct grep_pat *p)
 	switch (p->token) {
 	default: break;
 	case GREP_PATTERN_HEAD:
-		fprintf(stderr, "<head %d>", p->field); break;
+		fprintf(stderr, "<head %s>", p->header); break;
 	case GREP_PATTERN_BODY:
 		fprintf(stderr, "<body>"); break;
 	}
@@ -436,9 +440,10 @@ static struct grep_expr *grep_or_expr(struct grep_expr *left, struct grep_expr *
 static struct grep_expr *prep_header_patterns(struct grep_opt *opt)
 {
 	struct grep_pat *p;
-	struct grep_expr *header_expr;
-	struct grep_expr *(header_group[GREP_HEADER_FIELD_MAX]);
-	enum grep_header_field fld;
+	struct grep_expr *header_expr = NULL;
+	struct grep_expr **header_group;
+	struct string_list header = STRING_LIST_INIT_NODUP;
+	int fld;
 
 	if (!opt->header_list)
 		return NULL;
@@ -446,37 +451,45 @@ static struct grep_expr *prep_header_patterns(struct grep_opt *opt)
 	for (p = opt->header_list; p; p = p->next) {
 		if (p->token != GREP_PATTERN_HEAD)
 			die("bug: a non-header pattern in grep header list.");
-		if (p->field < 0 || GREP_HEADER_FIELD_MAX <= p->field)
-			die("bug: unknown header field %d", p->field);
+		if (!p->header || !p->header_len)
+			die("bug: unknown header field");
 		compile_regexp(p, opt);
+		string_list_append(&header, p->header);
 	}
 
-	for (fld = 0; fld < GREP_HEADER_FIELD_MAX; fld++)
-		header_group[fld] = NULL;
+	sort_string_list(&header);
+	string_list_remove_duplicates(&header, 0);
+	header_group = xmalloc(sizeof(*header_group) * header.nr);
+	memset(header_group, 0, sizeof(*header_group) * header.nr);
 
 	for (p = opt->header_list; p; p = p->next) {
 		struct grep_expr *h;
 		struct grep_pat *pp = p;
+		struct string_list_item *item;
 
 		h = compile_pattern_atom(&pp);
 		if (!h || pp != p->next)
 			die("bug: malformed header expr");
-		if (!header_group[p->field]) {
-			header_group[p->field] = h;
+		item = string_list_lookup(&header, p->header);
+		if (!item)
+			die("bug: malformed header expr");
+		fld = item - header.items;
+		if (!header_group[fld]) {
+			header_group[fld] = h;
 			continue;
 		}
-		header_group[p->field] = grep_or_expr(h, header_group[p->field]);
+		header_group[fld] = grep_or_expr(h, header_group[fld]);
 	}
 
-	header_expr = NULL;
-
-	for (fld = 0; fld < GREP_HEADER_FIELD_MAX; fld++) {
-		if (!header_group[fld])
-			continue;
+	for (fld = 0; fld < header.nr; fld++) {
 		if (!header_expr)
 			header_expr = grep_true_expr();
 		header_expr = grep_or_expr(header_group[fld], header_expr);
 	}
+
+	string_list_clear(&header, 0);
+	free(header_group);
+
 	return header_expr;
 }
 
@@ -691,14 +704,6 @@ static int strip_timestamp(char *bol, char **eol_p)
 	return 0;
 }
 
-static struct {
-	const char *field;
-	size_t len;
-} header_field[] = {
-	{ "author ", 7 },
-	{ "committer ", 10 },
-};
-
 static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,
 			     enum grep_context ctx,
 			     regmatch_t *pmatch, int eflags)
@@ -714,12 +719,11 @@ static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,
 	if (p->token == GREP_PATTERN_HEAD) {
 		const char *field;
 		size_t len;
-		assert(p->field < ARRAY_SIZE(header_field));
-		field = header_field[p->field].field;
-		len = header_field[p->field].len;
-		if (strncmp(bol, field, len))
+		field = p->header;
+		len = p->header_len;
+		if (strncmp(bol, field, len) || bol[len] != ' ')
 			return 0;
-		bol += len;
+		bol += len + 1;
 		saved_ch = strip_timestamp(bol, &eol);
 	}
 
diff --git a/grep.h b/grep.h
index 8a28a67..bc00f2a 100644
--- a/grep.h
+++ b/grep.h
@@ -27,12 +27,6 @@ enum grep_context {
 	GREP_CONTEXT_BODY
 };
 
-enum grep_header_field {
-	GREP_HEADER_AUTHOR = 0,
-	GREP_HEADER_COMMITTER
-};
-#define GREP_HEADER_FIELD_MAX (GREP_HEADER_COMMITTER + 1)
-
 struct grep_pat {
 	struct grep_pat *next;
 	const char *origin;
@@ -40,7 +34,8 @@ struct grep_pat {
 	enum grep_pat_token token;
 	char *pattern;
 	size_t patternlen;
-	enum grep_header_field field;
+	const char *header;
+	size_t header_len;
 	regex_t regexp;
 	pcre *pcre_regexp;
 	pcre_extra *pcre_extra_info;
@@ -136,7 +131,7 @@ struct grep_opt {
 
 extern void append_grep_pat(struct grep_opt *opt, const char *pat, size_t patlen, const char *origin, int no, enum grep_pat_token t);
 extern void append_grep_pattern(struct grep_opt *opt, const char *pat, const char *origin, int no, enum grep_pat_token t);
-extern void append_header_grep_pattern(struct grep_opt *, enum grep_header_field, const char *);
+extern void append_header_grep_pattern(struct grep_opt *, const char *, 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, char *buf, unsigned long size);
diff --git a/revision.c b/revision.c
index ae12e11..f7cf385 100644
--- a/revision.c
+++ b/revision.c
@@ -1288,9 +1288,9 @@ static void add_grep(struct rev_info *revs, const char *ptn, enum grep_pat_token
 	append_grep_pattern(&revs->grep_filter, ptn, "command line", 0, what);
 }
 
-static void add_header_grep(struct rev_info *revs, enum grep_header_field field, const char *pattern)
+static void add_header_grep(struct rev_info *revs, const char *header, const char *pattern)
 {
-	append_header_grep_pattern(&revs->grep_filter, field, pattern);
+	append_header_grep_pattern(&revs->grep_filter, header, pattern);
 }
 
 static void add_message_grep(struct rev_info *revs, const char *pattern)
@@ -1590,10 +1590,10 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 	 * Grepping the commit log
 	 */
 	else if ((argcount = parse_long_opt("author", argv, &optarg))) {
-		add_header_grep(revs, GREP_HEADER_AUTHOR, optarg);
+		add_header_grep(revs, "author", optarg);
 		return argcount;
 	} else if ((argcount = parse_long_opt("committer", argv, &optarg))) {
-		add_header_grep(revs, GREP_HEADER_COMMITTER, optarg);
+		add_header_grep(revs, "committer", optarg);
 		return argcount;
 	} else if ((argcount = parse_long_opt("grep", argv, &optarg))) {
 		add_message_grep(revs, optarg);
-- 
1.7.12.1.405.gb727dc9