Re: [PATCH v8 2/5] fetch: extract collect_upstream_from_remote() helper

> IOW I would have expected to see
>
>         } else {
>                 if (transport->remote->fetch.nr) {
>                         struct string_list tracked = STRING_LIST_INIT_DUP;
>                         struct string_list_item *item;
>
>                         refspec_ref_prefixes(&transport->remote->fetch,
>                                              &transport_ls_refs_options.ref_prefixes);
>                         if (follow_remote_head != FOLLOW_REMOTE_NEVER)
>                                 do_set_head = 1;
>                 }
>
>                 /*
>                  * The configured refspec may not cover the current
>                  * branch's upstream (e.g. a narrowed -t refspec), so
>                  * make sure we can still fetch it regardless.
>                  */
>                 collect_upstream_from_remote(the_repository, &tracked,
>                                               transport->remote, NULL);
>                 for_each_string_list_item(item, &tracked)
>                         strvec_push(&transport_ls_refs_options.ref_prefixes,
>                                     item->string);
>                 string_list_clear(&tracked, 0);
>         }
>
> that touches "help the current branch when rs->nr == 0" part and
> nothing else.
>
> Of course, because the next step [3/5] wants to split the above
> else{} block to do different things between the case where
> remote.*.fetch is and is not empty, at that point the extra
> code may be introduced there, ending up with the shape of if/else if
> cascade as we see in your patch.

> > diff --git a/remote.c b/remote.c
> > index 99a086ea5a..5e980625b8 100644
> > --- a/remote.c
> > +++ b/remote.c
> > @@ -1884,6 +1884,21 @@ int branch_merge_matches(struct branch *branch,
> >       return refname_match(branch->merge[i]->src, refname);
> >  }
> >
> > +void collect_upstream_from_remote(struct repository *repo,
> > +                                struct string_list *tracked,
> > +                                struct remote *remote,
> > +                                const char *refname)
> > +{
> > +     struct branch *branch = repo_branch_get(repo, refname);
> > +
> > +     if (!branch_has_merge_config(branch) ||
> > +         strcmp(branch->remote_name, remote->name))
> > +             return;
> > +     for (int i = 0; i < branch->merge_nr; i++)
> > +             string_list_insert(tracked, branch->merge[i]->src);
> > +}
>
> The original tries to avoid branch == NULL causing a segfault, but
> the above does not.  Intended or overlooked?