Re: [PATCH v6 1/3] read-cache: add strcmp_offset function

> Am 06.04.2017 um 18:34 schrieb git@jeffhostetler.com:
>> diff --git a/read-cache.c b/read-cache.c
>> index 9054369..e8f1900 100644
>> --- a/read-cache.c
>> +++ b/read-cache.c
>> @@ -887,6 +887,26 @@ static int has_file_name(struct index_state *istate,
>>       return retval;
>>   }
>>   +
>> +/*
>> + * Like strcmp(), but also return the offset of the first change.
>> + * If strings are equal, return the length.
>> + */
>> +int strcmp_offset(const char *s1, const char *s2, int *first_change)
>> +{
>> +    int k;
>> +
>> +    if (!first_change)
>> +        return strcmp(s1, s2);
>> +
>> +    for (k = 0; s1[k] == s2[k]; k++)
>> +        if (s1[k] == '\0')
>> +            break;
>> +
>> +    *first_change = k;
>> +    return ((unsigned char *)s1)[k] - ((unsigned char *)s2)[k];
>> +}
>
> I like how this is shaping up to become a reusable function, but I only
> found one other possible use case (in read-cache.c::ce_write_entry),
> which somehow irritates me.  Either I didn't look hard enough (likely),
> we haven't got fitting code just yet, or this function isn't meant to be
> exported -- in the latter case its interface doesn't have to be polished
> as much.  (Just a thought, don't let me stop you.)
>
> Assuming strcmp_offset() is a library-grade function then its
> first_change parameter should point to a size_t instead of an int, no?

>
> Casting away the const qualifier in the return line is a bit iffy.  Why
> not cast after dereferencing, like this?
>
>     return (unsigned char)s1[k] - (unsigned char)s2[k];