From 5a09991e32f3487702bd032703bacba1c4c46612 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 15 Jun 2022 23:35:36 +0000 Subject: [PATCH 1/6] fsmonitor: avoid memory leak in `fsm_settings__get_incompatible_msg()` Reported by Coverity. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- fsmonitor-settings.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/fsmonitor-settings.c b/fsmonitor-settings.c index 658cb79da0..464424a1e9 100644 --- a/fsmonitor-settings.c +++ b/fsmonitor-settings.c @@ -202,11 +202,15 @@ char *fsm_settings__get_incompatible_msg(const struct repository *r, case FSMONITOR_REASON_OK: goto done; - case FSMONITOR_REASON_BARE: + case FSMONITOR_REASON_BARE: { + char *cwd = xgetcwd(); + strbuf_addf(&msg, _("bare repository '%s' is incompatible with fsmonitor"), - xgetcwd()); + cwd); + free(cwd); goto done; + } case FSMONITOR_REASON_ERROR: strbuf_addf(&msg, From f53559227ccb600f4fdd1bfe08e1164a5aed60b5 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 15 Jun 2022 23:35:39 +0000 Subject: [PATCH 2/6] submodule-config: avoid memory leak In 961b130d20c9 (branch: add --recurse-submodules option for branch creation, 2022-01-28), a funny pattern was introduced where first some struct is `xmalloc()`ed, then we resize an array whose element type is the same struct, and then the first struct's contents are copied into the last element of that array. Crucially, the `xmalloc()`ed memory never gets released. Let's avoid that memory leak and that memory allocation dance altogether by first reallocating the array, then using a pointer to the last array element to go forward. Reported by Coverity. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- submodule-config.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/submodule-config.c b/submodule-config.c index ce3beaf5d4..51ecbe901e 100644 --- a/submodule-config.c +++ b/submodule-config.c @@ -756,7 +756,10 @@ static void traverse_tree_submodules(struct repository *r, if (S_ISGITLINK(name_entry->mode) && is_tree_submodule_active(r, root_tree, tree_path)) { - st_entry = xmalloc(sizeof(*st_entry)); + ALLOC_GROW(out->entries, out->entry_nr + 1, + out->entry_alloc); + st_entry = &out->entries[out->entry_nr++]; + st_entry->name_entry = xmalloc(sizeof(*st_entry->name_entry)); *st_entry->name_entry = *name_entry; st_entry->submodule = @@ -766,9 +769,6 @@ static void traverse_tree_submodules(struct repository *r, root_tree)) FREE_AND_NULL(st_entry->repo); - ALLOC_GROW(out->entries, out->entry_nr + 1, - out->entry_alloc); - out->entries[out->entry_nr++] = *st_entry; } else if (S_ISDIR(name_entry->mode)) traverse_tree_submodules(r, root_tree, tree_path, &name_entry->oid, out); From 41a86b64c093a6f36ffe0959aeed2e3f2590c7e8 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 15 Jun 2022 23:35:41 +0000 Subject: [PATCH 3/6] submodule--helper: avoid memory leak when fetching submodules In c51f8f94e5b3 (submodule--helper: run update procedures from C, 2021-08-24), we added code that first obtains the default remote, and then adds that to a `strvec`. However, we never released the default remote's memory. Reported by Coverity. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 5c77dfcffe..c597df7528 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -2208,6 +2208,7 @@ static int fetch_in_submodule(const char *module_path, int depth, int quiet, str char *hex = oid_to_hex(oid); char *remote = get_default_remote(); strvec_pushl(&cp.args, remote, hex, NULL); + free(remote); } return run_command(&cp); From 652891de4ff164d545daa5472ab67f4f9d41319b Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 15 Jun 2022 23:35:42 +0000 Subject: [PATCH 4/6] read_index_from(): avoid memory leak In 998330ac2e7c (read-cache: look for shared index files next to the index, too, 2021-08-26), we added code that allocates memory to store the base path of a shared index, but we never released that memory. Reported by Coverity. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- read-cache.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/read-cache.c b/read-cache.c index e61af3a3d4..76f372ff91 100644 --- a/read-cache.c +++ b/read-cache.c @@ -2473,15 +2473,15 @@ int read_index_from(struct index_state *istate, const char *path, the_repository, "%s", base_path); if (!ret) { char *path_copy = xstrdup(path); - const char *base_path2 = xstrfmt("%s/sharedindex.%s", - dirname(path_copy), - base_oid_hex); + char *base_path2 = xstrfmt("%s/sharedindex.%s", + dirname(path_copy), base_oid_hex); free(path_copy); trace2_region_enter_printf("index", "shared/do_read_index", the_repository, "%s", base_path2); ret = do_read_index(split_index->base, base_path2, 1); trace2_region_leave_printf("index", "shared/do_read_index", the_repository, "%s", base_path2); + free(base_path2); } if (!oideq(&split_index->base_oid, &split_index->base->oid)) die(_("broken index, expect %s in %s, got %s"), From 41f1a8e6a417bc3e56a0eef687e28247138276d1 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 15 Jun 2022 23:35:43 +0000 Subject: [PATCH 5/6] pack-mtimes: avoid closing a bogus file descriptor In 94cd775a6c52 (pack-mtimes: support reading .mtimes files, 2022-05-20), code was added to close the file descriptor corresponding to the mtimes file. However, it is possible that opening that file failed, in which case we are closing a file descriptor with the value `-1`. Let's guard that `close()` call. Reported by Coverity. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- pack-mtimes.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pack-mtimes.c b/pack-mtimes.c index 0e0aafdcb0..0f9785fc5e 100644 --- a/pack-mtimes.c +++ b/pack-mtimes.c @@ -89,7 +89,8 @@ cleanup: *data_p = data; } - close(fd); + if (fd >= 0) + close(fd); return ret; } From c918f5c1ab0c4dec916b747916236ca0d3655be5 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 15 Jun 2022 23:35:44 +0000 Subject: [PATCH 6/6] relative_url(): fix incorrect condition In 63e95beb085c (submodule: port resolve_relative_url from shell to C, 2016-04-15), we added a loop over `url` where we are looking for `../` or `./` components. The loop condition we used is the pointer `url` itself, which is clearly not what we wanted. Pointed out by Coverity. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- remote.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/remote.c b/remote.c index 9b9bbfe80e..bded6acbfe 100644 --- a/remote.c +++ b/remote.c @@ -2846,7 +2846,7 @@ char *relative_url(const char *remote_url, const char *url, * When the url starts with '../', remove that and the * last directory in remoteurl. */ - while (url) { + while (*url) { if (starts_with_dot_dot_slash_native(url)) { url += 3; colonsep |= chop_last_dir(&remoteurl, is_relative);