{"thread":{"id":"34363","subject":"[PATCH] diffcore-pickaxe: simplify has_changes and contains","startedAt":"2013-07-06T13:53:27Z","lastAt":"2013-07-07T17:25:59Z","messageCount":3,"participants":["René Scharfe","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"222671","messageId":"1373118807-1345-1-git-send-email-rene.scharfe@lsrfire.ath.cx","threadId":"34363","inReplyTo":null,"subject":"[PATCH] diffcore-pickaxe: simplify has_changes and contains","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2013-07-06T13:53:27Z","receivedAt":"2013-07-06T13:53:27Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Halve the number of callsites of contains() to two using temporary\nvariables, simplifying the code.  While at it, get rid of the\ndiff_options parameter, which became unused with 8fa4b09f.\n\nSigned-off-by: René Scharfe <rene.scharfe@lsrfire.ath.cx>\n---\n diffcore-pickaxe.c | 11 ++++-------\n 1 file changed, 4 insertions(+), 7 deletions(-)\n\ndiff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c\nindex c97ac9b..401eb72 100644\n--- a/diffcore-pickaxe.c\n+++ b/diffcore-pickaxe.c\n@@ -131,8 +131,7 @@ static void diffcore_pickaxe_grep(struct diff_options *o)\n \treturn;\n }\n \n-static unsigned int contains(mmfile_t *mf, struct diff_options *o,\n-\t\t\t     regex_t *regexp, kwset_t kws)\n+static unsigned int contains(mmfile_t *mf, regex_t *regexp, kwset_t kws)\n {\n \tunsigned int cnt;\n \tunsigned long sz;\n@@ -176,11 +175,9 @@ static int has_changes(mmfile_t *one, mmfile_t *two,\n \t\t       struct diff_options *o,\n \t\t       regex_t *regexp, kwset_t kws)\n {\n-\tif (!one)\n-\t\treturn contains(two, o, regexp, kws) != 0;\n-\tif (!two)\n-\t\treturn contains(one, o, regexp, kws) != 0;\n-\treturn contains(one, o, regexp, kws) != contains(two, o, regexp, kws);\n+\tunsigned int one_contains = one ? contains(one, regexp, kws) : 0;\n+\tunsigned int two_contains = two ? contains(two, regexp, kws) : 0;\n+\treturn one_contains != two_contains;\n }\n \n static int pickaxe_match(struct diff_filepair *p, struct diff_options *o,\n-- \n1.8.3.2\n"},{"id":"222696","messageId":"20130707040337.GA30898@sigill.intra.peff.net","threadId":"34363","inReplyTo":"1373118807-1345-1-git-send-email-rene.scharfe@lsrfire.ath.cx","subject":"Re: [PATCH] diffcore-pickaxe: simplify has_changes and contains","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-07-07T04:03:37Z","receivedAt":"2013-07-07T04:03:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jul 06, 2013 at 03:53:27PM +0200, René Scharfe wrote:\n\n> Halve the number of callsites of contains() to two using temporary\n> variables, simplifying the code.  While at it, get rid of the\n> diff_options parameter, which became unused with 8fa4b09f.\n\nThere is a slight change in behavior, too:\n\n> -\tif (!one)\n> -\t\treturn contains(two, o, regexp, kws) != 0;\n> -\tif (!two)\n> -\t\treturn contains(one, o, regexp, kws) != 0;\n> -\treturn contains(one, o, regexp, kws) != contains(two, o, regexp, kws);\n> +\tunsigned int one_contains = one ? contains(one, regexp, kws) : 0;\n> +\tunsigned int two_contains = two ? contains(two, regexp, kws) : 0;\n> +\treturn one_contains != two_contains;\n\nBefore, if (!one && !two) we would call contains(two, ...), and now we\nwill simply assume it is zero. Which I think is an improvement, as we\nwould have segfaulted before. I don't think it is a bug in the current\ncode (we would not ever feed the function two NULLs), but it is nice to\nbe more defensive.\n\nAcked-by: Jeff King <peff@peff.net>\n\n-Peff\n"},{"id":"222749","messageId":"7vbo6e5ljs.fsf@alter.siamese.dyndns.org","threadId":"34363","inReplyTo":"20130707040337.GA30898@sigill.intra.peff.net","subject":"Re: [PATCH] diffcore-pickaxe: simplify has_changes and contains","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-07T17:25:59Z","receivedAt":"2013-07-07T17:25:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Before, if (!one && !two) we would call contains(two, ...), and now we\n> will simply assume it is zero. Which I think is an improvement, as we\n> would have segfaulted before. I don't think it is a bug in the current\n> code (we would not ever feed the function two NULLs), but it is nice to\n> be more defensive.\n>\n> Acked-by: Jeff King <peff@peff.net>\n\nThanks, both.\n"}]}