Re: [PATCH 2/4 v4] ls-files: optionally recurse into submodules

[Brandon Williams <bmwill@google.com>]

On 09/27, Junio C Hamano wrote:
> Brandon Williams <bmwill@google.com> writes:
> > +static const char *submodule_prefix;
> 
> I would have expected this to added to environment.c in the previous
> step, but it is OK--I'd imagine you'd grab this from the environment
> and carrying a piece of information from git.c to here by setenv()
> followed by getenv() feels somewhat roundabout, though.

If it would make sense to do the caching of prefix string in
environment.c I can move it there and add a get_submodule_prefix()
function which either reads the envvar or the cached value if its
already been read.  Would that be a better route?

> 
> >  static const char *prefix;
> >  static int max_prefix_len;
> > @@ -68,6 +71,21 @@ static void write_eolinfo(const struct cache_entry *ce, const char *path)
> >  static void write_name(const char *name)
> >  {
> >  	/*
> > +	 * NEEDSWORK: To make this thread-safe, full_name would have to be owned
> > +	 * by the caller.
> 
> As Peff mentioned in his review in another thread, a large number of
> functions in git are not reentrant, and I do not think we would want
> to give the impression that those missing a warning are safe to use.
> 
> Other than that, this step looks OK.  3/4 and later would be a lot
> more fun to review ;-)

Oh yes, I can remove the comment.  Seemed to miss that bit while
rerolling the series.

-- 
Brandon Williams