On Wed, Apr 26, 2017 at 1:19 PM, Johannes Schindelin <johannes.schindelin@xxxxxx> 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