From: Stefan Beller <sbeller@google.com>
Date: Tue, 03 Jan 2017 20:11:25 GMT
Subject: Re: [PATCH v4 1/4] Avoid Coverity warning about unfree()d git_exec_path()
Message-ID: <CAGZ79kZ--jp08pK+xwn1N2VQQr8bA5+DveE2HsoY90R1gR6c_A@mail.gmail.com>
In-Reply-To: <18e9a1009aac2329cb9bf9d12fbac4e8ac19a5bb.1483373635.git.johannes.schindelin@gmx.de>

On Mon, Jan 2, 2017 at 8:22 AM, Johannes Schindelin
<johannes.schindelin@gmx.de> wrote:
> Technically, it is correct that git_exec_path() returns a possibly
> malloc()ed string. Practically, it is *sometimes* not malloc()ed. So
> let's just use a static variable to make it a singleton. That'll shut
> Coverity up, hopefully.

I picked up this patch and applied it to the coverity branch
that I maintain at github/stefanbeller/git.

I'd love to see this patch upstream as it reduces my maintenance
burden of the coverity branch by a patch.

Early on when Git was new to coverity, some arguments were made
that patches like these only clutter the main code base which is read
by a lot of people, hence we want these quirks for coverity not upstream.
And I think that still holds.

If this patch is only to appease coverity (as the commit message eludes
to) I think this may be a bad idea for upstream.  If this patch fixes an
actual problem, then the commit message needs to spell that out.

Thanks,
Stefan