On 06/25, Jonathan Tan wrote:
> > 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.
Perfect, I'll just drop the " = 0" part (making the diff slightly
smaller)
--
Brandon Williams
