Re: [PATCH v2 7/8] fetch-pack: put shallow info in output parameter
- From
- Jonathan Tan <jonathantanmy@google.com>
- Date
- Jun 14, 2018, 23:59 UTC
- Message-ID
- <20180614235939.205716-1-jonathantanmy@google.com>
- In-Reply-To
- <20180613213925.10560-8-bmwill@google.com>
Show 5 quoted lines
> @@ -1122,6 +1124,7 @@ static int do_fetch(struct transport *transport, > int autotags = (transport->remote->fetch_tags == 1); > int retcode = 0; > const struct ref *remote_refs; > + struct ref *new_remote_refs = NULL;
Above, you use the name "updated_remote_refs" - it's probably better to standardize on one. I think "updated" is better.
(The transport calling it "fetched_refs" is fine, because that's what they are from the perspective of the transport. From the perspective of fetch-pack, it is indeed a new or updated set of remote refs.)
Show 17 quoted lines
> - if (fetch_refs(transport, ref_map) || consume_refs(transport, ref_map)) {
> +
> + if (fetch_refs(transport, ref_map, &new_remote_refs)) {
> + free_refs(ref_map);
> + retcode = 1;
> + goto cleanup;
> + }
> + if (new_remote_refs) {
> + free_refs(ref_map);
> + ref_map = get_ref_map(transport->remote, new_remote_refs, rs,
> + tags, &autotags);
> + free_refs(new_remote_refs);
> + }
> + if (consume_refs(transport, ref_map)) {
> free_refs(ref_map);
> retcode = 1;
> goto cleanup;Here, if we got updated remote refs, we need to regenerate ref_map, since it is the source of truth.
Maybe add a comment in the "if (new_remote_refs)" block explaining this - something like: Regenerate ref_map using the updated remote refs, because the transport would place shallow (and other) information there.
Show 5 quoted lines
> - for (i = 0; i < nr_sought; i++) > + for (r = refs; r; r = r->next, i++) > if (status[i]) > - sought[i]->status = REF_STATUS_REJECT_SHALLOW; > + r->status = REF_STATUS_REJECT_SHALLOW;
You use i here without initializing it to 0. t5703 also fails with this patch - probably related to this, but I didn't check.
If you initialize i here, I don't think you need to initialize it to 0 at the top of this function.