freshlybakedca.ke / git
Git fork
fork atom

reflog: close leak of reflog expire entry

find_cfg_ent() allocates a struct reflog_expire_entry_option via
FLEX_ALLOC_MEM and inserts it into a linked list in the
reflog_expire_options structure. The entries in this list are never
freed, resulting in a leak in cmd_reflog_expire and the gc reflog expire
maintenance task:

Direct leak of 39 byte(s) in 1 object(s) allocated from:
#0 0x7ff975ee6883 in calloc (/lib64/libasan.so.8+0xe6883)
#1 0x0000010edada in xcalloc ../wrapper.c:154
#2 0x000000df0898 in find_cfg_ent ../reflog.c:28
#3 0x000000df0898 in reflog_expire_config ../reflog.c:70
#4 0x00000095c451 in configset_iter ../config.c:2116
#5 0x0000006d29e7 in git_config ../config.h:724
#6 0x0000006d29e7 in cmd_reflog_expire ../builtin/reflog.c:205
#7 0x0000006d504c in cmd_reflog ../builtin/reflog.c:419
#8 0x0000007e4054 in run_builtin ../git.c:480
#9 0x0000007e4054 in handle_builtin ../git.c:746
#10 0x0000007e8a35 in run_argv ../git.c:813
#11 0x0000007e8a35 in cmd_main ../git.c:953
#12 0x000000441e8f in main ../common-main.c:9
#13 0x7ff9754115f4 in __libc_start_call_main (/lib64/libc.so.6+0x35f4)
#14 0x7ff9754116a7 in __libc_start_main@@GLIBC_2.34 (/lib64/libc.so.6+0x36a7)
#15 0x000000444184 in _start (/home/jekeller/libexec/git-core/git+0x444184)

Close this leak by adding a reflog_clear_expire_config() function which
iterates the linked list and frees its elements. Call it upon exit of
cmd_reflog_expire() and reflog_expire_condition().

Add a basic test which covers this leak. While at it, cover the
functionality from commit commit 3cb22b8efe (Per-ref reflog expiry
configuration, 2008-06-15). We've had this support for years, but lacked
any tests.

Co-developed-by: Jeff King <peff@peff.net>
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

authored by

Jacob Keller and committed by
Junio C Hamano 26552cb6 16bd9f20

+48
unified split
+1
builtin/gc.c
··· 324 324 count_reflog_entries, &data); 325 325 326 326 reflog_expiry_cleanup(&data.policy); 327 + reflog_clear_expire_config(&data.policy.opts); 327 328 return data.count >= data.limit; 328 329 } 329 330
+3
builtin/reflog.c
··· 283 283 &cb); 284 284 free(ref); 285 285 } 286 + 287 + reflog_clear_expire_config(&opts); 288 + 286 289 return status; 287 290 } 288 291
+14
reflog.c
··· 81 81 return 0; 82 82 } 83 83 84 + void reflog_clear_expire_config(struct reflog_expire_options *opts) 85 + { 86 + struct reflog_expire_entry_option *ent = opts->entries, *tmp; 87 + 88 + while (ent) { 89 + tmp = ent; 90 + ent = ent->next; 91 + free(tmp); 92 + } 93 + 94 + opts->entries = NULL; 95 + opts->entries_tail = NULL; 96 + } 97 + 84 98 void reflog_expire_options_set_refname(struct reflog_expire_options *cb, 85 99 const char *ref) 86 100 {
+2
reflog.h
··· 34 34 int reflog_expire_config(const char *var, const char *value, 35 35 const struct config_context *ctx, void *cb); 36 36 37 + void reflog_clear_expire_config(struct reflog_expire_options *opts); 38 + 37 39 /* 38 40 * Adapt the options so that they apply to the given refname. This applies any 39 41 * per-reference reflog expiry configuration that may exist to the options.
+28
t/t1410-reflog.sh
··· 673 673 ) 674 674 ' 675 675 676 + test_expect_success 'expire with pattern config' ' 677 + # Split refs/heads/ into two roots so we can apply config to each. Make 678 + # two branches per root to verify that config is applied correctly 679 + # multiple times. 680 + git branch root1/branch1 && 681 + git branch root1/branch2 && 682 + git branch root2/branch1 && 683 + git branch root2/branch2 && 684 + 685 + test_config "gc.reflogexpire" "never" && 686 + test_config "gc.refs/heads/root2/*.reflogExpire" "now" && 687 + git reflog expire \ 688 + root1/branch1 root1/branch2 \ 689 + root2/branch1 root2/branch2 && 690 + 691 + cat >expect <<-\EOF && 692 + root1/branch1@{0} 693 + root1/branch2@{0} 694 + EOF 695 + git log -g --branches="root*" --format=%gD >actual.raw && 696 + # The sole reflog entry of each branch points to the same commit, so 697 + # the order in which they are shown is nondeterministic. We just care 698 + # about the what was expired (and what was not), so sort to get a known 699 + # order. 700 + sort <actual.raw >actual.sorted && 701 + test_cmp expect actual.sorted 702 + ' 703 + 676 704 test_done