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).
> Hm. Do we have to care about '\' on Windows, as well? This made me
> rediscover the following function:
>
> static int valid_remote_nick(const char *name)
> {
> if (!name[0] || is_dot_or_dotdot(name))
> return 0;
>
> /* remote nicknames cannot contain slashes */
> while (*name)
> if (is_dir_sep(*name++))
> return 0;
> return 1;
> }
>
> Which... puzzled me a bit at first, as it seems to indicate that a
> remote with a path separator is invalid. But as it turns out we only use
> this function if remotes are configured via ".git/remotes" or
> ".git/branches". Looks like we eventually lost this limitation, probably
> when config-based remotes were introduced.
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.
-Peff
