Thanks. The rest all look good too. > -----Original Message----- > From: Jeff King [mailto:peff@xxxxxxxx] > Sent: Friday, September 30, 2016 6:45 PM > To: David Turner > Cc: git@xxxxxxxxxxxxxxx > Subject: Re: [PATCH 3/6] tmp-objdir: introduce API for temporary object > directories > > 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