Re: [PATCH v4 1/4] Avoid Coverity warning about unfree()d git_exec_path()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Stefan,

On Tue, 3 Jan 2017, Stefan Beller wrote:

> On Mon, Jan 2, 2017 at 8:22 AM, Johannes Schindelin
> <johannes.schindelin@xxxxxx> 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.

This patch was originally only to appease Coverity, but it actually *does*
plug a very real memory leak: previously, *every* call to git_exec_path()
*possibly* returned a newly-malloc()ed buffer. Now, the first call will
store that pointer in a static variable and reuse it later.

Could you maybe help me with improving the commit message?

Ciao,
Dscho



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]