From 165da77960d7a32cd8b86e5932fcc6769f8498cc Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 19 Apr 2017 16:16:44 +0200 Subject: [PATCH 01/27] mingw: avoid memory leak when splitting PATH In the (admittedly, concocted) case that PATH consists only of colons, we would leak the duplicated string. Signed-off-by: Johannes Schindelin --- compat/mingw.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/compat/mingw.c b/compat/mingw.c index 7840377c65..4cbae6b621 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -1164,8 +1164,10 @@ static char **get_path_split(void) ++n; } } - if (!n) + if (!n) { + free(envpath); return NULL; + } ALLOC_ARRAY(path, n + 1); p = envpath; From bdc094ae202262e0f683efdc54eb10d2b185d6df Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 17 Apr 2017 12:17:17 +0200 Subject: [PATCH 02/27] winansi: avoid use of uninitialized value When stdout is not connected to a Win32 console, we incorrectly used an uninitialized value for the "plain" character attributes. Detected by Coverity. Signed-off-by: Johannes Schindelin --- compat/winansi.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/compat/winansi.c b/compat/winansi.c index 7e3534e9a1..8c87b0f552 100644 --- a/compat/winansi.c +++ b/compat/winansi.c @@ -100,6 +100,8 @@ static int is_console(int fd) if (!fd) { if (!GetConsoleMode(hcon, &mode)) return 0; + sbi.wAttributes = FOREGROUND_BLUE | FOREGROUND_GREEN | + FOREGROUND_RED; } else if (!GetConsoleScreenBufferInfo(hcon, &sbi)) return 0; From 475e5fe43e3bab0fad34cd4c7fa9c4a85830131e Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 26 Apr 2017 10:38:19 +0200 Subject: [PATCH 03/27] winansi: avoid buffer overrun When we could not convert the UTF-8 sequence into Unicode for writing to the Console, we should not try to write an insanely-long sequence of invalid wide characters (mistaking the negative return value for an unsigned length). Reported by Coverity. Signed-off-by: Johannes Schindelin --- compat/winansi.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/compat/winansi.c b/compat/winansi.c index 8c87b0f552..d348da4f6c 100644 --- a/compat/winansi.c +++ b/compat/winansi.c @@ -130,6 +130,11 @@ static void write_console(unsigned char *str, size_t len) /* convert utf-8 to utf-16 */ int wlen = xutftowcsn(wbuf, (char*) str, ARRAY_SIZE(wbuf), len); + if (wlen < 0) { + wchar_t *err = L"[invalid]"; + WriteConsoleW(console, err, wcslen(err), &dummy, NULL); + return; + } /* write directly to console */ WriteConsoleW(console, wbuf, wlen, &dummy, NULL); From 84870536b12cd43577334df42bf03a01bb1e9d96 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 17 Apr 2017 13:04:57 +0200 Subject: [PATCH 04/27] add_commit_patch_id(): avoid allocating memory unnecessarily It would appear that we allocate (and forget to release) memory if the patch ID is not even defined. Reported by the Coverity tool. Signed-off-by: Johannes Schindelin --- patch-ids.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/patch-ids.c b/patch-ids.c index ce285c2e0c..660177875a 100644 --- a/patch-ids.c +++ b/patch-ids.c @@ -99,11 +99,12 @@ struct patch_id *has_commit_patch_id(struct commit *commit, struct patch_id *add_commit_patch_id(struct commit *commit, struct patch_ids *ids) { - struct patch_id *key = xcalloc(1, sizeof(*key)); + struct patch_id *key; if (!patch_id_defined(commit)) return NULL; + key = xcalloc(1, sizeof(*key)); if (init_patch_id_entry(key, commit, ids)) { free(key); return NULL; From 775c6ffd87f88b1e7ec1fc7e5fc2eb19b4e5d047 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 17 Apr 2017 13:18:32 +0200 Subject: [PATCH 05/27] git_config_rename_section_in_file(): avoid resource leak In case of errors, we really want the file descriptor to be closed. Discovered by a Coverity scan. Signed-off-by: Johannes Schindelin --- config.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/config.c b/config.c index 09d5efeaa6..a77896fa61 100644 --- a/config.c +++ b/config.c @@ -2426,7 +2426,7 @@ int git_config_rename_section_in_file(const char *config_filename, struct lock_file *lock; int out_fd; char buf[1024]; - FILE *config_file; + FILE *config_file = NULL; struct stat st; if (new_name && !section_name_is_ok(new_name)) { @@ -2508,11 +2508,14 @@ int git_config_rename_section_in_file(const char *config_filename, } } fclose(config_file); + config_file = NULL; commit_and_out: if (commit_lock_file(lock) < 0) ret = error_errno("could not write config file %s", config_filename); out: + if (config_file) + fclose(config_file); rollback_lock_file(lock); out_no_rollback: free(filename_buf); From edb88ae6581b5e43b304a1329d46b61690784bdf Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 17 Apr 2017 13:42:44 +0200 Subject: [PATCH 06/27] get_mail_commit_oid(): avoid resource leak When we fail to read, or parse, the file, we still want to close the file descriptor and release the strbuf. Reported via Coverity. Signed-off-by: Johannes Schindelin --- builtin/am.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index 31fb60578f..a9c6d7088e 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -1355,15 +1355,16 @@ static int get_mail_commit_oid(struct object_id *commit_id, const char *mail) struct strbuf sb = STRBUF_INIT; FILE *fp = xfopen(mail, "r"); const char *x; + int ret = 0; if (strbuf_getline_lf(&sb, fp)) - return -1; + ret = -1; - if (!skip_prefix(sb.buf, "From ", &x)) - return -1; + if (!ret && !skip_prefix(sb.buf, "From ", &x)) + ret = -1; - if (get_oid_hex(x, commit_id) < 0) - return -1; + if (!ret && get_oid_hex(x, commit_id) < 0) + ret = -1; strbuf_release(&sb); fclose(fp); From a7892d835d197c7883971b3ff07aeab4dfe96ce5 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 17 Apr 2017 13:47:28 +0200 Subject: [PATCH 07/27] http-backend: avoid memory leaks Reported via Coverity. Signed-off-by: Johannes Schindelin --- http-backend.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/http-backend.c b/http-backend.c index eef0a361f4..d12572fda1 100644 --- a/http-backend.c +++ b/http-backend.c @@ -681,8 +681,10 @@ int cmd_main(int argc, const char **argv) if (!regexec(&re, dir, 1, out, 0)) { size_t n; - if (strcmp(method, c->method)) + if (strcmp(method, c->method)) { + free(dir); return bad_request(&hdr, c); + } cmd = c; n = out[0].rm_eo - out[0].rm_so; @@ -708,5 +710,7 @@ int cmd_main(int argc, const char **argv) max_request_buffer); cmd->imp(&hdr, cmd_arg); + free(dir); + free(cmd_arg); return 0; } From 56dba3d40aeb321468ea2f0c50cd58ecce025181 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 17 Apr 2017 22:39:04 +0200 Subject: [PATCH 08/27] difftool: close file descriptors after reading Spotted by Coverity. Signed-off-by: Johannes Schindelin --- builtin/difftool.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/builtin/difftool.c b/builtin/difftool.c index 1354d0e462..a4f1d117ef 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -226,6 +226,7 @@ static void changed_files(struct hashmap *result, const char *index_path, hashmap_entry_init(entry, strhash(buf.buf)); hashmap_add(result, entry); } + fclose(fp); if (finish_command(&diff_files)) die("diff-files did not exit properly"); strbuf_release(&index_env); @@ -497,6 +498,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix, } } + fclose(fp); if (finish_command(&child)) { ret = error("error occurred running diff --raw"); goto finish; From e2430a0f8a24362bdc4c94a79b1662353d39a7d7 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 17 Apr 2017 22:40:30 +0200 Subject: [PATCH 09/27] status: close file descriptor after reading git-rebase-todo Reported via Coverity. Signed-off-by: Johannes Schindelin --- wt-status.c | 1 + 1 file changed, 1 insertion(+) diff --git a/wt-status.c b/wt-status.c index 4bb46781c8..ff1ae80065 100644 --- a/wt-status.c +++ b/wt-status.c @@ -1155,6 +1155,7 @@ static int read_rebase_todolist(const char *fname, struct string_list *lines) abbrev_sha1_in_line(&line); string_list_append(lines, line.buf); } + fclose(f); return 0; } From 9a63766e6bda62a63d250e765fcbcf47e64b28ba Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 17 Apr 2017 22:51:58 +0200 Subject: [PATCH 10/27] Check for EOF while parsing mails Reported via Coverity. Signed-off-by: Johannes Schindelin --- builtin/mailsplit.c | 2 +- mailinfo.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/builtin/mailsplit.c b/builtin/mailsplit.c index 30681681c1..c0d88f9751 100644 --- a/builtin/mailsplit.c +++ b/builtin/mailsplit.c @@ -232,7 +232,7 @@ static int split_mbox(const char *file, const char *dir, int allow_bare, do { peek = fgetc(f); - } while (isspace(peek)); + } while (peek >= 0 && isspace(peek)); ungetc(peek, f); if (strbuf_getwholeline(&buf, f, '\n')) { diff --git a/mailinfo.c b/mailinfo.c index a489d9d0fb..659136cc58 100644 --- a/mailinfo.c +++ b/mailinfo.c @@ -1094,7 +1094,7 @@ int mailinfo(struct mailinfo *mi, const char *msg, const char *patch) do { peek = fgetc(mi->input); - } while (isspace(peek)); + } while (peek >= 0 && isspace(peek)); ungetc(peek, mi->input); /* process the email header */ From 1877204af57ae37a8f978474f2e25d2038a4800d Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 18 Apr 2017 13:57:58 +0200 Subject: [PATCH 11/27] cat-file: fix memory leak Discovered by Coverity. Signed-off-by: Johannes Schindelin --- builtin/cat-file.c | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index 30383e9eb4..1dcfde3b7c 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -165,6 +165,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name, die("git cat-file %s: bad file", obj_name); write_or_die(1, buf, size); + free(buf); return 0; } From 6eb855984775329078dc30c83996b5d306bbbe67 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 18 Apr 2017 14:02:51 +0200 Subject: [PATCH 12/27] checkout: fix memory leak Discovered via Coverity. Signed-off-by: Johannes Schindelin --- builtin/checkout.c | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/checkout.c b/builtin/checkout.c index 81f07c3ef2..fd273cdf2a 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -232,6 +232,7 @@ static int checkout_merged(int pos, const struct checkout *state) if (!ce) die(_("make_cache_entry failed for path '%s'"), path); status = checkout_entry(ce, state, NULL); + free(ce); return status; } From 0c405aa9910e9cd29b36a0b506a20ceb0f197eb1 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 18 Apr 2017 14:04:52 +0200 Subject: [PATCH 13/27] split_commit_in_progress(): fix memory leak Reported via Coverity. Signed-off-by: Johannes Schindelin --- wt-status.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/wt-status.c b/wt-status.c index ff1ae80065..3484deaeee 100644 --- a/wt-status.c +++ b/wt-status.c @@ -1075,8 +1075,13 @@ static int split_commit_in_progress(struct wt_status *s) 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")) + !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)) From a26de53ceeffe8b4cc95b74198aa917f5af014f4 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 18 Apr 2017 14:09:23 +0200 Subject: [PATCH 14/27] setup_bare_git_dir(): fix memory leak Reported by Coverity. Signed-off-by: Johannes Schindelin --- setup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.c b/setup.c index 72a5482c4e..dcee4c0f25 100644 --- a/setup.c +++ b/setup.c @@ -740,7 +740,7 @@ static const char *setup_bare_git_dir(struct strbuf *cwd, int offset, /* --work-tree is set without --git-dir; use discovered one */ if (getenv(GIT_WORK_TREE_ENVIRONMENT) || git_work_tree_cfg) { - const char *gitdir; + static const char *gitdir; gitdir = offset == cwd->len ? "." : xmemdupz(cwd->buf, offset); if (chdir(cwd->buf)) From 1f219890fb36fbeca210f32a3627a73e30153233 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 18 Apr 2017 14:11:24 +0200 Subject: [PATCH 15/27] setup_discovered_git_dir(): fix memory leak Identified by Coverity. Signed-off-by: Johannes Schindelin --- setup.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/setup.c b/setup.c index dcee4c0f25..8e77e42a5d 100644 --- a/setup.c +++ b/setup.c @@ -697,11 +697,16 @@ static const char *setup_discovered_git_dir(const char *gitdir, /* --work-tree is set without --git-dir; use discovered one */ if (getenv(GIT_WORK_TREE_ENVIRONMENT) || git_work_tree_cfg) { + char *p = NULL; + const char *ret; + if (offset != cwd->len && !is_absolute_path(gitdir)) - gitdir = real_pathdup(gitdir, 1); + gitdir = p = real_pathdup(gitdir, 1); if (chdir(cwd->buf)) die_errno("Could not come back to cwd"); - return setup_explicit_git_dir(gitdir, cwd, nongit_ok); + ret = setup_explicit_git_dir(gitdir, cwd, nongit_ok); + free(p); + return ret; } /* #16.2, #17.2, #20.2, #21.2, #24, #25, #28, #29 (see t1510) */ From 8ee6b9fd03cc8d7baa1d26ff334ef10e7c237dfb Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 19 Apr 2017 16:20:35 +0200 Subject: [PATCH 16/27] pack-redundant: plug memory leak Identified via Coverity. Signed-off-by: Johannes Schindelin --- builtin/pack-redundant.c | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c index 72c815844d..cb1df1c761 100644 --- a/builtin/pack-redundant.c +++ b/builtin/pack-redundant.c @@ -442,6 +442,7 @@ static void minimize(struct pack_list **min) /* return if there are no objects missing from the unique set */ if (missing->size == 0) { *min = unique; + free(missing); return; } From cfb2b4c0d682b35be9a7d6c75e6eeb508382afa8 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 19 Apr 2017 16:31:10 +0200 Subject: [PATCH 17/27] mktree: plug memory leaks reported by Coverity Signed-off-by: Johannes Schindelin --- builtin/mktree.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/builtin/mktree.c b/builtin/mktree.c index de9b40fc63..f0354bc971 100644 --- a/builtin/mktree.c +++ b/builtin/mktree.c @@ -72,7 +72,7 @@ static void mktree_line(char *buf, size_t len, int nul_term_line, int allow_miss unsigned mode; enum object_type mode_type; /* object type derived from mode */ enum object_type obj_type; /* object type derived from sha */ - char *path; + char *path, *p = NULL; unsigned char sha1[20]; ptr = buf; @@ -102,7 +102,7 @@ static void mktree_line(char *buf, size_t len, int nul_term_line, int allow_miss struct strbuf p_uq = STRBUF_INIT; if (unquote_c_style(&p_uq, path, NULL)) die("invalid quoting"); - path = strbuf_detach(&p_uq, NULL); + path = p = strbuf_detach(&p_uq, NULL); } /* @@ -136,6 +136,7 @@ static void mktree_line(char *buf, size_t len, int nul_term_line, int allow_miss } append_to_tree(mode, sha1, path); + free(p); } int cmd_mktree(int ac, const char **av, const char *prefix) From 1d24c82c57b478c1fdb7ff59009264119bd87b26 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 19 Apr 2017 16:45:03 +0200 Subject: [PATCH 18/27] fast-export: avoid leaking memory in handle_tag() Reported by, you guessed it, Coverity. Signed-off-by: Johannes Schindelin --- builtin/fast-export.c | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/fast-export.c b/builtin/fast-export.c index bfa10f5124..933042f940 100644 --- a/builtin/fast-export.c +++ b/builtin/fast-export.c @@ -765,6 +765,7 @@ static void handle_tag(const char *name, struct tag *tag) (int)(tagger_end - tagger), tagger, tagger == tagger_end ? "" : "\n", (int)message_size, (int)message_size, message ? message : ""); + free(buf); } static struct commit *get_commit(struct rev_cmdline_entry *e, char *full_name) From db9ce700d4b63f6c28aa3a273b0d3e0067832880 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 19 Apr 2017 17:04:22 +0200 Subject: [PATCH 19/27] receive-pack: plug memory leak in update() Reported via Coverity. Signed-off-by: Johannes Schindelin --- builtin/receive-pack.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index b618d52139..b184886345 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -984,7 +984,8 @@ static const char *update(struct command *cmd, struct shallow_info *si) { const char *name = cmd->ref_name; struct strbuf namespaced_name_buf = STRBUF_INIT; - const char *namespaced_name, *ret; + static char *namespaced_name; + const char *ret; unsigned char *old_sha1 = cmd->old_sha1; unsigned char *new_sha1 = cmd->new_sha1; @@ -995,6 +996,7 @@ static const char *update(struct command *cmd, struct shallow_info *si) } strbuf_addf(&namespaced_name_buf, "%s%s", get_git_namespace(), name); + free(namespaced_name); namespaced_name = strbuf_detach(&namespaced_name_buf, NULL); if (is_ref_checked_out(namespaced_name)) { From 8ab022a0a78a4f67d88710f163db7b2a7906ae62 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 19 Apr 2017 17:16:34 +0200 Subject: [PATCH 20/27] line-log: avoid memory leak Discovered by Coverity. Signed-off-by: Johannes Schindelin --- line-log.c | 1 + 1 file changed, 1 insertion(+) diff --git a/line-log.c b/line-log.c index a23b910471..19d46e9ea2 100644 --- a/line-log.c +++ b/line-log.c @@ -1125,6 +1125,7 @@ static int process_ranges_ordinary_commit(struct rev_info *rev, struct commit *c changed = process_all_files(&parent_range, rev, &queue, range); if (parent) add_line_range(rev, parent, parent_range); + free(parent_range); return changed; } From 5f81ef646029d97fef5551d0c7e72d46b82cf9a8 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 21 Apr 2017 14:06:22 +0200 Subject: [PATCH 21/27] shallow: avoid memory leak Reported by Coverity. Signed-off-by: Johannes Schindelin --- shallow.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/shallow.c b/shallow.c index 11f7dde9d9..9b9e8128bf 100644 --- a/shallow.c +++ b/shallow.c @@ -473,11 +473,15 @@ static void paint_down(struct paint_info *info, const unsigned char *sha1, struct commit_list *head = NULL; int bitmap_nr = (info->nr_bits + 31) / 32; size_t bitmap_size = st_mult(sizeof(uint32_t), bitmap_nr); - uint32_t *tmp = xmalloc(bitmap_size); /* to be freed before return */ - uint32_t *bitmap = paint_alloc(info); struct commit *c = lookup_commit_reference_gently(sha1, 1); + uint32_t *tmp; /* to be freed before return */ + uint32_t *bitmap; + if (!c) return; + + tmp = xmalloc(bitmap_size); + bitmap = paint_alloc(info); memset(bitmap, 0, bitmap_size); bitmap[id / 32] |= (1U << (id % 32)); commit_list_insert(c, &head); From b495d52e6c8122be806c9f7391a2fd724d554d2b Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 21 Apr 2017 14:15:13 +0200 Subject: [PATCH 22/27] add_reflog_for_walk: avoid memory leak We free()d the `log` buffer when dwim_log() returned 1, but not when it returned a larger value (which meant that it still allocated the buffer but we simply ignored it). Identified by Coverity. Signed-off-by: Johannes Schindelin --- reflog-walk.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/reflog-walk.c b/reflog-walk.c index a246af2767..089892bcc5 100644 --- a/reflog-walk.c +++ b/reflog-walk.c @@ -183,7 +183,10 @@ int add_reflog_for_walk(struct reflog_walk_info *info, if (!reflogs || reflogs->nr == 0) { unsigned char sha1[20]; char *b; - if (dwim_log(branch, strlen(branch), sha1, &b) == 1) { + int ret = dwim_log(branch, strlen(branch), sha1, &b); + if (ret > 1) + free(b); + else if (ret == 1) { if (reflogs) { free(reflogs->ref); free(reflogs); From a3ccecf2a78f24ca7a8627ac474717e818ade7c8 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 21 Apr 2017 14:19:06 +0200 Subject: [PATCH 23/27] remote: plug memory leak in match_explicit() The `guess_ref()` returns an allocated buffer of which `make_linked_ref()` does not take custody (`alloc_ref()` makes a copy), therefore we need to release the buffer afterwards. Noticed via Coverity. Signed-off-by: Johannes Schindelin --- remote.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/remote.c b/remote.c index 9f83fe2c4c..e581de0a05 100644 --- a/remote.c +++ b/remote.c @@ -1191,9 +1191,10 @@ static int match_explicit(struct ref *src, struct ref *dst, else if (is_null_oid(&matched_src->new_oid)) error("unable to delete '%s': remote ref does not exist", dst_value); - else if ((dst_guess = guess_ref(dst_value, matched_src))) + else if ((dst_guess = guess_ref(dst_value, matched_src))) { matched_dst = make_linked_ref(dst_guess, dst_tail); - else + free(dst_guess); + } else error("unable to push to unqualified destination: %s\n" "The destination refspec neither matches an " "existing ref on the remote nor\n" From 5fdcc9bcfed617f96309059899d47d1c62d4b4a3 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 21 Apr 2017 14:22:49 +0200 Subject: [PATCH 24/27] name-rev: avoid leaking memory in the `deref` case When the `name_rev()` function is asked to dereference the tip name, it allocates memory. But when it turns out that another tip already described the commit better than the current one, we forgot to release the memory. Pointed out by Coverity. Signed-off-by: Johannes Schindelin --- builtin/name-rev.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/builtin/name-rev.c b/builtin/name-rev.c index cd89d48b65..09642e46d1 100644 --- a/builtin/name-rev.c +++ b/builtin/name-rev.c @@ -28,6 +28,7 @@ static void name_rev(struct commit *commit, struct rev_name *name = (struct rev_name *)commit->util; struct commit_list *parents; int parent_number = 1; + char *p = NULL; parse_commit(commit); @@ -35,7 +36,7 @@ static void name_rev(struct commit *commit, return; if (deref) { - tip_name = xstrfmt("%s^0", tip_name); + tip_name = p = xstrfmt("%s^0", tip_name); if (generation) die("generation: %d, but deref?", generation); @@ -53,8 +54,10 @@ copy_data: name->taggerdate = taggerdate; name->generation = generation; name->distance = distance; - } else + } else { + free(p); return; + } for (parents = commit->parents; parents; From 5b54a1d4e02ec94c8c1388e3fae1b6e2f8bd6ab4 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 21 Apr 2017 14:34:47 +0200 Subject: [PATCH 25/27] show_worktree(): plug memory leak The buffer allocated by shorten_unambiguous_ref() needs to be released. Discovered by Coverity. Signed-off-by: Johannes Schindelin --- builtin/worktree.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/builtin/worktree.c b/builtin/worktree.c index 831fe058a5..6fd27d5cdd 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -408,9 +408,11 @@ static void show_worktree(struct worktree *wt, int path_maxlen, int abbrev_len) find_unique_abbrev(wt->head_sha1, DEFAULT_ABBREV)); if (wt->is_detached) strbuf_addstr(&sb, "(detached HEAD)"); - else if (wt->head_ref) - strbuf_addf(&sb, "[%s]", shorten_unambiguous_ref(wt->head_ref, 0)); - else + else if (wt->head_ref) { + char *ref = shorten_unambiguous_ref(wt->head_ref, 0); + strbuf_addf(&sb, "[%s]", ref); + free(ref); + } else strbuf_addstr(&sb, "(error)"); } printf("%s\n", sb.buf); From b9f82c37e575f664f57fa9760d710fee7ebe3de7 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 21 Apr 2017 14:40:56 +0200 Subject: [PATCH 26/27] submodule_uses_worktrees(): plug memory leak There is really no reason why we would need to hold onto the allocated string longer than necessary. Reported by Coverity. Signed-off-by: Johannes Schindelin --- worktree.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/worktree.c b/worktree.c index fa7bc67a50..1ffacc5a92 100644 --- a/worktree.c +++ b/worktree.c @@ -396,6 +396,7 @@ int submodule_uses_worktrees(const char *path) /* The env would be set for the superproject. */ get_common_dir_noenv(&sb, submodule_gitdir); + free(submodule_gitdir); /* * The check below is only known to be good for repository format @@ -415,7 +416,6 @@ int submodule_uses_worktrees(const char *path) /* See if there is any file inside the worktrees directory. */ dir = opendir(sb.buf); strbuf_release(&sb); - free(submodule_gitdir); if (!dir) return 0; From 50cb3248407d73b41fd313129c3ff76f2ec5ffa5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sun, 16 Apr 2017 18:55:30 +0200 Subject: [PATCH 27/27] am: close stream on error, but not stdin Avoid closing stdin, but do close an actual input file on error exit. Found with Cppcheck. Signed-off-by: Rene Scharfe Reviewed-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/am.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index a9c6d7088e..be88eac046 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -762,14 +762,18 @@ static int split_mail_conv(mail_conv_fn fn, struct am_state *state, mail = mkpath("%s/%0*d", state->dir, state->prec, i + 1); out = fopen(mail, "w"); - if (!out) + if (!out) { + if (in != stdin) + fclose(in); return error_errno(_("could not open '%s' for writing"), mail); + } ret = fn(out, in, keep_cr); fclose(out); - fclose(in); + if (in != stdin) + fclose(in); if (ret) return error(_("could not parse patch '%s'"), *paths);