Re: [PATCH v2 06/10] setup_git_env: use git_pathdup instead of xmalloc + sprintf

From: Jeff King <peff@peff.net>
Date: Jun 25, 2014, 17:22 UTC
Message-ID: <20140625172237.GA15294@sigill.intra.peff.net>
In-Reply-To: <xmqqmwd0aotu.fsf@gitster.dls.corp.google.com>

On Wed, Jun 25, 2014 at 10:20:13AM -0700, Junio C Hamano wrote:

Show 18 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > Here's a replacement patch that handles this (and just drops the ugly
> > mallocs as a side effect).
> >
> > -- >8 --
> > Subject: [PATCH] setup_git_env: copy getenv return value
> >
> > The return value of getenv is not guaranteed to survive
> > across further invocations of setenv or even getenv. When we
> > are assigning it to globals that last the lifetime of the
> > program, we should make our own copy.
> >
> > Signed-off-by: Jeff King <peff@peff.net>
> > ---
> 
> Sigh. This mail unfortunately crossed with 64f25581 (Merge branch 'jk/xstrfmt'
> into next, 2014-06-23) with about 20 hours of lag.

Ah, sorry. I had checked yesterday that jk/xstrfmt hadn't been merged yet, but I didn't check when responding to Duy.

> I'd make it relative like the attached on top of the series.  Note
> that I tweaked the args to git_pathdup() to avoid the "are you sure
> you want to give a variable format string to git_pathdup() which you
> said is like printf(3)?" warning from the compiler.

Both changes look good to me. Thanks for taking care of it.

-Peff