The logic to decide early to do nothing and prepare the data to be inspected are the same between has_changes() and diff_grep(). Introduce pickaxe_setup() helper to share the same code. Similarly, introduce pickaxe_finish_filepair() to clean up after these two functions are done with a filepair. Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx> --- diffcore-pickaxe.c | 103 ++++++++++++++++++++++++++++------------------------- 1 file changed, 55 insertions(+), 48 deletions(-) diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c index cadb071..ac5a28d 100644 --- a/diffcore-pickaxe.c +++ b/diffcore-pickaxe.c @@ -48,6 +48,54 @@ static void pickaxe(struct diff_queue_struct *q, struct diff_options *o, *q = outq; } +static int pickaxe_setup(struct diff_filepair *p, + struct diff_options *o, + mmfile_t *mf_one, + mmfile_t *mf_two, + unsigned *what_to_free) +{ + struct userdiff_driver *textconv_one = NULL; + struct userdiff_driver *textconv_two = NULL; + + if (!o->pickaxe[0]) + return 0; + + if (DIFF_OPT_TST(o, ALLOW_TEXTCONV)) { + textconv_one = get_textconv(p->one); + textconv_two = get_textconv(p->two); + } + + /* + * If we have an unmodified pair, we know that there is no + * interesting difference and we don't even have to load the + * blobs, unless textconv is in play, _and_ we are using two + * different textconv filters (e.g., because a pair is an + * exact rename with different textconv attributes for each + * side, which might generate different content). + */ + if (textconv_one == textconv_two && diff_unmodified_pair(p)) + return 0; + + mf_one->size = fill_textconv(textconv_one, p->one, &mf_one->ptr); + mf_two->size = fill_textconv(textconv_two, p->two, &mf_two->ptr); + + *what_to_free = (textconv_one ? 1 : 0) | (textconv_two ? 2 : 0); + return 1; +} + +static void pickaxe_finish_filepair(struct diff_filepair *p, + mmfile_t *mf_one, + mmfile_t *mf_two, + unsigned what_to_free) +{ + if (what_to_free & 1) + free(mf_one->ptr); + if (what_to_free & 2) + free(mf_two->ptr); + diff_free_filespec_data(p->one); + diff_free_filespec_data(p->two); +} + struct diffgrep_cb { regex_t *regexp; int hit; @@ -78,25 +126,13 @@ static int diff_grep(struct diff_filepair *p, struct diff_options *o, regex_t *regexp, kwset_t kws) { regmatch_t regmatch; - struct userdiff_driver *textconv_one = NULL; - struct userdiff_driver *textconv_two = NULL; mmfile_t mf1, mf2; + unsigned what_to_free; int hit; - if (!o->pickaxe[0]) + if (!pickaxe_setup(p, o, &mf1, &mf2, &what_to_free)) return 0; - if (DIFF_OPT_TST(o, ALLOW_TEXTCONV)) { - textconv_one = get_textconv(p->one); - textconv_two = get_textconv(p->two); - } - - if (textconv_one == textconv_two && diff_unmodified_pair(p)) - return 0; - - mf1.size = fill_textconv(textconv_one, p->one, &mf1.ptr); - mf2.size = fill_textconv(textconv_two, p->two, &mf2.ptr); - if (!DIFF_FILE_VALID(p->one)) { if (!DIFF_FILE_VALID(p->two)) hit = 0; /* ignore unmerged */ @@ -125,12 +161,8 @@ static int diff_grep(struct diff_filepair *p, struct diff_options *o, &xpp, &xecfg); hit = ecbdata.hit; } - if (textconv_one) - free(mf1.ptr); - if (textconv_two) - free(mf2.ptr); - diff_free_filespec_data(p->one); - diff_free_filespec_data(p->two); + + pickaxe_finish_filepair(p, &mf1, &mf2, what_to_free); return hit; } @@ -201,32 +233,13 @@ static unsigned int contains(mmfile_t *mf, struct diff_options *o, static int has_changes(struct diff_filepair *p, struct diff_options *o, regex_t *regexp, kwset_t kws) { - struct userdiff_driver *textconv_one = NULL; - struct userdiff_driver *textconv_two = NULL; mmfile_t mf1, mf2; + unsigned what_to_free; int ret; - if (!o->pickaxe[0]) - return 0; - - if (DIFF_OPT_TST(o, ALLOW_TEXTCONV)) { - textconv_one = get_textconv(p->one); - textconv_two = get_textconv(p->two); - } - - /* - * If we have an unmodified pair, we know that the count will be the - * same and don't even have to load the blobs. Unless textconv is in - * play, _and_ we are using two different textconv filters (e.g., - * because a pair is an exact rename with different textconv attributes - * for each side, which might generate different content). - */ - if (textconv_one == textconv_two && diff_unmodified_pair(p)) + if (!pickaxe_setup(p, o, &mf1, &mf2, &what_to_free)) return 0; - mf1.size = fill_textconv(textconv_one, p->one, &mf1.ptr); - mf2.size = fill_textconv(textconv_two, p->two, &mf2.ptr); - if (!DIFF_FILE_VALID(p->one)) { if (!DIFF_FILE_VALID(p->two)) ret = 0; /* ignore unmerged */ @@ -240,13 +253,7 @@ static int has_changes(struct diff_filepair *p, struct diff_options *o, ret = contains(&mf1, o, regexp, kws) != contains(&mf2, o, regexp, kws); - if (textconv_one) - free(mf1.ptr); - if (textconv_two) - free(mf2.ptr); - diff_free_filespec_data(p->one); - diff_free_filespec_data(p->two); - + pickaxe_finish_filepair(p, &mf1, &mf2, what_to_free); return ret; } -- 1.8.2-588-gbf1c992 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html