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

Re: [PATCH] remote: detect collisions in remote names

From
Junio C Hamano <gitster@pobox.com>
Date
Jul 7, 2025, 21:04 UTC
Message-ID
<xmqqtt3n3e7g.fsf@gitster.g>
In-Reply-To
<20250707202801.GA3115893@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 18 quoted lines
> On Mon, Jul 07, 2025 at 11:14:05AM +0200, Patrick Steinhardt wrote:
>
>> > +static int check_remote_collision(struct remote *remote, void *vname)
>> 
>> Tiniest nit: I was a bit puzzled what the `v` in `vname` stands for, and
>> it took a while until I noticed that it probably stands for `void`. If
>> you end up rerolling, I'd suggest to either call this `payload` or
>> `_name`.
>
> Yeah, it's for "void". This is a pattern used elsewhere for callbacks
> (usually as "vdata", but here we didn't need a container struct since
> there's only one item). I think "payload" is not a term we usually use,
> but maybe just "data" would be the usual thing (we only need "vdata"
> when we're assigning to the non-void data type).
>
> IMHO we should probably avoid the underscore pattern. It's OK here, but
> it runs close to violating the reserved names rules (a global variable
> variable _name is bad, and _Name anywhere is bad).

"name_" is available. In fact I think it is a very common pattern in this codebase to name an incoming parameter with trailing "_", and assign it to a local variable with the right name and with the right type at the top of the function.

Show 14 quoted lines
> AFAICT "remote add" allows anything that parses as a refspec, which
> implies that refs/remotes/<name>/ passes check_refname_format(). And we
> don't allow backslashes there:
>
>   $ git remote add foo/bar url
>   [no output, $? is 0]
>   $ git remote add 'bar\foo' url
>   fatal: 'bar\foo' is not a valid remote name
>
> I don't think this is platform dependent. It's coming from the
> refname_disposition table, so we're not calling is_dir_sep(). Only '/'
> is marked in that table as end-of-component, and "\\" is forbidden.
>
> So I don't think we need to worry about backslashes here.
That agrees with my understanding.  Thanks for carefully checking.
Previous: Jeff KingNext: Jeff King
Message 12 of 18 in “Allowing "/" in the name of a git remote is a strange choice”
  1. Per CederqvistJul 3, 2025
  2. Junio C HamanoJul 4, 2025
  3. Patrick SteinhardtJul 4, 2025
  4. Lidong YanJul 4, 2025
  5. Lidong YanJul 4, 2025
  6. Junio C HamanoJul 4, 2025
  7. Per CederqvistJul 4, 2025
  8. Jeff KingJul 5, 2025
  9. remote: detect collisions in remote namesJeff King, Jul 5, 2025
  10. Patrick SteinhardtJul 7, 2025
  11. Jeff KingJul 7, 2025
  12. Junio C HamanoJul 7, 2025
  13. Jeff KingJul 8, 2025
  14. Jeff KingJul 8, 2025
  15. Junio C HamanoJul 8, 2025
  16. Jeff KingJul 9, 2025
  17. Junio C HamanoJul 7, 2025
  18. Raymond E. PascoJul 9, 2025

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.