Junio C Hamano wrote:
> Ramkumar Ramachandra <artagnon@xxxxxxxxx> writes:
>
>> diff --git a/remote.c b/remote.c
>> index e53a6eb..d6fcfc0 100644
>> --- a/remote.c
>> +++ b/remote.c
>> @@ -48,6 +48,7 @@ static int branches_nr;
>>
>> static struct branch *current_branch;
>> static const char *default_remote_name;
>> +static const char *pushremote_name;
>> static int explicit_default_remote_name;
>>
>> static struct rewrites rewrites;
>> @@ -363,6 +364,12 @@ static int handle_config(const char *key, const char *value, void *cb)
>> default_remote_name = branch->remote_name;
>> explicit_default_remote_name = 1;
>> }
>> + } else if (!strcmp(subkey, ".pushremote")) {
>> + if (!value)
>> + return config_error_nonbool(key);
>> + branch->pushremote_name = xstrdup(value);
>
> Perhaps use git_config_string()?
I was just following the style of the surrounding code without
thinking. However, it looks like the surrounding code may be dated,
so I'll include a patch to update it to use git_config_string() before
making the change here.
>> + if (branch == current_branch)
>> + pushremote_name = branch->pushremote_name;
>
> Why is this global only when current_branch is involved?
>
> In other words, does it make sense to read branch.$name.pushremote
> for all the other irrelevant branches?
>
> In yet other words, perhaps adding pushremote_name to the branch
> structure is unneeded, and you only need this single global
> variable?
Frankly, I'm unhappy with this global. Setting a global here and
subsequently reading it in pushremote_get() feels flaky. Why use it
at all when we have branch->remote_name, branch->remote, and (the now
introduced) branch->pushremote_name? I left the pushremote_name field
around, with the expectation that other codepaths that use the
remote_name field might be able to use it.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html