On Fri, Sep 30, 2016 at 09:32:04PM +0000, David Turner wrote: > > +static void env_append(struct argv_array *env, const char *key, const > > +char *val) { > > + const char *old = getenv(key); > > + > > + if (!old) > > + argv_array_pushf(env, "%s=%s", key, val); > > + else > > + argv_array_pushf(env, "%s=%s%c%s", key, old, PATH_SEP, > > val); > >+} > > I would like a comment explaining this function. I'll squash in: diff --git a/tmp-objdir.c b/tmp-objdir.c index c92e6cc..a98c246 100644 --- a/tmp-objdir.c +++ b/tmp-objdir.c @@ -70,6 +70,13 @@ static void remove_tmp_objdir_on_signal(int signo) raise(signo); } +/* + * These env_* functions are for setting up the child environment; the + * "replace" variant overrides the value of any existing variable with that + * "key". The "append" variant puts our new value at the end of a list, + * separated by PATH_SEP (which is what separate values in + * GIT_ALTERNATE_OBJECT_DIRECTORIES). + */ static void env_append(struct argv_array *env, const char *key, const char *val) { const char *old = getenv(key); > > + * Finalize a temporary object directory by migrating its objects into > > +the main > > + * object database. > > + */ > > This should mention that it frees its argument. And: diff --git a/tmp-objdir.h b/tmp-objdir.h index aa47aa9..b1e45b4 100644 --- a/tmp-objdir.h +++ b/tmp-objdir.h @@ -35,7 +35,8 @@ const char **tmp_objdir_env(const struct tmp_objdir *); /* * Finalize a temporary object directory by migrating its objects into the main - * object database. + * object database, removing the temporary directory, and freeing any + * associated resources. */ int tmp_objdir_migrate(struct tmp_objdir *); -Peff