I recently registered the git-for-windows fork with Coverity to ensure that even the Windows-specific patches get some static analysis love. While at it, I squashed a couple of obvious issues in the part that is not Windows-specific. 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). 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. Changes since v3: - used 0 (black) for the foreground color attributes in winansi when we have no console to print to, anyway. - clarified in the commit message when we hit the path, and why, where we set the foreground color attributes to "all black". - reworded the commit message talking about splitting the PATH (on Windows, it is delimited by semicolons, not colons, but it is even better to just talk about the path delimiters because it does not really happen which character is used, but it is important which role it plays). - rewrote the split_commit_in_progress() function to have a more natural flow while still fixing the memory leak. Johannes Schindelin (25): mingw: avoid memory leak when splitting PATH winansi: avoid use of uninitialized value winansi: avoid buffer overrun add_commit_patch_id(): avoid allocating memory unnecessarily git_config_rename_section_in_file(): avoid resource leak get_mail_commit_oid(): avoid resource leak difftool: address a couple of resource/memory leaks status: close file descriptor after reading git-rebase-todo mailinfo & mailsplit: check for EOF while parsing cat-file: fix memory leak checkout: fix memory leak split_commit_in_progress(): simplify & fix memory leak setup_bare_git_dir(): help static analysis setup_discovered_git_dir(): plug memory leak pack-redundant: plug memory leak mktree: plug memory leaks reported by Coverity fast-export: avoid leaking memory in handle_tag() receive-pack: plug memory leak in update() line-log: avoid memory leak shallow: avoid memory leak add_reflog_for_walk: avoid memory leak remote: plug memory leak in match_explicit() name-rev: avoid leaking memory in the `deref` case show_worktree(): plug memory leak submodule_uses_worktrees(): plug memory leak builtin/am.c | 15 ++++++--------- builtin/cat-file.c | 1 + builtin/checkout.c | 17 +++++++++-------- builtin/difftool.c | 33 +++++++++++++++++++++++---------- builtin/fast-export.c | 2 ++ builtin/mailsplit.c | 10 ++++++++++ builtin/mktree.c | 5 +++-- builtin/name-rev.c | 7 +++++-- builtin/pack-redundant.c | 1 + builtin/receive-pack.c | 4 +++- builtin/worktree.c | 8 +++++--- compat/mingw.c | 4 +++- compat/winansi.c | 12 ++++++++++++ config.c | 5 ++++- line-log.c | 1 + mailinfo.c | 9 ++++++++- patch-ids.c | 3 ++- reflog-walk.c | 20 +++++++++++++++++--- remote.c | 5 +++-- setup.c | 11 ++++++++--- shallow.c | 8 ++++++-- worktree.c | 2 +- wt-status.c | 29 +++++++++++++++-------------- 23 files changed, 148 insertions(+), 64 deletions(-) base-commit: d2bbb7c2bcf6e77ebfcabf4e12110fe6d5c91de6 Published-As: https://github.com/dscho/git/releases/tag/coverity-v4 Fetch-It-Via: git fetch https://github.com/dscho/git coverity-v4 Interdiff vs v3: diff --git a/compat/winansi.c b/compat/winansi.c index 861b79d8c31..a11a0f16d27 100644 --- a/compat/winansi.c +++ b/compat/winansi.c @@ -105,8 +105,13 @@ static int is_console(int fd) if (!fd) { if (!GetConsoleMode(hcon, &mode)) return 0; - sbi.wAttributes = FOREGROUND_BLUE | FOREGROUND_GREEN | - FOREGROUND_RED; + /* + * This code path is only reached if there is no console + * attached to stdout/stderr, i.e. we will not need to output + * any text to any console, therefore we might just as well + * use black as foreground color. + */ + sbi.wAttributes = 0; } else if (!GetConsoleScreenBufferInfo(hcon, &sbi)) return 0; diff --git a/wt-status.c b/wt-status.c index 1f3f6bcb980..117ac8cfb01 100644 --- a/wt-status.c +++ b/wt-status.c @@ -1082,34 +1082,29 @@ static char *read_line_from_git_path(const char *filename) static int split_commit_in_progress(struct wt_status *s) { int split_in_progress = 0; - char *head = read_line_from_git_path("HEAD"); - char *orig_head = read_line_from_git_path("ORIG_HEAD"); - char *rebase_amend = read_line_from_git_path("rebase-merge/amend"); - char *rebase_orig_head = read_line_from_git_path("rebase-merge/orig-head"); - - if (!head || !orig_head || !rebase_amend || !rebase_orig_head || - !s->branch || strcmp(s->branch, "HEAD")) { - free(head); - free(orig_head); - free(rebase_amend); - free(rebase_orig_head); - return split_in_progress; - } - - if (!strcmp(rebase_amend, rebase_orig_head)) { - if (strcmp(head, rebase_amend)) - split_in_progress = 1; - } else if (strcmp(orig_head, rebase_orig_head)) { - split_in_progress = 1; - } + char *head, *orig_head, *rebase_amend, *rebase_orig_head; + + if ((!s->amend && !s->nowarn && !s->workdir_dirty) || + !s->branch || strcmp(s->branch, "HEAD")) + return 0; - if (!s->amend && !s->nowarn && !s->workdir_dirty) - split_in_progress = 0; + head = read_line_from_git_path("HEAD"); + orig_head = read_line_from_git_path("ORIG_HEAD"); + rebase_amend = read_line_from_git_path("rebase-merge/amend"); + rebase_orig_head = read_line_from_git_path("rebase-merge/orig-head"); + + if (!head || !orig_head || !rebase_amend || !rebase_orig_head) + ; /* fall through, no split in progress */ + else if (!strcmp(rebase_amend, rebase_orig_head)) + split_in_progress = !!strcmp(head, rebase_amend); + else if (strcmp(orig_head, rebase_orig_head)) + split_in_progress = 1; free(head); free(orig_head); free(rebase_amend); free(rebase_orig_head); + return split_in_progress; } -- 2.12.2.windows.2.800.gede8f145e06