# [PATCH] diffcore-pickaxe: simplify has_changes and contains 3 messages from 2013-07-06 to 2013-07-07. Participants: René Scharfe, Jeff King, Junio C Hamano. Thread: https://gitlist.dev/t/34363 ## René Scharfe, 2013-07-06 13:53 Subject: [PATCH] diffcore-pickaxe: simplify has_changes and contains Message-ID: <1373118807-1345-1-git-send-email-rene.scharfe@lsrfire.ath.cx> URL: https://gitlist.dev/e/1373118807-1345-1-git-send-email-rene.scharfe%40lsrfire.ath.cx ``` Halve the number of callsites of contains() to two using temporary variables, simplifying the code. While at it, get rid of the diff_options parameter, which became unused with 8fa4b09f. Signed-off-by: René Scharfe --- diffcore-pickaxe.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c index c97ac9b..401eb72 100644 --- a/diffcore-pickaxe.c +++ b/diffcore-pickaxe.c @@ -131,8 +131,7 @@ static void diffcore_pickaxe_grep(struct diff_options *o) return; } -static unsigned int contains(mmfile_t *mf, struct diff_options *o, - regex_t *regexp, kwset_t kws) +static unsigned int contains(mmfile_t *mf, regex_t *regexp, kwset_t kws) { unsigned int cnt; unsigned long sz; @@ -176,11 +175,9 @@ static int has_changes(mmfile_t *one, mmfile_t *two, struct diff_options *o, regex_t *regexp, kwset_t kws) { - if (!one) - return contains(two, o, regexp, kws) != 0; - if (!two) - return contains(one, o, regexp, kws) != 0; - return contains(one, o, regexp, kws) != contains(two, o, regexp, kws); + unsigned int one_contains = one ? contains(one, regexp, kws) : 0; + unsigned int two_contains = two ? contains(two, regexp, kws) : 0; + return one_contains != two_contains; } static int pickaxe_match(struct diff_filepair *p, struct diff_options *o, -- 1.8.3.2 ``` ## Jeff King, 2013-07-07 04:03 Subject: Re: [PATCH] diffcore-pickaxe: simplify has_changes and contains Message-ID: <20130707040337.GA30898@sigill.intra.peff.net> URL: https://gitlist.dev/e/20130707040337.GA30898%40sigill.intra.peff.net In-Reply-To: <1373118807-1345-1-git-send-email-rene.scharfe@lsrfire.ath.cx> ``` On Sat, Jul 06, 2013 at 03:53:27PM +0200, René Scharfe wrote: > Halve the number of callsites of contains() to two using temporary > variables, simplifying the code. While at it, get rid of the > diff_options parameter, which became unused with 8fa4b09f. There is a slight change in behavior, too: > - if (!one) > - return contains(two, o, regexp, kws) != 0; > - if (!two) > - return contains(one, o, regexp, kws) != 0; > - return contains(one, o, regexp, kws) != contains(two, o, regexp, kws); > + unsigned int one_contains = one ? contains(one, regexp, kws) : 0; > + unsigned int two_contains = two ? contains(two, regexp, kws) : 0; > + return one_contains != two_contains; Before, if (!one && !two) we would call contains(two, ...), and now we will simply assume it is zero. Which I think is an improvement, as we would have segfaulted before. I don't think it is a bug in the current code (we would not ever feed the function two NULLs), but it is nice to be more defensive. Acked-by: Jeff King -Peff ``` ## Junio C Hamano, 2013-07-07 17:25 Subject: Re: [PATCH] diffcore-pickaxe: simplify has_changes and contains Message-ID: <7vbo6e5ljs.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vbo6e5ljs.fsf%40alter.siamese.dyndns.org In-Reply-To: <20130707040337.GA30898@sigill.intra.peff.net> ``` Jeff King writes: > Before, if (!one && !two) we would call contains(two, ...), and now we > will simply assume it is zero. Which I think is an improvement, as we > would have segfaulted before. I don't think it is a bug in the current > code (we would not ever feed the function two NULLs), but it is nice to > be more defensive. > > Acked-by: Jeff King Thanks, both. ```