Re: [PATCH 1/2] git apply: option to ignore whitespace differences

>> diff --git a/builtin-apply.c b/builtin-apply.c
>> index dc0ff5e..86860d6 100644
>> --- a/builtin-apply.c
>> +++ b/builtin-apply.c
>> @@ -39,6 +39,7 @@ static int diffstat;
>>  static int numstat;
>>  static int summary;
>>  static int check;
>> +static int ignore_whitespace = 0;
>
> s/ = 0//;

>> +/*
>> + * Compare two memory areas ignoring whitespace differences
>> + */
>> +static int memcmp_ignore_whitespace(const char *s1, const char *s2, size_t n)
>> +{
>
> Don't you think this function signature is bogus?

>> +static int memcmp_switch(const char *s1, const char *s2, size_t n)
>
> This function signature shares the same bogosity with the previous one.
>
> In addition, it's name and semantics is bogus.

> If you are separating that "compare two lines" logic into a helper
> function, I would expect its name actually reflect its purpose, whose
> behaviour may change depending on --ignore-whitespace.  The traditional
> codepath would say "do they have the same length and match byte-for-byte?"
> while the new loosened codepath would say "we do not care about the
> whitespaces; do they match if we disregard ws differences?"

> I also suspect that you might be able to optimize the existing "allow
> whitespace-fixed match" a bit by "fix"ing the target buffer only once,
> instead of doing so line-by-line for every offset that find_pos() checks
> by calling match_fragment().  It is an independent issue, but it appears
> to me that the change this patch wants to do to match_fragment() might
> become cleaner if we did that conversion first, as match_fragment() itself
> won't have to have two cases (early return for exact match, and match with
> whitespace-fixed target).