Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:
> Hi Junio,
>
> On Mon, 4 Nov 2013, Junio C Hamano wrote:
>
>> Junio C Hamano <gitster@xxxxxxxxx> writes:
>>
>> > I do not think anybody sane uses prefixcmp() or suffixcmp() for
>> > anything but checking with zero; in other words, I suspect that all
>> > uses of Xcmp() can be replaced with !!Xcmp(), so as a separate
>> > clean-up patch, we may at least want to make it clear that the
>> > callers should not expect anything but "does str have sfx as its
>> > suffix, yes or no?" by doing something like this:
>> >
>> > int suffixcmp(const char *str, const char *suffix)
>> > {
>> > int len = strlen(str), suflen = strlen(suffix);
>> > if (len < suflen)
>> > return -1;
>> > else
>> > - return strcmp(str + len - suflen, suffix);
>> > + return !!strcmp(str + len - suflen, suffix);
>> > }
>> >
>> > I am not absolutely sure about doing the same to prefixcmp(),
>> > though. It could be used for ordering, even though no existing code
>> > seems to do so.
>>
>> I just realized why this suggestion is incomplete; if we were to go
>> this route, we should rename the function to has_suffix() or
>> something. anything-cmp() ought to be usable as an ordering
>> comparison function, but suffixcmp() clearly isn't.
>
> I have to admit that I do not understand why a change in suffixcmp()'s
> behavior is needed.
I made the suggestion only because I do not understand why the
function should order "f" and ".txt" in the
"f" < ".txt"
order. Even worse, the other function postfixcmp() orders them the
other way around.
If -1 returned from the function were an indication of error "The
string does not even have that suffix", then I would have been a bit
more sympathetic, and its current behaviour in that case could be
argued as a special case of the broader return value "non-zero (from
the ordinary strcmp() return codeflow) means the string does not
have that suffix and zero means the string ends with the suffix".
But then, a function that pretends to be for ordering comparison,
with a name that ends with cmp(), and then declaring that "no, this
is not for ordering; the sign of the result does not matter--what
only matters is if it returns zero or non-zero", feels quite
schizophrenic, at least to me.
And my earlier suggestion to change the return value *is* not a
right change. It still keeps the pretense of comparison for
ordering (i.e. ...cmp() name), while returning a value that cannot
possibly be used for ordering.
So I think the right patch should make the function like this:
int has_suffix(const char *str, const char *suffix)
{
int len = strlen(str);
int suffix_len = strlen(suffix);
if (len < suffix_len)
return 0;
return !strcmp(str + len - suffix_len, suffix);
}
--
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