git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] builtin/remote: remove postfixcmp() and use suffixcmp() instead

From
Christian Couder <chriscool@tuxfamily.org>
Date
Nov 4, 2013, 20:16 UTC
Message-ID
<20131104.211627.54064800775850019.chriscool@tuxfamily.org>
In-Reply-To
<xmqqbo20ynxs.fsf@gitster.dls.corp.google.com>
From: Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH] builtin/remote: remove postfixcmp() and use suffixcmp() instead
Date: Mon, 04 Nov 2013 11:19:43 -0800
Show 8 quoted lines
> Christian Couder <chriscool@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.
 
Show 6 quoted lines
> 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.

Show 20 quoted lines
> 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.

Previous: Junio C HamanoNext: Junio C Hamano
Message 4 of 7 in “builtin/remote: remove postfixcmp() and use suffixcmp() instead”
  1. builtin/remote: remove postfixcmp() and use suffixcmp() insteadChristian Couder, Nov 3, 2013
  2. Jonathan NiederNov 4, 2013
  3. Junio C HamanoNov 4, 2013
  4. Christian CouderNov 4, 2013
  5. Junio C HamanoNov 4, 2013
  6. Johannes SchindelinNov 4, 2013
  7. Junio C HamanoNov 4, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.