Re: [PATCH 00/26] Address a couple of issues identified by Coverity

On Wed, Apr 26, 2017 at 1:19 PM, Johannes Schindelin
[off-list ref] wrote:

I recently registered the git-for-windows fork with Coverity to ensure
that even the Windows-specific patches get some static analysis love.

YAY! How do you trigger the coverity scan? (via travis or uploading a tarball
manually every once in a while?) Can you talk about the setup? I'm interested
in that as I run coverity for Junios pu branch, plus a couple patches on top[1].

[1] https://github.com/stefanbeller/git/commits/coverity

While at it, I squashed a couple of obvious issues in the part that is
not Windows-specific.

Thanks.

Note: while this patch series squashes some of those issues, the
remaining issues are not necessarily all false positives (e.g. Coverity
getting fooled by our FLEX_ARRAY trick into believing that the last
member of the struct is indeed a 0 or 1 size array) or intentional (e.g.
builtins not releasing memory because exit() is called right after
returning from the function that leaks memory).

For the latter I think we could get away with maintaining patches on
a dedicated coverity branch (which I do currently, just not in an extensive
manner). For the first, I think we can just compile with FLEX_ARRAY
defined as an insanely large number. I'll experiment with that.

Notable examples of the remaining issues are:

- a couple of callers of shorten_unambiguous_ref() assume that they do
  not have to release the memory returned from that function, often
  assigning the pointer to a `const` variable that typically does not
  hold a pointer that needs to be free()d. My hunch is that we will want
  to convert that function to have a fixed number of static buffers and
  use those in a round robin fashion à la sha1_to_hex().

- pack-redundant.c seems to have hard-to-reason-about code paths that
  may eventually leak memory. Essentially, the custody of the allocated
  memory is not clear at all.

- fast-import.c calls strbuf_detach() on the command_buf without any
  obvious rationale. Turns out that *some* code paths assign
  command_buf.buf to a `recent_command` which releases the buffer later.
  However, from a cursory look it appears to me as if there are some
  other code paths that skip that assignment, effectively causing a memory
  leak once strbuf_detach() is called.

Sadly, I lack the time to pursue those remaining issues further.

Thanks,
Stefan