On 02/12, Stefan Beller wrote:
> In a process with multiple repositories open, alternates should be
> associated to a single repository and not shared globally. Move
> alt_odb_list and alt_odb_tail into the_repository and adjust callers
> to reflect this.
>
> Now that the alternative object data base is per repository, we're
> leaking its memory upon freeing a repository. The next patch plugs
> this hole.
>
> No functional change intended.
>
> Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx>
> Signed-off-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
> ---
> builtin/fsck.c | 4 +++-
> cache.h | 4 ++--
> object-store.h | 7 ++++++-
> packfile.c | 3 ++-
> sha1_file.c | 25 ++++++++++++-------------
> sha1_name.c | 3 ++-
> 6 files changed, 27 insertions(+), 19 deletions(-)
>
> diff --git a/builtin/fsck.c b/builtin/fsck.c
> index 04846d46f9..1048255da1 100644
> --- a/builtin/fsck.c
> +++ b/builtin/fsck.c
> @@ -1,5 +1,6 @@
> #include "builtin.h"
> #include "cache.h"
> +#include "repository.h"
> #include "config.h"
> #include "commit.h"
> #include "tree.h"
> @@ -694,7 +695,8 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
> fsck_object_dir(get_object_directory());
>
> prepare_alt_odb();
> - for (alt = alt_odb_list; alt; alt = alt->next)
> + for (alt = the_repository->objects.alt_odb_list;
> + alt; alt = alt->next)
> fsck_object_dir(alt->path);
>
> if (check_full) {
> diff --git a/cache.h b/cache.h
> index d8b975a571..918c2f15b4 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -1573,7 +1573,7 @@ extern int has_dirs_only_path(const char *name, int len, int prefix_len);
> extern void schedule_dir_for_removal(const char *name, int len);
> extern void remove_scheduled_dirs(void);
>
> -extern struct alternate_object_database {
> +struct alternate_object_database {
> struct alternate_object_database *next;
>
> /* see alt_scratch_buf() */
> @@ -1591,7 +1591,7 @@ extern struct alternate_object_database {
> struct oid_array loose_objects_cache;
>
> char path[FLEX_ARRAY];
> -} *alt_odb_list;
> +};
> extern void prepare_alt_odb(void);
> extern char *compute_alternate_path(const char *path, struct strbuf *err);
> typedef int alt_odb_fn(struct alternate_object_database *, void *);
> diff --git a/object-store.h b/object-store.h
> index cf35760ceb..44b8f84753 100644
> --- a/object-store.h
> +++ b/object-store.h
> @@ -1,14 +1,19 @@
> #ifndef OBJECT_STORE_H
> #define OBJECT_STORE_H
>
> +#include "cache.h"
> +
Maybe we would want to move the definition of the alternate store to
this header file? That way we don't need to include cache.h here.
> struct raw_object_store {
> /*
> * Path to the repository's object store.
> * Cannot be NULL after initialization.
> */
> char *objectdir;
> +
> + struct alternate_object_database *alt_odb_list;
> + struct alternate_object_database **alt_odb_tail;
> };
> -#define RAW_OBJECT_STORE_INIT { NULL }
> +#define RAW_OBJECT_STORE_INIT { NULL, NULL, NULL }
>
> void raw_object_store_clear(struct raw_object_store *o);
>
> diff --git a/packfile.c b/packfile.c
> index 4a5fe7ab18..d61076faaf 100644
> --- a/packfile.c
> +++ b/packfile.c
> @@ -1,6 +1,7 @@
> #include "cache.h"
> #include "mru.h"
> #include "pack.h"
> +#include "repository.h"
> #include "dir.h"
> #include "mergesort.h"
> #include "packfile.h"
> @@ -880,7 +881,7 @@ void prepare_packed_git(void)
> return;
> prepare_packed_git_one(get_object_directory(), 1);
> prepare_alt_odb();
> - for (alt = alt_odb_list; alt; alt = alt->next)
> + for (alt = the_repository->objects.alt_odb_list; alt; alt = alt->next)
> prepare_packed_git_one(alt->path, 0);
> rearrange_packed_git();
> prepare_packed_git_mru();
> diff --git a/sha1_file.c b/sha1_file.c
> index 3da70ac650..2826d5d6ed 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -22,6 +22,7 @@
> #include "pack-revindex.h"
> #include "sha1-lookup.h"
> #include "bulk-checkin.h"
> +#include "repository.h"
> #include "streaming.h"
> #include "dir.h"
> #include "mru.h"
> @@ -346,9 +347,6 @@ static const char *alt_sha1_path(struct alternate_object_database *alt,
> return buf->buf;
> }
>
> -struct alternate_object_database *alt_odb_list;
> -static struct alternate_object_database **alt_odb_tail;
> -
> /*
> * Return non-zero iff the path is usable as an alternate object database.
> */
> @@ -368,7 +366,7 @@ static int alt_odb_usable(struct strbuf *path, const char *normalized_objdir)
> * Prevent the common mistake of listing the same
> * thing twice, or object directory itself.
> */
> - for (alt = alt_odb_list; alt; alt = alt->next) {
> + for (alt = the_repository->objects.alt_odb_list; alt; alt = alt->next) {
> if (!fspathcmp(path->buf, alt->path))
> return 0;
> }
> @@ -428,8 +426,8 @@ static int link_alt_odb_entry(const char *entry, const char *relative_base,
> ent = alloc_alt_odb(pathbuf.buf);
>
> /* add the alternate entry */
> - *alt_odb_tail = ent;
> - alt_odb_tail = &(ent->next);
> + *the_repository->objects.alt_odb_tail = ent;
> + the_repository->objects.alt_odb_tail = &(ent->next);
> ent->next = NULL;
>
> /* recursively add alternates */
> @@ -563,7 +561,7 @@ void add_to_alternates_file(const char *reference)
> fprintf_or_die(out, "%s\n", reference);
> if (commit_lock_file(&lock))
> die_errno("unable to move new alternates file into place");
> - if (alt_odb_tail)
> + if (the_repository->objects.alt_odb_tail)
> link_alt_odb_entries(reference, '\n', NULL, 0);
> }
> free(alts);
> @@ -661,7 +659,7 @@ int foreach_alt_odb(alt_odb_fn fn, void *cb)
> int r = 0;
>
> prepare_alt_odb();
> - for (ent = alt_odb_list; ent; ent = ent->next) {
> + for (ent = the_repository->objects.alt_odb_list; ent; ent = ent->next) {
> r = fn(ent, cb);
> if (r)
> break;
> @@ -673,12 +671,13 @@ void prepare_alt_odb(void)
> {
> const char *alt;
>
> - if (alt_odb_tail)
> + if (the_repository->objects.alt_odb_tail)
> return;
>
> alt = getenv(ALTERNATE_DB_ENVIRONMENT);
>
> - alt_odb_tail = &alt_odb_list;
> + the_repository->objects.alt_odb_tail =
> + &the_repository->objects.alt_odb_list;
> link_alt_odb_entries(alt, PATH_SEP, NULL, 0);
>
> read_info_alternates(get_object_directory(), 0);
> @@ -717,7 +716,7 @@ static int check_and_freshen_nonlocal(const unsigned char *sha1, int freshen)
> {
> struct alternate_object_database *alt;
> prepare_alt_odb();
> - for (alt = alt_odb_list; alt; alt = alt->next) {
> + for (alt = the_repository->objects.alt_odb_list; alt; alt = alt->next) {
> const char *path = alt_sha1_path(alt, sha1);
> if (check_and_freshen_file(path, freshen))
> return 1;
> @@ -873,7 +872,7 @@ static int stat_sha1_file(const unsigned char *sha1, struct stat *st,
>
> prepare_alt_odb();
> errno = ENOENT;
> - for (alt = alt_odb_list; alt; alt = alt->next) {
> + for (alt = the_repository->objects.alt_odb_list; alt; alt = alt->next) {
> *path = alt_sha1_path(alt, sha1);
> if (!lstat(*path, st))
> return 0;
> @@ -899,7 +898,7 @@ static int open_sha1_file(const unsigned char *sha1, const char **path)
> most_interesting_errno = errno;
>
> prepare_alt_odb();
> - for (alt = alt_odb_list; alt; alt = alt->next) {
> + for (alt = the_repository->objects.alt_odb_list; alt; alt = alt->next) {
> *path = alt_sha1_path(alt, sha1);
> fd = git_open(*path);
> if (fd >= 0)
> diff --git a/sha1_name.c b/sha1_name.c
> index 611c7d24dd..957ce25680 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -10,6 +10,7 @@
> #include "dir.h"
> #include "sha1-array.h"
> #include "packfile.h"
> +#include "repository.h"
>
> static int get_oid_oneline(const char *, struct object_id *, struct commit_list *);
>
> @@ -104,7 +105,7 @@ static void find_short_object_filename(struct disambiguate_state *ds)
> */
> fakeent = alloc_alt_odb(get_object_directory());
> }
> - fakeent->next = alt_odb_list;
> + fakeent->next = the_repository->objects.alt_odb_list;
>
> for (alt = fakeent; alt && !ds->ambiguous; alt = alt->next) {
> int pos;
> --
> 2.16.1.73.ga2c3e9663f.dirty
>
--
Brandon Williams
