Re: [PATCH v2 0/4] Delete ignore_env member in struct repository
From: Eric Sunshine <hidden>
Date: 2018-02-27 10:10:14
On Tue, Feb 27, 2018 at 4:58 AM, Nguyễn Thái Ngọc Duy [off-list ref] wrote:
quoted hunk ↗ jump to hunk
v2 fixes the incorrect use of consecutive getenv() and adds a comment to clarify the role of old_gitdirdiff --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@aiede.svl.corp.google.com/
quoted hunk ↗ jump to hunk
- 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);