From 8464f94aeb2a5eb1ba834460b695b171b95e3fc0 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Fri, 20 Sep 2019 17:03:48 -0700 Subject: [PATCH 1/2] promisor-remote.h: drop extern from function declaration During the creation of this file, each time a new function declaration was introduced, it included an `extern`. However, starting from 554544276a (*.[ch]: remove extern from function declarations using spatch, 2019-04-29), we've been actively trying to prevent externs from being used in function declarations because they're unnecessary. Remove these spurious `extern`s. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- promisor-remote.h | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/promisor-remote.h b/promisor-remote.h index 8200dfc940..c60aaa5cce 100644 --- a/promisor-remote.h +++ b/promisor-remote.h @@ -15,17 +15,17 @@ struct promisor_remote { const char name[FLEX_ARRAY]; }; -extern void promisor_remote_reinit(void); -extern struct promisor_remote *promisor_remote_find(const char *remote_name); -extern int has_promisor_remote(void); -extern int promisor_remote_get_direct(struct repository *repo, - const struct object_id *oids, - int oid_nr); +void promisor_remote_reinit(void); +struct promisor_remote *promisor_remote_find(const char *remote_name); +int has_promisor_remote(void); +int promisor_remote_get_direct(struct repository *repo, + const struct object_id *oids, + int oid_nr); /* * This should be used only once from setup.c to set the value we got * from the extensions.partialclone config option. */ -extern void set_repository_format_partial_clone(char *partial_clone); +void set_repository_format_partial_clone(char *partial_clone); #endif /* PROMISOR_REMOTE_H */ From 65904b8b2b27c71a96d8a9c37c19bcc8f2d1380c Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Mon, 30 Sep 2019 15:03:55 -0700 Subject: [PATCH 2/2] promisor-remote: skip move_to_tail when no-op Previously, when promisor_remote_move_to_tail() is called for a promisor_remote which is currently the final element in promisors, a cycle is created in the promisors linked list. This cycle leads to a double free later on in promisor_remote_clear() when the final element of the promisors list is removed: promisors is set to promisors->next (a no-op, as promisors->next == promisors); the previous value of promisors is free()'d; then the new value of promisors (which is equal to the previous value of promisors) is also free()'d. This double-free error was unrecoverable for the user without removing the filter or re-cloning the repo and hoping to miss this edge case. Now, when promisor_remote_move_to_tail() would be a no-op, just do a no-op. In cases of promisor_remote_move_to_tail() where r is not already at the tail of the list, it works as before. Helped-by: Jeff King Signed-off-by: Emily Shaffer Acked-by: Christian Couder Signed-off-by: Junio C Hamano --- promisor-remote.c | 3 +++ t/t0410-partial-clone.sh | 13 +++++++++++++ 2 files changed, 16 insertions(+) diff --git a/promisor-remote.c b/promisor-remote.c index 9bc296cdde..9bd5b79d59 100644 --- a/promisor-remote.c +++ b/promisor-remote.c @@ -89,6 +89,9 @@ static struct promisor_remote *promisor_remote_lookup(const char *remote_name, static void promisor_remote_move_to_tail(struct promisor_remote *r, struct promisor_remote *previous) { + if (r->next == NULL) + return; + if (previous) previous->next = r->next; else diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh index 2498e72a34..c5b2018a30 100755 --- a/t/t0410-partial-clone.sh +++ b/t/t0410-partial-clone.sh @@ -429,6 +429,19 @@ test_expect_success 'rev-list dies for missing objects on cmd line' ' done ' +test_expect_success 'single promisor remote can be re-initialized gracefully' ' + # ensure one promisor is in the promisors list + rm -rf repo && + test_create_repo repo && + test_create_repo other && + git -C repo remote add foo "file://$(pwd)/other" && + git -C repo config remote.foo.promisor true && + git -C repo config extensions.partialclone foo && + + # reinitialize the promisors list + git -C repo fetch --filter=blob:none foo +' + test_expect_success 'gc repacks promisor objects separately from non-promisor objects' ' rm -rf repo && test_create_repo repo &&