On Tue, Feb 27, 2018 at 4:58 AM, Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> wrote:
> v2 fixes the incorrect use of consecutive getenv() and adds a comment
> to clarify the role of old_gitdir
>
> diff --git a/environment.c b/environment.c
> @@ -148,18 +149,34 @@ static char *expand_namespace(const char *raw_namespace)
> +/* Wrapper of getenv() that returns a strdup value. This value is kept
> + * in argv to be freed later.
> + */
/*
* Comment style.
*/
> +static const char *getenv_safe(struct argv_array *argv, const char *name)
> +{
> + const char *value = getenv(name);
> +
> + if (!value)
> + return NULL;
> +
> + argv_array_push(argv, value);
> + return argv->argv[argv->argc - 1];
> +}
> +
> void setup_git_env(const char *git_dir)
> {
> const char *shallow_file;
> const char *replace_ref_base;
> struct set_gitdir_args args = { NULL };
> + struct argv_array to_free = ARGV_ARRAY_INIT;
Cute. Another example[1] showing that renaming argv_array to something
else might be a good idea.
[1]: https://public-inbox.org/git/20180227062128.GG65699@xxxxxxxxxxxxxxxxxxxxxxxxx/
> - args.shared_root = getenv(GIT_COMMON_DIR_ENVIRONMENT);
> - args.object_dir = getenv(DB_ENVIRONMENT);
> - args.graft_file = getenv(GRAFT_ENVIRONMENT);
> - args.index_file = getenv(INDEX_ENVIRONMENT);
> - args.alternate_db = getenv(ALTERNATE_DB_ENVIRONMENT);
> + args.shared_root = getenv_safe(&to_free, GIT_COMMON_DIR_ENVIRONMENT);
> + args.object_dir = getenv_safe(&to_free, DB_ENVIRONMENT);
> + args.graft_file = getenv_safe(&to_free, GRAFT_ENVIRONMENT);
> + args.index_file = getenv_safe(&to_free, INDEX_ENVIRONMENT);
> + args.alternate_db = getenv_safe(&to_free, ALTERNATE_DB_ENVIRONMENT);
> repo_set_gitdir(the_repository, git_dir, &args);
> + argv_array_clear(&to_free);
> diff --git a/repository.c b/repository.c
> @@ -48,6 +48,11 @@ void repo_set_gitdir(struct repository *repo,
> const char *gitfile = read_gitfile(root);
> + /*
> + * repo->gitdir is saved because the caller could pass "root"
> + * that also points to repo->gitdir. We want to keep it alive
> + * until after xstrdup(root). Then we can free it.
> + */
> char *old_gitdir = repo->gitdir;
Good. This comment makes the reasoning clear.
> repo->gitdir = xstrdup(gitfile ? gitfile : root);
