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

diff --git a/builtin-fetch.c b/builtin-fetch.c
index f871f2bfe8b81a508c8850c607a88249a63933f6..2728860399460243cdd57ebca995f418b4c19848 100644 (file)
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -489,7 +489,8 @@ static int add_existing(const char *refname, const unsigned char *sha1,
                        int flag, void *cbdata)
 {
        struct string_list *list = (struct string_list *)cbdata;
-       string_list_insert(refname, list);
+       struct string_list_item *item = string_list_insert(refname, list);
+       item->util = (void *)sha1;
        return 0;
 }
 
@@ -615,9 +616,14 @@ static void check_not_current_branch(struct ref *ref_map)
 static int do_fetch(struct transport *transport,
                    struct refspec *refs, int ref_count)
 {
+       struct string_list existing_refs = { NULL, 0, 0, 0 };
+       struct string_list_item *peer_item = NULL;
        struct ref *ref_map;
        struct ref *rm;
        int autotags = (transport->remote->fetch_tags == 1);
+
+       for_each_ref(add_existing, &existing_refs);
+
        if (transport->remote->fetch_tags == 2 && tags != TAGS_UNSET)
                tags = TAGS_SET;
        if (transport->remote->fetch_tags == -1)
@@ -640,8 +646,13 @@ static int do_fetch(struct transport *transport,
                check_not_current_branch(ref_map);
 
        for (rm = ref_map; rm; rm = rm->next) {
-               if (rm->peer_ref)
-                       read_ref(rm->peer_ref->name, rm->peer_ref->old_sha1);
+               if (rm->peer_ref) {
+                       peer_item = string_list_lookup(rm->peer_ref->name,
+                                                      &existing_refs);
+                       if (peer_item)
+                               hashcpy(rm->peer_ref->old_sha1,
+                                       peer_item->util);
+               }
        }
 
        if (tags == TAGS_DEFAULT && autotags)

diff --git a/remote.c b/remote.c
index 73d33f2584b061085fcaa0958c26b1c3d4665904..e0d17bb83060561c17114905c253eebdf925a01c 100644 (file)
--- a/remote.c
+++ b/remote.c
@@ -6,6 +6,7 @@
 #include "revision.h"
 #include "dir.h"
 #include "tag.h"
+#include "string-list.h"
 
 static struct refspec s_tag_refspec = {
        0,
@@ -734,29 +735,33 @@ int for_each_remote(each_remote_fn fn, void *priv)
 
 void ref_remove_duplicates(struct ref *ref_map)
 {
-       struct ref **posn;
-       struct ref *next;
-       for (; ref_map; ref_map = ref_map->next) {
+       struct string_list refs = { NULL, 0, 0, 0 };
+       struct string_list_item *item = NULL;
+       struct ref *prev = NULL, *next = NULL;
+       for (; ref_map; prev = ref_map, ref_map = next) {
+               next = ref_map->next;
                if (!ref_map->peer_ref)
                        continue;
-               posn = &ref_map->next;
-               while (*posn) {
-                       if ((*posn)->peer_ref &&
-                           !strcmp((*posn)->peer_ref->name,
-                                   ref_map->peer_ref->name)) {
-                               if (strcmp((*posn)->name, ref_map->name))
-                                       die("%s tracks both %s and %s",
-                                           ref_map->peer_ref->name,
-                                           (*posn)->name, ref_map->name);
-                               next = (*posn)->next;
-                               free((*posn)->peer_ref);
-                               free(*posn);
-                               *posn = next;
-                       } else {
-                               posn = &(*posn)->next;
-                       }
+
+               item = string_list_lookup(ref_map->peer_ref->name, &refs);
+               if (item) {
+                       if (strcmp(((struct ref *)item->util)->name,
+                                  ref_map->name))
+                               die("%s tracks both %s and %s",
+                                   ref_map->peer_ref->name,
+                                   ((struct ref *)item->util)->name,
+                                   ref_map->name);
+                       prev->next = ref_map->next;
+                       free(ref_map->peer_ref);
+                       free(ref_map);
+                       ref_map = prev; /* skip this; we freed it */
+                       continue;
                }
+
+               item = string_list_insert(ref_map->peer_ref->name, &refs);
+               item->util = ref_map;
        }
+       string_list_clear(&refs, 0);
 }
 
 int remote_has_url(struct remote *remote, const char *url)

diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index d13c806624bcc8a404be97d61500e8e1d4614c6b..169af1edde557f054ea76b8de681c6dd74e436f2 100755 (executable)
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -341,4 +341,15 @@ test_expect_success 'fetch into the current branch with --update-head-ok' '
 
 '
 
+test_expect_success "should be able to fetch with duplicate refspecs" '
+        mkdir dups &&
+        cd dups &&
+        git init &&
+        git config branch.master.remote three &&
+        git config remote.three.url ../three/.git &&
+        git config remote.three.fetch +refs/heads/*:refs/remotes/origin/* &&
+        git config --add remote.three.fetch +refs/heads/*:refs/remotes/origin/* &&
+        git fetch three
+'
+
 test_done