From: Junio C Hamano <gits...@pobox.com>
Subject: Re: [PATCH] builtin/remote: remove postfixcmp() and use suffixcmp()
instead
Date: Mon, 04 Nov 2013 11:19:43 -0800
> Christian Couder <chrisc...@tuxfamily.org> writes:
>
>> Commit 8cc5b290 (git merge -X<option>, 25 Nov 2009) introduced
>> suffixcmp() with nearly the same implementation as postfixcmp()
>> that already existed since commit 211c8968 (Make git-remote a
>> builtin, 29 Feb 2008).
>
> This "nearly the same" piqued my curiosity ;-)
Yeah, I realize I should have explained the differences.
> The postfixcmp() you are removing will say "f" > ".txt" while
> suffixcmp() will say "f" < ".txt".
>
> As postfixcmp() is only used to compare for equality, the
> distinction does not matter and does not affect the correctness of
> this patch, but I am not sure which answer is more correct.
Yeah, that's also my opinion. I am not even sure if there is one more
correct answer than the other.
> 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 agree.
I will resend a 2 patch long patch series where the first patch will
have an improved commit message and the second patch will do what you suggest
above.
Thanks,
Christian.
--
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
