[PATCH v2 4/5] config: fix a leak in git_config_copy_or_rename_section_in_file
- From
Rubén Justo <rjusto@gmail.com>
- Date
- Jun 16, 2023, 23:34 UTC
- Message-ID
- <adaa7fa7-befe-ce59-7391-5a4ddd3bb2b8@gmail.com>
- In-Reply-To
- <5650c4ed-cec0-d11e-4f68-1661b3638786@gmail.com>
A branch can have its configuration spread over several configuration sections. This situation was already foreseen in 52d59cc645 (branch: add a --copy (-c) option to go with --move (-m), 2017-06-18) when "branch -c" was introduced.
Unfortunately, a leak was also introduced:
$ git branch foo $ cat >> .git/config <<EOF [branch "foo"] some-key-a = a value [branch "foo"] some-key-b = b value [branch "foo"] some-key-c = c value EOF $ git branch -c foo bar
Direct leak of 130 byte(s) in 2 object(s) allocated from:
... in xrealloc wrapper.c
... in strbuf_grow strbuf.c
... in strbuf_vaddf strbuf.c
... in strbuf_addf strbuf.c
... in store_create_section config.c
... in git_config_copy_or_rename_section_in_file config.c
... in git_config_copy_section_in_file config.c
... in git_config_copy_section config.c
... in copy_or_rename_branch builtin/branch.c
... in cmd_branch builtin/branch.c
... in run_builtin git.cLet's fix it.
Let's also modify the function store_create_section() so that it stops returning a strbuf, which is an uncommon pattern in our code base, and instead, starts receiving the strbuf to use from its caller.
Signed-off-by: Rubén Justo <rjusto@gmail.com> --- config.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/config.c b/config.c index 39a7d7422c..c5f4b59ef3 100644 --- a/config.c +++ b/config.c @@ -3140,37 +3140,36 @@ static int write_error(const char *filename) return 4; } -static struct strbuf store_create_section(const char *key, - const struct config_store_data *store) +static void store_create_section(const char *key, + const struct config_store_data *store, + struct strbuf *sb) { const char *dot; size_t i; - struct strbuf sb = STRBUF_INIT; dot = memchr(key, '.', store->baselen); if (dot) { - strbuf_addf(&sb, "[%.*s \"", (int)(dot - key), key); + strbuf_addf(sb, "[%.*s \"", (int)(dot - key), key); for (i = dot - key + 1; i < store->baselen; i++) { if (key[i] == '"' || key[i] == '\\') - strbuf_addch(&sb, '\\'); - strbuf_addch(&sb, key[i]); + strbuf_addch(sb, '\\'); + strbuf_addch(sb, key[i]); } - strbuf_addstr(&sb, "\"]\n"); + strbuf_addstr(sb, "\"]\n"); } else { - strbuf_addch(&sb, '['); - strbuf_add(&sb, key, store->baselen); - strbuf_addstr(&sb, "]\n"); + strbuf_addch(sb, '['); + strbuf_add(sb, key, store->baselen); + strbuf_addstr(sb, "]\n"); } - - return sb; } static ssize_t write_section(int fd, const char *key, const struct config_store_data *store) { - struct strbuf sb = store_create_section(key, store); + struct strbuf sb = STRBUF_INIT; ssize_t ret; + store_create_section(key, store, &sb); ret = write_in_full(fd, sb.buf, sb.len); strbuf_release(&sb); @@ -3833,7 +3832,9 @@ static int git_config_copy_or_rename_section_in_file(const char *config_filename output[0] = '\t'; } } else { - copystr = store_create_section(new_name, &store); + strbuf_reset(©str); + store_create_section(new_name, &store, + ©str); } } remove = 0;
-- 2.40.1