{"thread":{"id":"28605","subject":"[PATCH 0/7] pickaxe: plug memory leaks, deduplicate code","startedAt":"2011-10-06T15:59:33Z","lastAt":"2011-10-07T22:47:05Z","messageCount":9,"participants":["René Scharfe","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":7},"messages":[{"id":"177039","messageId":"4E8DD065.3040607@lsrfire.ath.cx","threadId":"28605","inReplyTo":null,"subject":"[PATCH 0/7] pickaxe: plug memory leaks, deduplicate code","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2011-10-06T15:59:33Z","receivedAt":"2011-10-06T15:59:33Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Hello,\n\nthis series aims to clean up the code for pickaxe (-S/-G). The first\nthree patches plug minor to medium memory leaks.\n\n\t[PATCH 1/7] pickaxe: plug diff filespec leak with empty needle\n\t[PATCH 2/7] pickaxe: plug regex leak\n\t[PATCH 3/7] pickaxe: plug regex/kws leak\n\nThe next one moves a duplicate if/else cascade into its own helper\nfunction:\n\n\t[PATCH 4/7] pickaxe: factor out has_changes\n\nThe remainder unifies the code for pickaxe (-S) and log grep (-G).\n\n\t[PATCH 5/7] pickaxe: pass diff_options to contains and has_changes\n\t[PATCH 6/7] pickaxe: give diff_grep the same signature as has_changes\n\t[PATCH 7/7] pickaxe: factor out pickaxe\n\nAs a result the code should be shorter and easier to maintain.\n\n diffcore-pickaxe.c | 178 ++++++++++++++++++++-------------------------------\n 1 files changed, 70 insertions(+), 108 deletions(-)\n"},{"id":"177040","messageId":"4E8DD157.3020808@lsrfire.ath.cx","threadId":"28605","inReplyTo":"4E8DD065.3040607@lsrfire.ath.cx","subject":"[PATCH 1/7] pickaxe: plug diff filespec leak with empty needle","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2011-10-06T16:03:35Z","receivedAt":"2011-10-06T16:03:35Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Check first for the unlikely case of an empty needle string and only\nthen populate the filespec, lest we leak it.\n\nSigned-off-by: Rene Scharfe \n---\n diffcore-pickaxe.c | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c\nindex c3760cf..0835a3b 100644\n--- a/diffcore-pickaxe.c\n+++ b/diffcore-pickaxe.c\n@@ -152,10 +152,10 @@ static unsigned int contains(struct diff_filespec *one,\n \tunsigned int cnt;\n \tunsigned long sz;\n \tconst char *data;\n-\tif (diff_populate_filespec(one, 0))\n-\t\treturn 0;\n \tif (!len)\n \t\treturn 0;\n+\tif (diff_populate_filespec(one, 0))\n+\t\treturn 0;\n \n \tsz = one->size;\n \tdata = one->data;\n-- \n1.7.7\n"},{"id":"177043","messageId":"4E8DD3FF.6020302@lsrfire.ath.cx","threadId":"28605","inReplyTo":"4E8DD065.3040607@lsrfire.ath.cx","subject":"[PATCH 2/7] pickaxe: plug regex leak","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2011-10-06T16:14:55Z","receivedAt":"2011-10-06T16:14:55Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"With -G... --pickaxe-all, free the regex before returning even if we\nfound a match. Also get rid of the variable has_changes, as we can\nsimply break out of the loop.\n\nSigned-off-by: Rene Scharfe \n---\n diffcore-pickaxe.c | 13 ++++++-------\n 1 files changed, 6 insertions(+), 7 deletions(-)\n\ndiff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c\nindex 0835a3b..96f7ea6 100644\n--- a/diffcore-pickaxe.c\n+++ b/diffcore-pickaxe.c\n@@ -96,7 +96,7 @@ static int diff_grep(struct diff_filepair *p, regex_t *regexp, struct diff_optio\n static void diffcore_pickaxe_grep(struct diff_options *o)\n {\n \tstruct diff_queue_struct *q = &diff_queued_diff;\n-\tint i, has_changes, err;\n+\tint i, err;\n \tregex_t regex;\n \tstruct diff_queue_struct outq;\n \toutq.queue = NULL;\n@@ -112,13 +112,11 @@ static void diffcore_pickaxe_grep(struct diff_options *o)\n \n \tif (o->pickaxe_opts & DIFF_PICKAXE_ALL) {\n \t\t/* Showing the whole changeset if needle exists */\n-\t\tfor (i = has_changes = 0; !has_changes && i < q->nr; i++) {\n+\t\tfor (i = 0; i < q->nr; i++) {\n \t\t\tstruct diff_filepair *p = q->queue[i];\n \t\t\tif (diff_grep(p, ®ex, o))\n-\t\t\t\thas_changes++;\n+\t\t\t\tgoto out; /* do not munge the queue */\n \t\t}\n-\t\tif (has_changes)\n-\t\t\treturn; /* do not munge the queue */\n \n \t\t/*\n \t\t * Otherwise we will clear the whole queue by copying\n@@ -138,10 +136,11 @@ static void diffcore_pickaxe_grep(struct diff_options *o)\n \t\t}\n \t}\n \n-\tregfree(®ex);\n-\n \tfree(q->queue);\n \t*q = outq;\n+\n+ out:\n+\tregfree(®ex);\n \treturn;\n }\n \n-- \n1.7.7\n"},{"id":"177045","messageId":"4E8DD5EF.9000700@lsrfire.ath.cx","threadId":"28605","inReplyTo":"4E8DD065.3040607@lsrfire.ath.cx","subject":"[PATCH 3/7] pickaxe: plug regex/kws leak","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2011-10-06T16:23:11Z","receivedAt":"2011-10-06T16:23:11Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"With -S... --pickaxe-all, free the regex or the kws before returning\neven if we found a match. Also get rid of the variable has_changes,\nas we can simply break out of the loop.\n\nSigned-off-by: Rene Scharfe \n---\n diffcore-pickaxe.c | 13 +++++++------\n 1 files changed, 7 insertions(+), 6 deletions(-)\n\ndiff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c\nindex 96f7ea6..c367d8d 100644\n--- a/diffcore-pickaxe.c\n+++ b/diffcore-pickaxe.c\n@@ -221,7 +221,7 @@ static void diffcore_pickaxe_count(struct diff_options *o)\n \n \tif (opts & DIFF_PICKAXE_ALL) {\n \t\t/* Showing the whole changeset if needle exists */\n-\t\tfor (i = has_changes = 0; !has_changes && i < q->nr; i++) {\n+\t\tfor (i = 0; i < q->nr; i++) {\n \t\t\tstruct diff_filepair *p = q->queue[i];\n \t\t\tif (!DIFF_FILE_VALID(p->one)) {\n \t\t\t\tif (!DIFF_FILE_VALID(p->two))\n@@ -238,9 +238,9 @@ static void diffcore_pickaxe_count(struct diff_options *o)\n \t\t\t\t contains(p->one, needle, len, regexp, kws) !=\n \t\t\t\t contains(p->two, needle, len, regexp, kws))\n \t\t\t\thas_changes++;\n+\t\t\tif (has_changes)\n+\t\t\t\tgoto out; /* do not munge the queue */\n \t\t}\n-\t\tif (has_changes)\n-\t\t\treturn; /* not munge the queue */\n \n \t\t/* otherwise we will clear the whole queue\n \t\t * by copying the empty outq at the end of this\n@@ -278,13 +278,14 @@ static void diffcore_pickaxe_count(struct diff_options *o)\n \t\t\t\tdiff_free_filepair(p);\n \t\t}\n \n+\tfree(q->queue);\n+\t*q = outq;\n+\n+ out:\n \tif (opts & DIFF_PICKAXE_REGEX)\n \t\tregfree(®ex);\n \telse\n \t\tkwsfree(kws);\n-\n-\tfree(q->queue);\n-\t*q = outq;\n \treturn;\n }\n \n-- \n1.7.7\n"},{"id":"177047","messageId":"4E8DD6B0.2070103@lsrfire.ath.cx","threadId":"28605","inReplyTo":"4E8DD065.3040607@lsrfire.ath.cx","subject":"[PATCH 4/7] pickaxe: factor out has_changes","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2011-10-06T16:26:24Z","receivedAt":"2011-10-06T16:26:24Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Move duplicate if/else construct into its own helper function.\n\nSigned-off-by: Rene Scharfe \n---\n diffcore-pickaxe.c | 57 +++++++++++++++++++--------------------------------\n 1 files changed, 21 insertions(+), 36 deletions(-)\n\ndiff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c\nindex c367d8d..4347ec1 100644\n--- a/diffcore-pickaxe.c\n+++ b/diffcore-pickaxe.c\n@@ -190,13 +190,31 @@ static unsigned int contains(struct diff_filespec *one,\n \treturn cnt;\n }\n \n+static int has_changes(struct diff_filepair *p, const char *needle,\n+\t\t unsigned long len, regex_t *regexp, kwset_t kws)\n+{\n+\tif (!DIFF_FILE_VALID(p->one)) {\n+\t\tif (!DIFF_FILE_VALID(p->two))\n+\t\t\treturn 0; /* ignore unmerged */\n+\t\t/* created */\n+\t\treturn contains(p->two, needle, len, regexp, kws) != 0;\n+\t}\n+\tif (!DIFF_FILE_VALID(p->two))\n+\t\treturn contains(p->one, needle, len, regexp, kws) != 0;\n+\tif (!diff_unmodified_pair(p)) {\n+\t\treturn contains(p->one, needle, len, regexp, kws) !=\n+\t\t contains(p->two, needle, len, regexp, kws);\n+\t}\n+\treturn 0;\n+}\n+\n static void diffcore_pickaxe_count(struct diff_options *o)\n {\n \tconst char *needle = o->pickaxe;\n \tint opts = o->pickaxe_opts;\n \tstruct diff_queue_struct *q = &diff_queued_diff;\n \tunsigned long len = strlen(needle);\n-\tint i, has_changes;\n+\tint i;\n \tregex_t regex, *regexp = NULL;\n \tkwset_t kws = NULL;\n \tstruct diff_queue_struct outq;\n@@ -223,22 +241,7 @@ static void diffcore_pickaxe_count(struct diff_options *o)\n \t\t/* Showing the whole changeset if needle exists */\n \t\tfor (i = 0; i < q->nr; i++) {\n \t\t\tstruct diff_filepair *p = q->queue[i];\n-\t\t\tif (!DIFF_FILE_VALID(p->one)) {\n-\t\t\t\tif (!DIFF_FILE_VALID(p->two))\n-\t\t\t\t\tcontinue; /* ignore unmerged */\n-\t\t\t\t/* created */\n-\t\t\t\tif (contains(p->two, needle, len, regexp, kws))\n-\t\t\t\t\thas_changes++;\n-\t\t\t}\n-\t\t\telse if (!DIFF_FILE_VALID(p->two)) {\n-\t\t\t\tif (contains(p->one, needle, len, regexp, kws))\n-\t\t\t\t\thas_changes++;\n-\t\t\t}\n-\t\t\telse if (!diff_unmodified_pair(p) &&\n-\t\t\t\t contains(p->one, needle, len, regexp, kws) !=\n-\t\t\t\t contains(p->two, needle, len, regexp, kws))\n-\t\t\t\thas_changes++;\n-\t\t\tif (has_changes)\n+\t\t\tif (has_changes(p, needle, len, regexp, kws))\n \t\t\t\tgoto out; /* do not munge the queue */\n \t\t}\n \n@@ -254,25 +257,7 @@ static void diffcore_pickaxe_count(struct diff_options *o)\n \t\t/* Showing only the filepairs that has the needle */\n \t\tfor (i = 0; i < q->nr; i++) {\n \t\t\tstruct diff_filepair *p = q->queue[i];\n-\t\t\thas_changes = 0;\n-\t\t\tif (!DIFF_FILE_VALID(p->one)) {\n-\t\t\t\tif (!DIFF_FILE_VALID(p->two))\n-\t\t\t\t\t; /* ignore unmerged */\n-\t\t\t\t/* created */\n-\t\t\t\telse if (contains(p->two, needle, len, regexp,\n-\t\t\t\t\t\t kws))\n-\t\t\t\t\thas_changes = 1;\n-\t\t\t}\n-\t\t\telse if (!DIFF_FILE_VALID(p->two)) {\n-\t\t\t\tif (contains(p->one, needle, len, regexp, kws))\n-\t\t\t\t\thas_changes = 1;\n-\t\t\t}\n-\t\t\telse if (!diff_unmodified_pair(p) &&\n-\t\t\t\t contains(p->one, needle, len, regexp, kws) !=\n-\t\t\t\t contains(p->two, needle, len, regexp, kws))\n-\t\t\t\thas_changes = 1;\n-\n-\t\t\tif (has_changes)\n+\t\t\tif (has_changes(p, needle, len, regexp, kws))\n \t\t\t\tdiff_q(&outq, p);\n \t\t\telse\n \t\t\t\tdiff_free_filepair(p);\n-- \n1.7.7\n"},{"id":"177049","messageId":"4E8DDC3E.8050308@lsrfire.ath.cx","threadId":"28605","inReplyTo":"4E8DD065.3040607@lsrfire.ath.cx","subject":"[PATCH 5/7] pickaxe: pass diff_options to contains and has_changes","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2011-10-06T16:50:06Z","receivedAt":"2011-10-06T16:50:06Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Remove the unused parameter needle from contains() and has_changes().\n\nAlso replace the parameter len with a pointer to the diff_options. We\ncan use its member pickaxe to check if the needle is an empty string\nand use the kwsmatch structure to find out the length of the match\ninstead.\n\nThis change is done as a preparation to unify the signatures of\nhas_changes() and diff_grep(), which will be used in the patch after\nthe next one to factor out common code.\n\nSigned-off-by: Rene Scharfe \n---\n diffcore-pickaxe.c | 28 ++++++++++++++--------------\n 1 files changed, 14 insertions(+), 14 deletions(-)\n\ndiff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c\nindex 4347ec1..4d66ba9 100644\n--- a/diffcore-pickaxe.c\n+++ b/diffcore-pickaxe.c\n@@ -144,14 +144,13 @@ static void diffcore_pickaxe_grep(struct diff_options *o)\n \treturn;\n }\n \n-static unsigned int contains(struct diff_filespec *one,\n-\t\t\t const char *needle, unsigned long len,\n+static unsigned int contains(struct diff_filespec *one, struct diff_options *o,\n \t\t\t regex_t *regexp, kwset_t kws)\n {\n \tunsigned int cnt;\n \tunsigned long sz;\n \tconst char *data;\n-\tif (!len)\n+\tif (!o->pickaxe[0])\n \t\treturn 0;\n \tif (diff_populate_filespec(one, 0))\n \t\treturn 0;\n@@ -175,14 +174,15 @@ static unsigned int contains(struct diff_filespec *one,\n \n \t} else { /* Classic exact string match */\n \t\twhile (sz) {\n-\t\t\tsize_t offset = kwsexec(kws, data, sz, NULL);\n+\t\t\tstruct kwsmatch kwsm;\n+\t\t\tsize_t offset = kwsexec(kws, data, sz, &kwsm);\n \t\t\tconst char *found;\n \t\t\tif (offset == -1)\n \t\t\t\tbreak;\n \t\t\telse\n \t\t\t\tfound = data + offset;\n-\t\t\tsz -= found - data + len;\n-\t\t\tdata = found + len;\n+\t\t\tsz -= found - data + kwsm.size[0];\n+\t\t\tdata = found + kwsm.size[0];\n \t\t\tcnt++;\n \t\t}\n \t}\n@@ -190,20 +190,20 @@ static unsigned int contains(struct diff_filespec *one,\n \treturn cnt;\n }\n \n-static int has_changes(struct diff_filepair *p, const char *needle,\n-\t\t unsigned long len, regex_t *regexp, kwset_t kws)\n+static int has_changes(struct diff_filepair *p, struct diff_options *o,\n+\t\t regex_t *regexp, kwset_t kws)\n {\n \tif (!DIFF_FILE_VALID(p->one)) {\n \t\tif (!DIFF_FILE_VALID(p->two))\n \t\t\treturn 0; /* ignore unmerged */\n \t\t/* created */\n-\t\treturn contains(p->two, needle, len, regexp, kws) != 0;\n+\t\treturn contains(p->two, o, regexp, kws) != 0;\n \t}\n \tif (!DIFF_FILE_VALID(p->two))\n-\t\treturn contains(p->one, needle, len, regexp, kws) != 0;\n+\t\treturn contains(p->one, o, regexp, kws) != 0;\n \tif (!diff_unmodified_pair(p)) {\n-\t\treturn contains(p->one, needle, len, regexp, kws) !=\n-\t\t contains(p->two, needle, len, regexp, kws);\n+\t\treturn contains(p->one, o, regexp, kws) !=\n+\t\t contains(p->two, o, regexp, kws);\n \t}\n \treturn 0;\n }\n@@ -241,7 +241,7 @@ static void diffcore_pickaxe_count(struct diff_options *o)\n \t\t/* Showing the whole changeset if needle exists */\n \t\tfor (i = 0; i < q->nr; i++) {\n \t\t\tstruct diff_filepair *p = q->queue[i];\n-\t\t\tif (has_changes(p, needle, len, regexp, kws))\n+\t\t\tif (has_changes(p, o, regexp, kws))\n \t\t\t\tgoto out; /* do not munge the queue */\n \t\t}\n \n@@ -257,7 +257,7 @@ static void diffcore_pickaxe_count(struct diff_options *o)\n \t\t/* Showing only the filepairs that has the needle */\n \t\tfor (i = 0; i < q->nr; i++) {\n \t\t\tstruct diff_filepair *p = q->queue[i];\n-\t\t\tif (has_changes(p, needle, len, regexp, kws))\n+\t\t\tif (has_changes(p, o, regexp, kws))\n \t\t\t\tdiff_q(&outq, p);\n \t\t\telse\n \t\t\t\tdiff_free_filepair(p);\n-- \n1.7.7\n"},{"id":"177050","messageId":"4E8DDC4A.8060001@lsrfire.ath.cx","threadId":"28605","inReplyTo":"4E8DD065.3040607@lsrfire.ath.cx","subject":"[PATCH 6/7] pickaxe: give diff_grep the same signature as has_changes","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2011-10-06T16:50:18Z","receivedAt":"2011-10-06T16:50:18Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Change diff_grep() to match the signature of has_changes() as a\npreparation for the next patch that will use function pointers to\nthe two.\n\nSigned-off-by: Rene Scharfe \n---\n diffcore-pickaxe.c | 7 ++++---\n 1 files changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c\nindex 4d66ba9..226fa0c 100644\n--- a/diffcore-pickaxe.c\n+++ b/diffcore-pickaxe.c\n@@ -45,7 +45,8 @@ static void fill_one(struct diff_filespec *one,\n \t}\n }\n \n-static int diff_grep(struct diff_filepair *p, regex_t *regexp, struct diff_options *o)\n+static int diff_grep(struct diff_filepair *p, struct diff_options *o,\n+\t\t regex_t *regexp, kwset_t kws)\n {\n \tregmatch_t regmatch;\n \tstruct userdiff_driver *textconv_one = NULL;\n@@ -114,7 +115,7 @@ static void diffcore_pickaxe_grep(struct diff_options *o)\n \t\t/* Showing the whole changeset if needle exists */\n \t\tfor (i = 0; i < q->nr; i++) {\n \t\t\tstruct diff_filepair *p = q->queue[i];\n-\t\t\tif (diff_grep(p, ®ex, o))\n+\t\t\tif (diff_grep(p, o, ®ex, NULL))\n \t\t\t\tgoto out; /* do not munge the queue */\n \t\t}\n \n@@ -129,7 +130,7 @@ static void diffcore_pickaxe_grep(struct diff_options *o)\n \t\t/* Showing only the filepairs that has the needle */\n \t\tfor (i = 0; i < q->nr; i++) {\n \t\t\tstruct diff_filepair *p = q->queue[i];\n-\t\t\tif (diff_grep(p, ®ex, o))\n+\t\t\tif (diff_grep(p, o, ®ex, NULL))\n \t\t\t\tdiff_q(&outq, p);\n \t\t\telse\n \t\t\t\tdiff_free_filepair(p);\n-- \n1.7.7\n"},{"id":"177051","messageId":"4E8DDC54.2040805@lsrfire.ath.cx","threadId":"28605","inReplyTo":"4E8DD065.3040607@lsrfire.ath.cx","subject":"[PATCH 7/7] pickaxe: factor out pickaxe","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2011-10-06T16:50:28Z","receivedAt":"2011-10-06T16:50:28Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Move the duplicate diff queue loop into its own function that accepts\na match function: has_changes() for -S and diff_grep() for -G.\n\nSigned-off-by: Rene Scharfe \n---\n diffcore-pickaxe.c | 110 ++++++++++++++++++++-------------------------------\n 1 files changed, 43 insertions(+), 67 deletions(-)\n\ndiff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c\nindex 226fa0c..380a837 100644\n--- a/diffcore-pickaxe.c\n+++ b/diffcore-pickaxe.c\n@@ -8,6 +8,46 @@\n #include \"xdiff-interface.h\"\n #include \"kwset.h\"\n \n+typedef int (*pickaxe_fn)(struct diff_filepair *p, struct diff_options *o, regex_t *regexp, kwset_t kws);\n+\n+static void pickaxe(struct diff_queue_struct *q, struct diff_options *o,\n+\t\t regex_t *regexp, kwset_t kws, pickaxe_fn fn)\n+{\n+\tint i;\n+\tstruct diff_queue_struct outq;\n+\n+\tDIFF_QUEUE_CLEAR(&outq);\n+\n+\tif (o->pickaxe_opts & DIFF_PICKAXE_ALL) {\n+\t\t/* Showing the whole changeset if needle exists */\n+\t\tfor (i = 0; i < q->nr; i++) {\n+\t\t\tstruct diff_filepair *p = q->queue[i];\n+\t\t\tif (fn(p, o, regexp, kws))\n+\t\t\t\treturn; /* do not munge the queue */\n+\t\t}\n+\n+\t\t/*\n+\t\t * Otherwise we will clear the whole queue by copying\n+\t\t * the empty outq at the end of this function, but\n+\t\t * first clear the current entries in the queue.\n+\t\t */\n+\t\tfor (i = 0; i < q->nr; i++)\n+\t\t\tdiff_free_filepair(q->queue[i]);\n+\t} else {\n+\t\t/* Showing only the filepairs that has the needle */\n+\t\tfor (i = 0; i < q->nr; i++) {\n+\t\t\tstruct diff_filepair *p = q->queue[i];\n+\t\t\tif (fn(p, o, regexp, kws))\n+\t\t\t\tdiff_q(&outq, p);\n+\t\t\telse\n+\t\t\t\tdiff_free_filepair(p);\n+\t\t}\n+\t}\n+\n+\tfree(q->queue);\n+\t*q = outq;\n+}\n+\n struct diffgrep_cb {\n \tregex_t *regexp;\n \tint hit;\n@@ -96,12 +136,8 @@ static int diff_grep(struct diff_filepair *p, struct diff_options *o,\n \n static void diffcore_pickaxe_grep(struct diff_options *o)\n {\n-\tstruct diff_queue_struct *q = &diff_queued_diff;\n-\tint i, err;\n+\tint err;\n \tregex_t regex;\n-\tstruct diff_queue_struct outq;\n-\toutq.queue = NULL;\n-\toutq.nr = outq.alloc = 0;\n \n \terr = regcomp(®ex, o->pickaxe, REG_EXTENDED | REG_NEWLINE);\n \tif (err) {\n@@ -111,36 +147,8 @@ static void diffcore_pickaxe_grep(struct diff_options *o)\n \t\tdie(\"invalid log-grep regex: %s\", errbuf);\n \t}\n \n-\tif (o->pickaxe_opts & DIFF_PICKAXE_ALL) {\n-\t\t/* Showing the whole changeset if needle exists */\n-\t\tfor (i = 0; i < q->nr; i++) {\n-\t\t\tstruct diff_filepair *p = q->queue[i];\n-\t\t\tif (diff_grep(p, o, ®ex, NULL))\n-\t\t\t\tgoto out; /* do not munge the queue */\n-\t\t}\n+\tpickaxe(&diff_queued_diff, o, ®ex, NULL, diff_grep);\n \n-\t\t/*\n-\t\t * Otherwise we will clear the whole queue by copying\n-\t\t * the empty outq at the end of this function, but\n-\t\t * first clear the current entries in the queue.\n-\t\t */\n-\t\tfor (i = 0; i < q->nr; i++)\n-\t\t\tdiff_free_filepair(q->queue[i]);\n-\t} else {\n-\t\t/* Showing only the filepairs that has the needle */\n-\t\tfor (i = 0; i < q->nr; i++) {\n-\t\t\tstruct diff_filepair *p = q->queue[i];\n-\t\t\tif (diff_grep(p, o, ®ex, NULL))\n-\t\t\t\tdiff_q(&outq, p);\n-\t\t\telse\n-\t\t\t\tdiff_free_filepair(p);\n-\t\t}\n-\t}\n-\n-\tfree(q->queue);\n-\t*q = outq;\n-\n- out:\n \tregfree(®ex);\n \treturn;\n }\n@@ -213,13 +221,9 @@ static void diffcore_pickaxe_count(struct diff_options *o)\n {\n \tconst char *needle = o->pickaxe;\n \tint opts = o->pickaxe_opts;\n-\tstruct diff_queue_struct *q = &diff_queued_diff;\n \tunsigned long len = strlen(needle);\n-\tint i;\n \tregex_t regex, *regexp = NULL;\n \tkwset_t kws = NULL;\n-\tstruct diff_queue_struct outq;\n-\tDIFF_QUEUE_CLEAR(&outq);\n \n \tif (opts & DIFF_PICKAXE_REGEX) {\n \t\tint err;\n@@ -238,36 +242,8 @@ static void diffcore_pickaxe_count(struct diff_options *o)\n \t\tkwsprep(kws);\n \t}\n \n-\tif (opts & DIFF_PICKAXE_ALL) {\n-\t\t/* Showing the whole changeset if needle exists */\n-\t\tfor (i = 0; i < q->nr; i++) {\n-\t\t\tstruct diff_filepair *p = q->queue[i];\n-\t\t\tif (has_changes(p, o, regexp, kws))\n-\t\t\t\tgoto out; /* do not munge the queue */\n-\t\t}\n-\n-\t\t/* otherwise we will clear the whole queue\n-\t\t * by copying the empty outq at the end of this\n-\t\t * function, but first clear the current entries\n-\t\t * in the queue.\n-\t\t */\n-\t\tfor (i = 0; i < q->nr; i++)\n-\t\t\tdiff_free_filepair(q->queue[i]);\n-\t}\n-\telse\n-\t\t/* Showing only the filepairs that has the needle */\n-\t\tfor (i = 0; i < q->nr; i++) {\n-\t\t\tstruct diff_filepair *p = q->queue[i];\n-\t\t\tif (has_changes(p, o, regexp, kws))\n-\t\t\t\tdiff_q(&outq, p);\n-\t\t\telse\n-\t\t\t\tdiff_free_filepair(p);\n-\t\t}\n-\n-\tfree(q->queue);\n-\t*q = outq;\n+\tpickaxe(&diff_queued_diff, o, regexp, kws, has_changes);\n \n- out:\n \tif (opts & DIFF_PICKAXE_REGEX)\n \t\tregfree(®ex);\n \telse\n-- \n1.7.7\n"},{"id":"177195","messageId":"7v7h4gto5y.fsf@alter.siamese.dyndns.org","threadId":"28605","inReplyTo":"4E8DD065.3040607@lsrfire.ath.cx","subject":"Re: [PATCH 0/7] pickaxe: plug memory leaks, deduplicate code","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-07T22:47:05Z","receivedAt":"2011-10-07T22:47:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Impressed; very nicely done.\n\nThanks.\n"}]}