Re: [PATCH] merge-recursive: check GIT_MERGE_VERBOSITY only once

On Wed, Oct 25, 2017 at 12:07:12AM -0400, Eric Sunshine wrote:

On Tue, Oct 24, 2017 at 9:48 PM, Junio C Hamano [off-list ref] wrote:

quoted

Jeff King [off-list ref] writes:

quoted

I definitely agree with the sentiment that as few things as possible
should happen between calling getenv() and using its result. I've seen
real bugs there from unexpected invalidation of the static buffer.

Sure.  I do not think we mind xstrdup()ing the result and freeing
after we are done, though ;-)

More specifically, xstrdup_or_null(getenv(...)), if that route is chosen.

That would be the way to do it, but I do not see thta we need to record
the string at all. The current code is calling strtol on it on it
immediately.

So the options are:

  1. Save the result of getenv() in a variable. If it is non-NULL, then
     immediately call strtol() on it.

  2. Do nothing. The double-call to getenv() is actually fine in the
     real world as it will return consistent results.

But the patch under discussion, which calls getenv() then expects it
to be correct after a call to merge_recursive_config(), introduces a
problem.

-Peff