On 09/27, Junio C Hamano wrote:
> Brandon Williams <bmwill@xxxxxxxxxx> 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