From: Glen Choo <chooglen@google.com>
Date: Mon, 25 Oct 2021 19:36:26 GMT
Subject: Re: [PATCH v3 2/4] remote: use remote_state parameter internally
Message-ID: <kl6lzgqwaocl.fsf@chooglen-macbookpro.roam.corp.google.com>
In-Reply-To: <kl6l35opasd1.fsf@chooglen-macbookpro.roam.corp.google.com>

Glen Choo <chooglen@google.com> writes:

>> The caller is remote_get_1(); this funciton was called with
>> "current_branch", which can be NULL until you have a repository and
>> you've called read_config(), but otherwise shouldn't be.

One possible culprit is working with detached HEAD, are you pushing with
detached HEAD?

"current_branch" is allowed to be NULL when HEAD does not point to a
branch. From read_config():

	if (startup_info->have_repository) {
		const char *head_ref = refs_resolve_ref_unsafe(
			get_main_ref_store(repo), "HEAD", 0, NULL, &flag);
		if (head_ref && (flag & REF_ISSYMREF) &&
		    skip_prefix(head_ref, "refs/heads/", &head_ref)) {
			repo->remote_state->current_branch = make_branch(
				repo->remote_state, head_ref, strlen(head_ref));
		}
	}

This condition fails in detached head and "current_branch" is not set:

  head_ref && (flag & REF_ISSYMREF) && skip_prefix(head_ref, "refs/heads/", &head_ref)

>> The direct culprit is this part:
>>
>>     const char *pushremote_for_branch(struct branch *branch, int *explicit)
>>     {
>>             if (branch && branch->pushremote_name) {
>>                     if (explicit)
>>                             *explicit = 1;
>>                     return branch->pushremote_name;
>>             }
>>             if (branch->remote_state->pushremote_name) {
>>
>> where the second if() statement used to check "pushremote_name", but
>> now unconditionally dereferences "branch".
>>
To work with branch = NULL, it seems obvious that branch should be
conditionally dereferenced in pushremote_for_branch, i.e.

  - if (branch->remote_state->pushremote_name) {
  + if (brnach && branch->remote_state->pushremote_name) {

However, if you are not using detached HEAD, the problem might be
elsewhere..