Johannes Schindelin <johannes.schinde...@gmx.de> writes:

> Hi Junio,
>
> On Mon, 4 Nov 2013, Junio C Hamano wrote:
>
>> Junio C Hamano <gits...@pobox.com> 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to