Re: [PATCH v6 1/4] git: make super-prefix option

From: Jeff King <peff@peff.net>
Date: Oct 4, 2016, 17:39 UTC
Message-ID: <20161004173904.tu37rqn4rl26gabx@sigill.intra.peff.net>
In-Reply-To: <CAGZ79kax9g-FLMhPnDBP+7wJwYT884B5bGodpopo9GKgnE6+PQ@mail.gmail.com>

On Tue, Oct 04, 2016 at 10:31:51AM -0700, Stefan Beller wrote:

Show 19 quoted lines

> On Thu, Sep 29, 2016 at 2:48 PM, Brandon Williams <bmwill@google.com> wrote:
> 
> >
> > +const char *get_super_prefix(void)
> > +{
> > +       if (!super_prefix)
> > +               super_prefix = getenv(GIT_SUPER_PREFIX_ENVIRONMENT);
> > +       return super_prefix;
> > +}
> > +
> 
> As said earlier, is the following a valid thought:
> 
> > The getenv() function returns a pointer to the value in the
> > environment, or NULL if there is no match.
> > So in case this is not set (when e.g. the user did not specify the
> > super prefix), we would probe it a couple of times.
> > The caching effect only occurs when the string is set. So this looks
> > like we save repetitive calls, but we do not always do that.

I think your concern is valid. If it is not set, we will do an O(n) search through the whole environment on each call.

I also think the result of getenv() needs to be copied. In some implementations it persists for the life of the program, but that's not guaranteed; it may be overwritten by unrelated calls to getenv() or setenv().

-Peff