>  static void update_shallow(struct fetch_pack_args *args,
> -                        struct ref **sought, int nr_sought,
> +                        struct ref *refs,

update_shallow() now takes in a linked list of refs instead of an array.
I see that the translation of this function is straightforward -
occasionally, we need to iterate through the linked list and count up
from 0 at the same time, but that is not a problem.

>                          struct shallow_info *si)
>  {
>       struct oid_array ref = OID_ARRAY_INIT;
>       int *status;
> -     int i;
> +     int i = 0;

Remove the " = 0" - I've verified that it does not need to be there, and
it might inhibit useful "unintialized variable" warnings if others were
to change the code later.

Optional: I would also remove this declaration and declare "int i;" in
each of the blocks that need it.

>  static int fetch_refs_via_pack(struct transport *transport,
> -                            int nr_heads, struct ref **to_fetch)
> +                            int nr_heads, struct ref **to_fetch,
> +                            struct ref **fetched_refs)
>  {
>       int ret = 0;
>       struct git_transport_data *data = transport->data;
> @@ -354,8 +356,12 @@ static int fetch_refs_via_pack(struct transport 
> *transport,
>       if (report_unmatched_refs(to_fetch, nr_heads))
>               ret = -1;
>  
> +     if (fetched_refs)
> +             *fetched_refs = refs;
> +     else
> +             free_refs(refs);
> +
>       free_refs(refs_tmp);
> -     free_refs(refs);
>       free(dest);
>       return ret;
>  }

Instead of just freeing the linked list, we return it if requested by
the client. This makes sense.

> -int transport_fetch_refs(struct transport *transport, struct ref *refs)
> +int transport_fetch_refs(struct transport *transport, struct ref *refs,
> +                      struct ref **fetched_refs)
>  {
>       int rc;
>       int nr_heads = 0, nr_alloc = 0, nr_refs = 0;
>       struct ref **heads = NULL;
> +     struct ref *nop_head = NULL, **nop_tail = &nop_head;
>       struct ref *rm;
>  
>       for (rm = refs; rm; rm = rm->next) {
>               nr_refs++;
>               if (rm->peer_ref &&
>                   !is_null_oid(&rm->old_oid) &&
> -                 !oidcmp(&rm->peer_ref->old_oid, &rm->old_oid))
> +                 !oidcmp(&rm->peer_ref->old_oid, &rm->old_oid)) {
> +                     /*
> +                      * These need to be reported as fetched, but we don't
> +                      * actually need to fetch them.
> +                      */
> +                     if (fetched_refs) {
> +                             struct ref *nop_ref = copy_ref(rm);
> +                             *nop_tail = nop_ref;
> +                             nop_tail = &nop_ref->next;
> +                     }
>                       continue;
> +             }
>               ALLOC_GROW(heads, nr_heads + 1, nr_alloc);
>               heads[nr_heads++] = rm;
>       }
> @@ -1245,7 +1263,11 @@ int transport_fetch_refs(struct transport *transport, 
> struct ref *refs)
>                       heads[nr_heads++] = rm;
>       }
>  
> -     rc = transport->vtable->fetch(transport, nr_heads, heads);
> +     rc = transport->vtable->fetch(transport, nr_heads, heads, fetched_refs);
> +     if (fetched_refs && nop_head) {
> +             *nop_tail = *fetched_refs;
> +             *fetched_refs = nop_head;
> +     }
>  
>       free(heads);
>       return rc;

And sometimes, even if we are merely simulating the fetching of refs, we
still need to report those refs in fetched_refs. This is correct.

I also see that t5703 now passes.

Besides enabling the writing of subsequent patches, I see that this also
makes the API clearer in that the input refs to transport_fetch_refs()
are not overloaded to output shallow information. Other than the " = 0"
change above, this patch looks good to me.

Reply via email to