On Thu, May 11, 2017 at 03:30:54PM -0700, Jonathan Tan wrote:
> fetch-pack, when fetching a literal SHA-1 from a server that is not
> configured with uploadpack.allowtipsha1inwant (or similar), always
> returns an error message of the form "Server does not allow request for
> unadvertised object %s". However, it is sometimes the case that such
> object is advertised. This situation would occur, for example, if a user
> or a script was provided a SHA-1 instead of a branch or tag name for
> fetching, and wanted to invoke "git fetch" or "git fetch-pack" using
> that SHA-1.
>
> Teach fetch-pack to also check the SHA-1s of the refs in the received
> ref advertisement if a literal SHA-1 was given by the user.
>
> Helped-by: Jeff King <peff@xxxxxxxx>
> Signed-off-by: Jonathan Tan <jonathantanmy@xxxxxxxxxx>
> ---
This looks good to me. There's one minor nit that I don't think I saw
mentioned, and I don't think needs to hold up the patch. But I wanted to
mention it just in case I'm wrong that it doesn't matter.
> static void filter_refs(struct fetch_pack_args *args,
> struct ref **refs,
> struct ref **sought, int nr_sought)
> {
> struct ref *newlist = NULL;
> struct ref **newtail = &newlist;
> + struct ref *unmatched = NULL;
> struct ref *ref, *next;
> + struct oidset tip_oids = OIDSET_INIT;
> int i;
>
> i = 0;
> @@ -631,7 +651,8 @@ static void filter_refs(struct fetch_pack_args *args,
> ref->next = NULL;
> newtail = &ref->next;
> } else {
> - free(ref);
> + ref->next = unmatched;
> + unmatched = ref;
> }
The incoming "refs" list is sorted, and we rely on that sorting to do
the linear walk. Likewise, we append to newlist via newtail, so it
remains sorted (and I suspect the consumers of the list rely on that).
But your new "unmatched" list is done by prepending, so it's in reverse
order.
I don't think that matters for our purposes here, and the list doesn't
escape our function. So there's no bug, but I just wonder if it might
end up biting somebody in the future. I'm OK with leaving it, though.
-Peff