Skip to content

Commit 1b8dbdb

Browse files
gitstergitster
committed
Merge branch 'jp/fetch-cull-many-refs'
* jp/fetch-cull-many-refs: remote: fix use-after-free error detected by glibc in ref_remove_duplicates fetch: Speed up fetch of large numbers of refs remote: Make ref_remove_duplicates faster for large numbers of refs
2 parents 7a0d61b + 95c96d4 commit 1b8dbdb

3 files changed

Lines changed: 49 additions & 22 deletions

File tree

‎builtin-fetch.c‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -489,7 +489,8 @@ static int add_existing(const char *refname, const unsigned char *sha1,
489489
int flag, void *cbdata)
490490
{
491491
struct string_list *list = (struct string_list *)cbdata;
492-
string_list_insert(refname, list);
492+
struct string_list_item *item = string_list_insert(refname, list);
493+
item->util = (void *)sha1;
493494
return 0;
494495
}
495496

@@ -615,9 +616,14 @@ static void check_not_current_branch(struct ref *ref_map)
615616
static int do_fetch(struct transport *transport,
616617
struct refspec *refs, int ref_count)
617618
{
619+
struct string_list existing_refs = { NULL, 0, 0, 0 };
620+
struct string_list_item *peer_item = NULL;
618621
struct ref *ref_map;
619622
struct ref *rm;
620623
int autotags = (transport->remote->fetch_tags == 1);
624+
625+
for_each_ref(add_existing, &existing_refs);
626+
621627
if (transport->remote->fetch_tags == 2 && tags != TAGS_UNSET)
622628
tags = TAGS_SET;
623629
if (transport->remote->fetch_tags == -1)
@@ -640,8 +646,13 @@ static int do_fetch(struct transport *transport,
640646
check_not_current_branch(ref_map);
641647

642648
for (rm = ref_map; rm; rm = rm->next) {
643-
if (rm->peer_ref)
644-
read_ref(rm->peer_ref->name, rm->peer_ref->old_sha1);
649+
if (rm->peer_ref) {
650+
peer_item = string_list_lookup(rm->peer_ref->name,
651+
&existing_refs);
652+
if (peer_item)
653+
hashcpy(rm->peer_ref->old_sha1,
654+
peer_item->util);
655+
}
645656
}
646657

647658
if (tags == TAGS_DEFAULT && autotags)

‎remote.c‎

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include "revision.h"
77
#include "dir.h"
88
#include "tag.h"
9+
#include "string-list.h"
910

1011
static struct refspec s_tag_refspec = {
1112
0,
@@ -734,29 +735,33 @@ int for_each_remote(each_remote_fn fn, void *priv)
734735

735736
void ref_remove_duplicates(struct ref *ref_map)
736737
{
737-
struct ref **posn;
738-
struct ref *next;
739-
for (; ref_map; ref_map = ref_map->next) {
738+
struct string_list refs = { NULL, 0, 0, 0 };
739+
struct string_list_item *item = NULL;
740+
struct ref *prev = NULL, *next = NULL;
741+
for (; ref_map; prev = ref_map, ref_map = next) {
742+
next = ref_map->next;
740743
if (!ref_map->peer_ref)
741744
continue;
742-
posn = &ref_map->next;
743-
while (*posn) {
744-
if ((*posn)->peer_ref &&
745-
!strcmp((*posn)->peer_ref->name,
746-
ref_map->peer_ref->name)) {
747-
if (strcmp((*posn)->name, ref_map->name))
748-
die("%s tracks both %s and %s",
749-
ref_map->peer_ref->name,
750-
(*posn)->name, ref_map->name);
751-
next = (*posn)->next;
752-
free((*posn)->peer_ref);
753-
free(*posn);
754-
*posn = next;
755-
} else {
756-
posn = &(*posn)->next;
757-
}
745+
746+
item = string_list_lookup(ref_map->peer_ref->name, &refs);
747+
if (item) {
748+
if (strcmp(((struct ref *)item->util)->name,
749+
ref_map->name))
750+
die("%s tracks both %s and %s",
751+
ref_map->peer_ref->name,
752+
((struct ref *)item->util)->name,
753+
ref_map->name);
754+
prev->next = ref_map->next;
755+
free(ref_map->peer_ref);
756+
free(ref_map);
757+
ref_map = prev; /* skip this; we freed it */
758+
continue;
758759
}
760+
761+
item = string_list_insert(ref_map->peer_ref->name, &refs);
762+
item->util = ref_map;
759763
}
764+
string_list_clear(&refs, 0);
760765
}
761766

762767
int remote_has_url(struct remote *remote, const char *url)

‎t/t5510-fetch.sh‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,4 +341,15 @@ test_expect_success 'fetch into the current branch with --update-head-ok' '
341341
342342
'
343343

344+
test_expect_success "should be able to fetch with duplicate refspecs" '
345+
mkdir dups &&
346+
cd dups &&
347+
git init &&
348+
git config branch.master.remote three &&
349+
git config remote.three.url ../three/.git &&
350+
git config remote.three.fetch +refs/heads/*:refs/remotes/origin/* &&
351+
git config --add remote.three.fetch +refs/heads/*:refs/remotes/origin/* &&
352+
git fetch three
353+
'
354+
344355
test_done

0 commit comments

Comments
 (0)