From 620a66b9e2d68433aab2ae3daf5a62896c0547d8 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 10 Feb 2017 12:16:11 +0100 Subject: [PATCH 1/9] refs: reorder some function definitions This avoids the need to add forward declarations in the next step. Signed-off-by: Michael Haggerty Reviewed-by: Jeff King Signed-off-by: Junio C Hamano --- refs.c | 64 +++++++++++++++++++++++++++++----------------------------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/refs.c b/refs.c index 9bd0bc177b..707092f99b 100644 --- a/refs.c +++ b/refs.c @@ -1358,27 +1358,19 @@ static struct ref_store *main_ref_store; /* A linked list of ref_stores for submodules: */ static struct ref_store *submodule_ref_stores; -void base_ref_store_init(struct ref_store *refs, - const struct ref_storage_be *be, - const char *submodule) +struct ref_store *lookup_ref_store(const char *submodule) { - refs->be = be; - if (!submodule) { - if (main_ref_store) - die("BUG: main_ref_store initialized twice"); + struct ref_store *refs; - refs->submodule = ""; - refs->next = NULL; - main_ref_store = refs; - } else { - if (lookup_ref_store(submodule)) - die("BUG: ref_store for submodule '%s' initialized twice", - submodule); + if (!submodule || !*submodule) + return main_ref_store; - refs->submodule = xstrdup(submodule); - refs->next = submodule_ref_stores; - submodule_ref_stores = refs; + for (refs = submodule_ref_stores; refs; refs = refs->next) { + if (!strcmp(submodule, refs->submodule)) + return refs; } + + return NULL; } struct ref_store *ref_store_init(const char *submodule) @@ -1395,21 +1387,6 @@ struct ref_store *ref_store_init(const char *submodule) return be->init(submodule); } -struct ref_store *lookup_ref_store(const char *submodule) -{ - struct ref_store *refs; - - if (!submodule || !*submodule) - return main_ref_store; - - for (refs = submodule_ref_stores; refs; refs = refs->next) { - if (!strcmp(submodule, refs->submodule)) - return refs; - } - - return NULL; -} - struct ref_store *get_ref_store(const char *submodule) { struct ref_store *refs; @@ -1435,6 +1412,29 @@ struct ref_store *get_ref_store(const char *submodule) return refs; } +void base_ref_store_init(struct ref_store *refs, + const struct ref_storage_be *be, + const char *submodule) +{ + refs->be = be; + if (!submodule) { + if (main_ref_store) + die("BUG: main_ref_store initialized twice"); + + refs->submodule = ""; + refs->next = NULL; + main_ref_store = refs; + } else { + if (lookup_ref_store(submodule)) + die("BUG: ref_store for submodule '%s' initialized twice", + submodule); + + refs->submodule = xstrdup(submodule); + refs->next = submodule_ref_stores; + submodule_ref_stores = refs; + } +} + void assert_main_repository(struct ref_store *refs, const char *caller) { if (*refs->submodule) From c468da4e2796868362c2f33b499d9bb6d6cd7043 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 10 Feb 2017 12:16:12 +0100 Subject: [PATCH 2/9] refs: make some ref_store lookup functions private The following functions currently don't need to be exposed: * ref_store_init() * lookup_ref_store() That might change in the future, but for now make them private. Signed-off-by: Michael Haggerty Reviewed-by: Jeff King Signed-off-by: Junio C Hamano --- refs.c | 19 +++++++++++++++++-- refs/refs-internal.h | 19 ------------------- 2 files changed, 17 insertions(+), 21 deletions(-) diff --git a/refs.c b/refs.c index 707092f99b..d7265ccc8f 100644 --- a/refs.c +++ b/refs.c @@ -1358,7 +1358,15 @@ static struct ref_store *main_ref_store; /* A linked list of ref_stores for submodules: */ static struct ref_store *submodule_ref_stores; -struct ref_store *lookup_ref_store(const char *submodule) +/* + * Return the ref_store instance for the specified submodule (or the + * main repository if submodule is NULL). If that ref_store hasn't + * been initialized yet, return NULL. + * + * For backwards compatibility, submodule=="" is treated the same as + * submodule==NULL. + */ +static struct ref_store *lookup_ref_store(const char *submodule) { struct ref_store *refs; @@ -1373,7 +1381,14 @@ struct ref_store *lookup_ref_store(const char *submodule) return NULL; } -struct ref_store *ref_store_init(const char *submodule) +/* + * Create, record, and return a ref_store instance for the specified + * submodule (or the main repository if submodule is NULL). + * + * For backwards compatibility, submodule=="" is treated the same as + * submodule==NULL. + */ +static struct ref_store *ref_store_init(const char *submodule) { const char *be_name = "files"; struct ref_storage_be *be = find_ref_storage_backend(be_name); diff --git a/refs/refs-internal.h b/refs/refs-internal.h index 708b26082a..d8a7eb1841 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -652,25 +652,6 @@ void base_ref_store_init(struct ref_store *refs, const struct ref_storage_be *be, const char *submodule); -/* - * Create, record, and return a ref_store instance for the specified - * submodule (or the main repository if submodule is NULL). - * - * For backwards compatibility, submodule=="" is treated the same as - * submodule==NULL. - */ -struct ref_store *ref_store_init(const char *submodule); - -/* - * Return the ref_store instance for the specified submodule (or the - * main repository if submodule is NULL). If that ref_store hasn't - * been initialized yet, return NULL. - * - * For backwards compatibility, submodule=="" is treated the same as - * submodule==NULL. - */ -struct ref_store *lookup_ref_store(const char *submodule); - /* * Return the ref_store instance for the specified submodule. For the * main repository, use submodule==NULL; such a call cannot fail. For From b4f540b6cefdb33f35ffd5f8b2fb459c184a1bd7 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 10 Feb 2017 12:16:13 +0100 Subject: [PATCH 3/9] refs: remove some unnecessary handling of submodule == "" The only external entry point to the ref_store lookup functions is get_ref_store(), which ensures that submodule == "" is passed along as NULL. So ref_store_init() and lookup_ref_store() don't have to handle submodule being specified as the empty string. Signed-off-by: Michael Haggerty Reviewed-by: Jeff King Signed-off-by: Junio C Hamano --- refs.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/refs.c b/refs.c index d7265ccc8f..63484140f6 100644 --- a/refs.c +++ b/refs.c @@ -1362,15 +1362,12 @@ static struct ref_store *submodule_ref_stores; * Return the ref_store instance for the specified submodule (or the * main repository if submodule is NULL). If that ref_store hasn't * been initialized yet, return NULL. - * - * For backwards compatibility, submodule=="" is treated the same as - * submodule==NULL. */ static struct ref_store *lookup_ref_store(const char *submodule) { struct ref_store *refs; - if (!submodule || !*submodule) + if (!submodule) return main_ref_store; for (refs = submodule_ref_stores; refs; refs = refs->next) { @@ -1384,9 +1381,6 @@ static struct ref_store *lookup_ref_store(const char *submodule) /* * Create, record, and return a ref_store instance for the specified * submodule (or the main repository if submodule is NULL). - * - * For backwards compatibility, submodule=="" is treated the same as - * submodule==NULL. */ static struct ref_store *ref_store_init(const char *submodule) { @@ -1396,10 +1390,7 @@ static struct ref_store *ref_store_init(const char *submodule) if (!be) die("BUG: reference backend %s is unknown", be_name); - if (!submodule || !*submodule) - return be->init(NULL); - else - return be->init(submodule); + return be->init(submodule); } struct ref_store *get_ref_store(const char *submodule) From ba88add58176a883e4137ee9bb5e66235799a9d9 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 10 Feb 2017 12:16:14 +0100 Subject: [PATCH 4/9] register_ref_store(): new function Move the responsibility for registering the ref_store for a submodule from base_ref_store_init() to a new function, register_ref_store(). Call the latter from ref_store_init(). Signed-off-by: Michael Haggerty Reviewed-by: Jeff King Signed-off-by: Junio C Hamano --- refs.c | 43 +++++++++++++++++++++++++++++-------------- 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/refs.c b/refs.c index 63484140f6..d7158b6289 100644 --- a/refs.c +++ b/refs.c @@ -1378,6 +1378,29 @@ static struct ref_store *lookup_ref_store(const char *submodule) return NULL; } +/* + * Register the specified ref_store to be the one that should be used + * for submodule (or the main repository if submodule is NULL). It is + * a fatal error to call this function twice for the same submodule. + */ +static void register_ref_store(struct ref_store *refs, const char *submodule) +{ + if (!submodule) { + if (main_ref_store) + die("BUG: main_ref_store initialized twice"); + + refs->next = NULL; + main_ref_store = refs; + } else { + if (lookup_ref_store(submodule)) + die("BUG: ref_store for submodule '%s' initialized twice", + submodule); + + refs->next = submodule_ref_stores; + submodule_ref_stores = refs; + } +} + /* * Create, record, and return a ref_store instance for the specified * submodule (or the main repository if submodule is NULL). @@ -1386,11 +1409,14 @@ static struct ref_store *ref_store_init(const char *submodule) { const char *be_name = "files"; struct ref_storage_be *be = find_ref_storage_backend(be_name); + struct ref_store *refs; if (!be) die("BUG: reference backend %s is unknown", be_name); - return be->init(submodule); + refs = be->init(submodule); + register_ref_store(refs, submodule); + return refs; } struct ref_store *get_ref_store(const char *submodule) @@ -1423,22 +1449,11 @@ void base_ref_store_init(struct ref_store *refs, const char *submodule) { refs->be = be; - if (!submodule) { - if (main_ref_store) - die("BUG: main_ref_store initialized twice"); + if (!submodule) refs->submodule = ""; - refs->next = NULL; - main_ref_store = refs; - } else { - if (lookup_ref_store(submodule)) - die("BUG: ref_store for submodule '%s' initialized twice", - submodule); - + else refs->submodule = xstrdup(submodule); - refs->next = submodule_ref_stores; - submodule_ref_stores = refs; - } } void assert_main_repository(struct ref_store *refs, const char *caller) From 7d4558c462f0d1a280abec59e8c64a2f2b42a9f2 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 10 Feb 2017 12:16:15 +0100 Subject: [PATCH 5/9] refs: store submodule ref stores in a hashmap Aside from scaling better, this means that the submodule name needn't be stored in the ref_store instance anymore (which will be changed in a moment). This, in turn, will help loosen the strict 1:1 relationship between ref_stores and submodules. Signed-off-by: Michael Haggerty Reviewed-by: Jeff King Signed-off-by: Junio C Hamano --- refs.c | 58 ++++++++++++++++++++++++++++++++++---------- refs/refs-internal.h | 6 ----- 2 files changed, 45 insertions(+), 19 deletions(-) diff --git a/refs.c b/refs.c index d7158b6289..5121c57b78 100644 --- a/refs.c +++ b/refs.c @@ -3,6 +3,7 @@ */ #include "cache.h" +#include "hashmap.h" #include "lockfile.h" #include "refs.h" #include "refs/refs-internal.h" @@ -1352,11 +1353,41 @@ int resolve_gitlink_ref(const char *submodule, const char *refname, return 0; } +struct submodule_hash_entry +{ + struct hashmap_entry ent; /* must be the first member! */ + + struct ref_store *refs; + + /* NUL-terminated name of submodule: */ + char submodule[FLEX_ARRAY]; +}; + +static int submodule_hash_cmp(const void *entry, const void *entry_or_key, + const void *keydata) +{ + const struct submodule_hash_entry *e1 = entry, *e2 = entry_or_key; + const char *submodule = keydata ? keydata : e2->submodule; + + return strcmp(e1->submodule, submodule); +} + +static struct submodule_hash_entry *alloc_submodule_hash_entry( + const char *submodule, struct ref_store *refs) +{ + struct submodule_hash_entry *entry; + + FLEX_ALLOC_STR(entry, submodule, submodule); + hashmap_entry_init(entry, strhash(submodule)); + entry->refs = refs; + return entry; +} + /* A pointer to the ref_store for the main repository: */ static struct ref_store *main_ref_store; -/* A linked list of ref_stores for submodules: */ -static struct ref_store *submodule_ref_stores; +/* A hashmap of ref_stores, stored by submodule name: */ +static struct hashmap submodule_ref_stores; /* * Return the ref_store instance for the specified submodule (or the @@ -1365,17 +1396,18 @@ static struct ref_store *submodule_ref_stores; */ static struct ref_store *lookup_ref_store(const char *submodule) { - struct ref_store *refs; + struct submodule_hash_entry *entry; if (!submodule) return main_ref_store; - for (refs = submodule_ref_stores; refs; refs = refs->next) { - if (!strcmp(submodule, refs->submodule)) - return refs; - } + if (!submodule_ref_stores.tablesize) + /* It's initialized on demand in register_ref_store(). */ + return NULL; - return NULL; + entry = hashmap_get_from_hash(&submodule_ref_stores, + strhash(submodule), submodule); + return entry ? entry->refs : NULL; } /* @@ -1389,15 +1421,15 @@ static void register_ref_store(struct ref_store *refs, const char *submodule) if (main_ref_store) die("BUG: main_ref_store initialized twice"); - refs->next = NULL; main_ref_store = refs; } else { - if (lookup_ref_store(submodule)) + if (!submodule_ref_stores.tablesize) + hashmap_init(&submodule_ref_stores, submodule_hash_cmp, 0); + + if (hashmap_put(&submodule_ref_stores, + alloc_submodule_hash_entry(submodule, refs))) die("BUG: ref_store for submodule '%s' initialized twice", submodule); - - refs->next = submodule_ref_stores; - submodule_ref_stores = refs; } } diff --git a/refs/refs-internal.h b/refs/refs-internal.h index d8a7eb1841..07fd208f2c 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -636,12 +636,6 @@ struct ref_store { * reference store: */ const char *submodule; - - /* - * Submodule reference store instances are stored in a linked - * list using this pointer. - */ - struct ref_store *next; }; /* From 32c597e7b2d85c1ae313e369b21398638cafe45d Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 10 Feb 2017 12:16:16 +0100 Subject: [PATCH 6/9] refs: push the submodule attribute down Push the submodule attribute down from ref_store to files_ref_store. This is another step towards loosening the 1:1 connection between ref_stores and submodules. Signed-off-by: Michael Haggerty Reviewed-by: Jeff King Signed-off-by: Junio C Hamano --- refs.c | 11 -------- refs/files-backend.c | 61 +++++++++++++++++++++++++++++++------------- refs/refs-internal.h | 13 ---------- 3 files changed, 43 insertions(+), 42 deletions(-) diff --git a/refs.c b/refs.c index 5121c57b78..07959ff3e6 100644 --- a/refs.c +++ b/refs.c @@ -1481,17 +1481,6 @@ void base_ref_store_init(struct ref_store *refs, const char *submodule) { refs->be = be; - - if (!submodule) - refs->submodule = ""; - else - refs->submodule = xstrdup(submodule); -} - -void assert_main_repository(struct ref_store *refs, const char *caller) -{ - if (*refs->submodule) - die("BUG: %s called for a submodule", caller); } /* backend functions */ diff --git a/refs/files-backend.c b/refs/files-backend.c index f9023939d5..5e135a49aa 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -912,6 +912,14 @@ struct packed_ref_cache { */ struct files_ref_store { struct ref_store base; + + /* + * The name of the submodule represented by this object, or + * the empty string if it represents the main repository's + * reference store: + */ + const char *submodule; + struct ref_entry *loose; struct packed_ref_cache *packed; }; @@ -974,9 +982,22 @@ static struct ref_store *files_ref_store_create(const char *submodule) base_ref_store_init(ref_store, &refs_be_files, submodule); + refs->submodule = submodule ? xstrdup(submodule) : ""; + return ref_store; } +/* + * Die if refs is for a submodule (i.e., not for the main repository). + * caller is used in any necessary error messages. + */ +static void files_assert_main_repository(struct files_ref_store *refs, + const char *caller) +{ + if (*refs->submodule) + die("BUG: %s called for a submodule", caller); +} + /* * Downcast ref_store to files_ref_store. Die if ref_store is not a * files_ref_store. If submodule_allowed is not true, then also die if @@ -987,14 +1008,18 @@ static struct files_ref_store *files_downcast( struct ref_store *ref_store, int submodule_allowed, const char *caller) { + struct files_ref_store *refs; + if (ref_store->be != &refs_be_files) die("BUG: ref_store is type \"%s\" not \"files\" in %s", ref_store->be->name, caller); + refs = (struct files_ref_store *)ref_store; + if (!submodule_allowed) - assert_main_repository(ref_store, caller); + files_assert_main_repository(refs, caller); - return (struct files_ref_store *)ref_store; + return refs; } /* The length of a peeled reference line in packed-refs, including EOL: */ @@ -1133,8 +1158,8 @@ static struct packed_ref_cache *get_packed_ref_cache(struct files_ref_store *ref { char *packed_refs_file; - if (*refs->base.submodule) - packed_refs_file = git_pathdup_submodule(refs->base.submodule, + if (*refs->submodule) + packed_refs_file = git_pathdup_submodule(refs->submodule, "packed-refs"); else packed_refs_file = git_pathdup("packed-refs"); @@ -1203,8 +1228,8 @@ static void read_loose_refs(const char *dirname, struct ref_dir *dir) size_t path_baselen; int err = 0; - if (*refs->base.submodule) - err = strbuf_git_path_submodule(&path, refs->base.submodule, "%s", dirname); + if (*refs->submodule) + err = strbuf_git_path_submodule(&path, refs->submodule, "%s", dirname); else strbuf_git_path(&path, "%s", dirname); path_baselen = path.len; @@ -1244,10 +1269,10 @@ static void read_loose_refs(const char *dirname, struct ref_dir *dir) } else { int read_ok; - if (*refs->base.submodule) { + if (*refs->submodule) { hashclr(sha1); flag = 0; - read_ok = !resolve_gitlink_ref(refs->base.submodule, + read_ok = !resolve_gitlink_ref(refs->submodule, refname.buf, sha1); } else { read_ok = !read_ref_full(refname.buf, @@ -1358,8 +1383,8 @@ static int files_read_raw_ref(struct ref_store *ref_store, *type = 0; strbuf_reset(&sb_path); - if (*refs->base.submodule) - strbuf_git_path_submodule(&sb_path, refs->base.submodule, "%s", refname); + if (*refs->submodule) + strbuf_git_path_submodule(&sb_path, refs->submodule, "%s", refname); else strbuf_git_path(&sb_path, "%s", refname); @@ -1540,7 +1565,7 @@ static int lock_raw_ref(struct files_ref_store *refs, int ret = TRANSACTION_GENERIC_ERROR; assert(err); - assert_main_repository(&refs->base, "lock_raw_ref"); + files_assert_main_repository(refs, "lock_raw_ref"); *type = 0; @@ -2006,7 +2031,7 @@ static struct ref_lock *lock_ref_sha1_basic(struct files_ref_store *refs, int attempts_remaining = 3; int resolved; - assert_main_repository(&refs->base, "lock_ref_sha1_basic"); + files_assert_main_repository(refs, "lock_ref_sha1_basic"); assert(err); lock = xcalloc(1, sizeof(struct ref_lock)); @@ -2152,7 +2177,7 @@ static int lock_packed_refs(struct files_ref_store *refs, int flags) static int timeout_value = 1000; struct packed_ref_cache *packed_ref_cache; - assert_main_repository(&refs->base, "lock_packed_refs"); + files_assert_main_repository(refs, "lock_packed_refs"); if (!timeout_configured) { git_config_get_int("core.packedrefstimeout", &timeout_value); @@ -2190,7 +2215,7 @@ static int commit_packed_refs(struct files_ref_store *refs) int save_errno = 0; FILE *out; - assert_main_repository(&refs->base, "commit_packed_refs"); + files_assert_main_repository(refs, "commit_packed_refs"); if (!packed_ref_cache->lock) die("internal error: packed-refs not locked"); @@ -2223,7 +2248,7 @@ static void rollback_packed_refs(struct files_ref_store *refs) struct packed_ref_cache *packed_ref_cache = get_packed_ref_cache(refs); - assert_main_repository(&refs->base, "rollback_packed_refs"); + files_assert_main_repository(refs, "rollback_packed_refs"); if (!packed_ref_cache->lock) die("internal error: packed-refs not locked"); @@ -2397,7 +2422,7 @@ static int repack_without_refs(struct files_ref_store *refs, struct string_list_item *refname; int ret, needs_repacking = 0, removed = 0; - assert_main_repository(&refs->base, "repack_without_refs"); + files_assert_main_repository(refs, "repack_without_refs"); assert(err); /* Look for a packed ref */ @@ -2930,7 +2955,7 @@ static int commit_ref_update(struct files_ref_store *refs, const unsigned char *sha1, const char *logmsg, struct strbuf *err) { - assert_main_repository(&refs->base, "commit_ref_update"); + files_assert_main_repository(refs, "commit_ref_update"); clear_loose_ref_cache(refs); if (log_ref_write(lock->ref_name, lock->old_oid.hash, sha1, logmsg, 0, err)) { @@ -3560,7 +3585,7 @@ static int lock_ref_for_update(struct files_ref_store *refs, int ret; struct ref_lock *lock; - assert_main_repository(&refs->base, "lock_ref_for_update"); + files_assert_main_repository(refs, "lock_ref_for_update"); if ((update->flags & REF_HAVE_NEW) && is_null_sha1(update->new_sha1)) update->flags |= REF_DELETING; diff --git a/refs/refs-internal.h b/refs/refs-internal.h index 07fd208f2c..008822d67c 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -629,13 +629,6 @@ extern struct ref_storage_be refs_be_files; struct ref_store { /* The backend describing this ref_store's storage scheme: */ const struct ref_storage_be *be; - - /* - * The name of the submodule represented by this object, or - * the empty string if it represents the main repository's - * reference store: - */ - const char *submodule; }; /* @@ -658,10 +651,4 @@ void base_ref_store_init(struct ref_store *refs, */ struct ref_store *get_ref_store(const char *submodule); -/* - * Die if refs is for a submodule (i.e., not for the main repository). - * caller is used in any necessary error messages. - */ -void assert_main_repository(struct ref_store *refs, const char *caller); - #endif /* REFS_REFS_INTERNAL_H */ From fbfd0a291596a698773d3f687af3ab4c36a841bf Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 10 Feb 2017 12:16:17 +0100 Subject: [PATCH 7/9] base_ref_store_init(): remove submodule argument This is another step towards weakening the 1:1 relationship between ref_stores and submodules. Signed-off-by: Michael Haggerty Reviewed-by: Jeff King Signed-off-by: Junio C Hamano --- refs.c | 3 +-- refs/files-backend.c | 2 +- refs/refs-internal.h | 7 +++---- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/refs.c b/refs.c index 07959ff3e6..05af56b50a 100644 --- a/refs.c +++ b/refs.c @@ -1477,8 +1477,7 @@ struct ref_store *get_ref_store(const char *submodule) } void base_ref_store_init(struct ref_store *refs, - const struct ref_storage_be *be, - const char *submodule) + const struct ref_storage_be *be) { refs->be = be; } diff --git a/refs/files-backend.c b/refs/files-backend.c index 5e135a49aa..c9d2d28488 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -980,7 +980,7 @@ static struct ref_store *files_ref_store_create(const char *submodule) struct files_ref_store *refs = xcalloc(1, sizeof(*refs)); struct ref_store *ref_store = (struct ref_store *)refs; - base_ref_store_init(ref_store, &refs_be_files, submodule); + base_ref_store_init(ref_store, &refs_be_files); refs->submodule = submodule ? xstrdup(submodule) : ""; diff --git a/refs/refs-internal.h b/refs/refs-internal.h index 008822d67c..793c850e98 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -632,12 +632,11 @@ struct ref_store { }; /* - * Fill in the generic part of refs for the specified submodule and - * add it to our collection of reference stores. + * Fill in the generic part of refs and add it to our collection of + * reference stores. */ void base_ref_store_init(struct ref_store *refs, - const struct ref_storage_be *be, - const char *submodule); + const struct ref_storage_be *be); /* * Return the ref_store instance for the specified submodule. For the From 9c7d772b6b716ec5f81afd3922340588985066b8 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Fri, 10 Feb 2017 12:16:18 +0100 Subject: [PATCH 8/9] files_ref_store::submodule: use NULL for the main repository The old practice of storing the empty string in this member for the main repository was a holdover from before 00eebe3 (refs: create a base class "ref_store" for files_ref_store, 2016-09-04), when the submodule was stored in a flex array at the end of `struct files_ref_store`. Storing NULL for this case is more idiomatic and a tiny bit less code. Signed-off-by: Michael Haggerty Reviewed-by: Jeff King Signed-off-by: Junio C Hamano --- refs/files-backend.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index c9d2d28488..4fe92f0637 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -915,8 +915,8 @@ struct files_ref_store { /* * The name of the submodule represented by this object, or - * the empty string if it represents the main repository's - * reference store: + * NULL if it represents the main repository's reference + * store: */ const char *submodule; @@ -982,7 +982,7 @@ static struct ref_store *files_ref_store_create(const char *submodule) base_ref_store_init(ref_store, &refs_be_files); - refs->submodule = submodule ? xstrdup(submodule) : ""; + refs->submodule = xstrdup_or_null(submodule); return ref_store; } @@ -994,7 +994,7 @@ static struct ref_store *files_ref_store_create(const char *submodule) static void files_assert_main_repository(struct files_ref_store *refs, const char *caller) { - if (*refs->submodule) + if (refs->submodule) die("BUG: %s called for a submodule", caller); } @@ -1158,7 +1158,7 @@ static struct packed_ref_cache *get_packed_ref_cache(struct files_ref_store *ref { char *packed_refs_file; - if (*refs->submodule) + if (refs->submodule) packed_refs_file = git_pathdup_submodule(refs->submodule, "packed-refs"); else @@ -1228,7 +1228,7 @@ static void read_loose_refs(const char *dirname, struct ref_dir *dir) size_t path_baselen; int err = 0; - if (*refs->submodule) + if (refs->submodule) err = strbuf_git_path_submodule(&path, refs->submodule, "%s", dirname); else strbuf_git_path(&path, "%s", dirname); @@ -1269,7 +1269,7 @@ static void read_loose_refs(const char *dirname, struct ref_dir *dir) } else { int read_ok; - if (*refs->submodule) { + if (refs->submodule) { hashclr(sha1); flag = 0; read_ok = !resolve_gitlink_ref(refs->submodule, @@ -1383,7 +1383,7 @@ static int files_read_raw_ref(struct ref_store *ref_store, *type = 0; strbuf_reset(&sb_path); - if (*refs->submodule) + if (refs->submodule) strbuf_git_path_submodule(&sb_path, refs->submodule, "%s", refname); else strbuf_git_path(&sb_path, "%s", refname); From 3c0cb0cbaef699f699b79c8be716086053760ea9 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Thu, 9 Feb 2017 21:53:52 +0100 Subject: [PATCH 9/9] read_loose_refs(): read refs using resolve_ref_recursively() There is no need to call read_ref_full() or resolve_gitlink_ref() from read_loose_refs(), because we already have a ref_store object in hand. So we can call resolve_ref_recursively() ourselves. Happily, this unifies the code for the submodule vs. non-submodule cases. This requires resolve_ref_recursively() to be exposed to the refs subsystem, though not to non-refs code. Signed-off-by: Michael Haggerty Reviewed-by: Jeff King Signed-off-by: Junio C Hamano --- refs.c | 8 ++++---- refs/files-backend.c | 18 ++++-------------- refs/refs-internal.h | 5 +++++ 3 files changed, 13 insertions(+), 18 deletions(-) diff --git a/refs.c b/refs.c index 05af56b50a..4f845798ba 100644 --- a/refs.c +++ b/refs.c @@ -1230,10 +1230,10 @@ int for_each_rawref(each_ref_fn fn, void *cb_data) } /* This function needs to return a meaningful errno on failure */ -static const char *resolve_ref_recursively(struct ref_store *refs, - const char *refname, - int resolve_flags, - unsigned char *sha1, int *flags) +const char *resolve_ref_recursively(struct ref_store *refs, + const char *refname, + int resolve_flags, + unsigned char *sha1, int *flags) { static struct strbuf sb_refname = STRBUF_INIT; int unused_flags; diff --git a/refs/files-backend.c b/refs/files-backend.c index 4fe92f0637..cdb6b8ff57 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1267,20 +1267,10 @@ static void read_loose_refs(const char *dirname, struct ref_dir *dir) create_dir_entry(refs, refname.buf, refname.len, 1)); } else { - int read_ok; - - if (refs->submodule) { - hashclr(sha1); - flag = 0; - read_ok = !resolve_gitlink_ref(refs->submodule, - refname.buf, sha1); - } else { - read_ok = !read_ref_full(refname.buf, - RESOLVE_REF_READING, - sha1, &flag); - } - - if (!read_ok) { + if (!resolve_ref_recursively(&refs->base, + refname.buf, + RESOLVE_REF_READING, + sha1, &flag)) { hashclr(sha1); flag |= REF_ISBROKEN; } else if (is_null_sha1(sha1)) { diff --git a/refs/refs-internal.h b/refs/refs-internal.h index 793c850e98..33adbf93b5 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -650,4 +650,9 @@ void base_ref_store_init(struct ref_store *refs, */ struct ref_store *get_ref_store(const char *submodule); +const char *resolve_ref_recursively(struct ref_store *refs, + const char *refname, + int resolve_flags, + unsigned char *sha1, int *flags); + #endif /* REFS_REFS_INTERNAL_H */