Git fork

Merge branch 'jk/ref-filter-trailer-fixes'

Bugfixes and leak plugging in "git for-each-ref --format=..." code
paths.

* jk/ref-filter-trailer-fixes:
ref-filter: fix leak with unterminated %(if) atoms
ref-filter: add ref_format_clear() function
ref-filter: fix leak when formatting %(push:remoteref)
ref-filter: fix leak with %(describe) arguments
ref-filter: fix leak of %(trailers) "argbuf"
ref-filter: store ref_trailer_buf data per-atom
ref-filter: drop useless cast in trailers_atom_parser()
ref-filter: strip signature when parsing tag trailers
ref-filter: avoid extra copies of payload/signature
t6300: drop newline from wrapped test title

+124 -33
+1
builtin/branch.c
··· 878 878 string_list_clear(&output, 0); 879 879 ref_sorting_release(sorting); 880 880 ref_filter_clear(&filter); 881 + ref_format_clear(&format); 881 882 return 0; 882 883 } else if (edit_description) { 883 884 const char *branch_name;
+1
builtin/for-each-ref.c
··· 104 104 filter_and_format_refs(&filter, flags, sorting, &format); 105 105 106 106 ref_filter_clear(&filter); 107 + ref_format_clear(&format); 107 108 ref_sorting_release(sorting); 108 109 strvec_clear(&vec); 109 110 return 0;
+1
builtin/tag.c
··· 702 702 cleanup: 703 703 ref_sorting_release(sorting); 704 704 ref_filter_clear(&filter); 705 + ref_format_clear(&format); 705 706 strbuf_release(&buf); 706 707 strbuf_release(&ref); 707 708 strbuf_release(&reflog_msg);
+1
builtin/verify-tag.c
··· 65 65 if (format.format) 66 66 pretty_print_ref(name, &oid, &format); 67 67 } 68 + ref_format_clear(&format); 68 69 return had_error; 69 70 }
+72 -26
ref-filter.c
··· 75 75 int lstrip, rstrip; 76 76 }; 77 77 78 - static struct ref_trailer_buf { 78 + struct ref_trailer_buf { 79 79 struct string_list filter_list; 80 80 struct strbuf sepbuf; 81 81 struct strbuf kvsepbuf; 82 - } ref_trailer_buf = {STRING_LIST_INIT_NODUP, STRBUF_INIT, STRBUF_INIT}; 82 + }; 83 83 84 84 static struct expand_data { 85 85 struct object_id oid; ··· 201 201 enum { C_BARE, C_BODY, C_BODY_DEP, C_LENGTH, C_LINES, 202 202 C_SIG, C_SUB, C_SUB_SANITIZE, C_TRAILERS } option; 203 203 struct process_trailer_options trailer_opts; 204 + struct ref_trailer_buf *trailer_buf; 204 205 unsigned int nlines; 205 206 } contents; 206 207 struct { ··· 232 233 enum { S_BARE, S_GRADE, S_SIGNER, S_KEY, 233 234 S_FINGERPRINT, S_PRI_KEY_FP, S_TRUST_LEVEL } option; 234 235 } signature; 235 - const char **describe_args; 236 + struct strvec describe_args; 236 237 struct refname_atom refname; 237 238 char *head; 238 239 } u; ··· 566 567 atom->u.contents.trailer_opts.no_divider = 1; 567 568 568 569 if (arg) { 569 - const char *argbuf = xstrfmt("%s)", arg); 570 + char *argbuf = xstrfmt("%s)", arg); 571 + const char *arg = argbuf; 570 572 char *invalid_arg = NULL; 573 + struct ref_trailer_buf *tb; 574 + 575 + /* 576 + * Do not inline these directly into the used_atom struct! 577 + * When we parse them in format_set_trailers_options(), 578 + * we will make pointer references directly to them, 579 + * which will not survive a realloc() of the used_atom list. 580 + * They must be allocated in a separate, stable struct. 581 + */ 582 + atom->u.contents.trailer_buf = tb = xmalloc(sizeof(*tb)); 583 + string_list_init_dup(&tb->filter_list); 584 + strbuf_init(&tb->sepbuf, 0); 585 + strbuf_init(&tb->kvsepbuf, 0); 571 586 572 587 if (format_set_trailers_options(&atom->u.contents.trailer_opts, 573 - &ref_trailer_buf.filter_list, 574 - &ref_trailer_buf.sepbuf, 575 - &ref_trailer_buf.kvsepbuf, 576 - &argbuf, &invalid_arg)) { 588 + &tb->filter_list, 589 + &tb->sepbuf, &tb->kvsepbuf, 590 + &arg, &invalid_arg)) { 577 591 if (!invalid_arg) 578 592 strbuf_addf(err, _("expected %%(trailers:key=<value>)")); 579 593 else 580 594 strbuf_addf(err, _("unknown %%(trailers) argument: %s"), invalid_arg); 581 - free((char *)invalid_arg); 595 + free(invalid_arg); 596 + free(argbuf); 582 597 return -1; 583 598 } 599 + free(argbuf); 584 600 } 585 601 atom->u.contents.option = C_TRAILERS; 586 602 return 0; ··· 677 693 struct used_atom *atom, 678 694 const char *arg, struct strbuf *err) 679 695 { 680 - struct strvec args = STRVEC_INIT; 696 + strvec_init(&atom->u.describe_args); 681 697 682 698 for (;;) { 683 699 int found = 0; ··· 686 702 if (!arg || !*arg) 687 703 break; 688 704 689 - found = describe_atom_option_parser(&args, &arg, err); 705 + found = describe_atom_option_parser(&atom->u.describe_args, &arg, err); 690 706 if (found < 0) 691 707 return found; 692 708 if (!found) 693 709 return err_bad_arg(err, "describe", bad_arg); 694 710 } 695 - atom->u.describe_args = strvec_detach(&args); 696 711 return 0; 697 712 } 698 713 ··· 986 1001 struct ref_formatting_stack *prev; 987 1002 struct strbuf output; 988 1003 void (*at_end)(struct ref_formatting_stack **stack); 1004 + void (*at_end_data_free)(void *data); 989 1005 void *at_end_data; 990 1006 }; 991 1007 ··· 1154 1170 if (prev) 1155 1171 strbuf_addbuf(&prev->output, &current->output); 1156 1172 strbuf_release(&current->output); 1173 + if (current->at_end_data_free) 1174 + current->at_end_data_free(current->at_end_data); 1157 1175 free(current); 1158 1176 *stack = prev; 1159 1177 } ··· 1213 1231 } 1214 1232 1215 1233 *stack = cur; 1216 - free(if_then_else); 1217 1234 } 1218 1235 1219 1236 static int if_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state, 1220 1237 struct strbuf *err UNUSED) 1221 1238 { 1222 1239 struct ref_formatting_stack *new_stack; 1223 - struct if_then_else *if_then_else = xcalloc(1, 1224 - sizeof(struct if_then_else)); 1240 + struct if_then_else *if_then_else = xcalloc(1, sizeof(*if_then_else)); 1225 1241 1226 1242 if_then_else->str = atomv->atom->u.if_then_else.str; 1227 1243 if_then_else->cmp_status = atomv->atom->u.if_then_else.cmp_status; ··· 1230 1246 new_stack = state->stack; 1231 1247 new_stack->at_end = if_then_else_handler; 1232 1248 new_stack->at_end_data = if_then_else; 1249 + new_stack->at_end_data_free = free; 1233 1250 return 0; 1234 1251 } 1235 1252 ··· 1833 1850 size_t *nonsiglen, 1834 1851 const char **sig, size_t *siglen) 1835 1852 { 1836 - struct strbuf payload = STRBUF_INIT; 1837 - struct strbuf signature = STRBUF_INIT; 1838 1853 const char *eol; 1839 1854 const char *end = buf + strlen(buf); 1840 1855 const char *sigstart; 1841 1856 1842 - /* parse signature first; we might not even have a subject line */ 1843 - parse_signature(buf, end - buf, &payload, &signature); 1844 - strbuf_release(&payload); 1845 - 1846 1857 /* skip past header until we hit empty line */ 1847 1858 while (*buf && *buf != '\n') { 1848 1859 eol = strchrnul(buf, '\n'); ··· 1853 1864 /* skip any empty lines */ 1854 1865 while (*buf == '\n') 1855 1866 buf++; 1856 - *sig = strbuf_detach(&signature, siglen); 1867 + /* parse signature first; we might not even have a subject line */ 1857 1868 sigstart = buf + parse_signed_buffer(buf, strlen(buf)); 1869 + *sig = sigstart; 1870 + *siglen = end - *sig; 1858 1871 1859 1872 /* subject is first non-empty line */ 1860 1873 *sub = buf; ··· 1929 1942 1930 1943 cmd.git_cmd = 1; 1931 1944 strvec_push(&cmd.args, "describe"); 1932 - strvec_pushv(&cmd.args, atom->u.describe_args); 1945 + strvec_pushv(&cmd.args, atom->u.describe_args.v); 1933 1946 strvec_push(&cmd.args, oid_to_hex(&commit->object.oid)); 1934 1947 if (pipe_command(&cmd, NULL, 0, &out, 0, &err, 0) < 0) { 1935 1948 error(_("failed to run 'describe'")); ··· 2012 2025 v->s = strbuf_detach(&s, NULL); 2013 2026 } else if (atom->u.contents.option == C_TRAILERS) { 2014 2027 struct strbuf s = STRBUF_INIT; 2028 + const char *msg; 2029 + char *to_free = NULL; 2030 + 2031 + if (siglen) 2032 + msg = to_free = xmemdupz(subpos, sigpos - subpos); 2033 + else 2034 + msg = subpos; 2015 2035 2016 2036 /* Format the trailer info according to the trailer_opts given */ 2017 - format_trailers_from_commit(&atom->u.contents.trailer_opts, subpos, &s); 2037 + format_trailers_from_commit(&atom->u.contents.trailer_opts, msg, &s); 2038 + free(to_free); 2018 2039 2019 2040 v->s = strbuf_detach(&s, NULL); 2020 2041 } else if (atom->u.contents.option == C_BARE) 2021 2042 v->s = xstrdup(subpos); 2022 2043 2023 2044 } 2024 - free((void *)sigpos); 2025 2045 } 2026 2046 2027 2047 /* ··· 2219 2239 const char *merge; 2220 2240 2221 2241 merge = remote_ref_for_branch(branch, atom->u.remote_ref.push); 2222 - *s = xstrdup(merge ? merge : ""); 2242 + *s = merge ? merge : xstrdup(""); 2223 2243 } else 2224 2244 BUG("unhandled RR_* enum"); 2225 2245 } ··· 2985 3005 struct used_atom *atom = &used_atom[i]; 2986 3006 if (atom->atom_type == ATOM_HEAD) 2987 3007 free(atom->u.head); 3008 + else if (atom->atom_type == ATOM_DESCRIBE) 3009 + strvec_clear(&atom->u.describe_args); 3010 + else if (atom->atom_type == ATOM_TRAILERS || 3011 + (atom->atom_type == ATOM_CONTENTS && 3012 + atom->u.contents.option == C_TRAILERS)) { 3013 + struct ref_trailer_buf *tb = atom->u.contents.trailer_buf; 3014 + if (tb) { 3015 + string_list_clear(&tb->filter_list, 0); 3016 + strbuf_release(&tb->sepbuf); 3017 + strbuf_release(&tb->kvsepbuf); 3018 + free(tb); 3019 + } 3020 + } 2988 3021 free((char *)atom->name); 2989 3022 } 2990 3023 FREE_AND_NULL(used_atom); ··· 3590 3623 free_commit_list(filter->unreachable_from); 3591 3624 ref_filter_init(filter); 3592 3625 } 3626 + 3627 + void ref_format_init(struct ref_format *format) 3628 + { 3629 + struct ref_format blank = REF_FORMAT_INIT; 3630 + memcpy(format, &blank, sizeof(blank)); 3631 + } 3632 + 3633 + void ref_format_clear(struct ref_format *format) 3634 + { 3635 + string_list_clear(&format->bases, 0); 3636 + string_list_clear(&format->is_base_tips, 0); 3637 + ref_format_init(format); 3638 + }
+3
ref-filter.h
··· 221 221 void ref_filter_init(struct ref_filter *filter); 222 222 void ref_filter_clear(struct ref_filter *filter); 223 223 224 + void ref_format_init(struct ref_format *format); 225 + void ref_format_clear(struct ref_format *format); 226 + 224 227 #endif /* REF_FILTER_H */
+4 -4
remote.c
··· 632 632 static struct remote *remotes_remote_get(struct remote_state *remote_state, 633 633 const char *name); 634 634 635 - const char *remote_ref_for_branch(struct branch *branch, int for_push) 635 + char *remote_ref_for_branch(struct branch *branch, int for_push) 636 636 { 637 637 read_config(the_repository, 0); 638 638 die_on_missing_branch(the_repository, branch); ··· 640 640 if (branch) { 641 641 if (!for_push) { 642 642 if (branch->merge_nr) { 643 - return branch->merge_name[0]; 643 + return xstrdup(branch->merge_name[0]); 644 644 } 645 645 } else { 646 - const char *dst, 647 - *remote_name = remotes_pushremote_for_branch( 646 + char *dst; 647 + const char *remote_name = remotes_pushremote_for_branch( 648 648 the_repository->remote_state, branch, 649 649 NULL); 650 650 struct remote *remote = remotes_remote_get(
+1 -1
remote.h
··· 329 329 struct branch *branch_get(const char *name); 330 330 const char *remote_for_branch(struct branch *branch, int *explicit); 331 331 const char *pushremote_for_branch(struct branch *branch, int *explicit); 332 - const char *remote_ref_for_branch(struct branch *branch, int for_push); 332 + char *remote_ref_for_branch(struct branch *branch, int for_push); 333 333 334 334 /* returns true if the given branch has merge configuration given. */ 335 335 int branch_has_merge_config(struct branch *branch);
+39 -2
t/t6300-for-each-ref.sh
··· 5 5 6 6 test_description='for-each-ref test' 7 7 8 + TEST_PASSES_SANITIZE_LEAK=true 8 9 . ./test-lib.sh 9 10 GNUPGHOME_NOT_USED=$GNUPGHOME 10 11 . "$TEST_DIRECTORY"/lib-gpg.sh ··· 1560 1561 Reviewed-by,A U Thor <author@example.com>,Signed-off-by,A U Thor <author@example.com> 1561 1562 EOF 1562 1563 1564 + test_expect_success 'multiple %(trailers) use their own options' ' 1565 + git tag -F - tag-with-trailers <<-\EOF && 1566 + body 1567 + 1568 + one: foo 1569 + one: bar 1570 + two: baz 1571 + two: qux 1572 + EOF 1573 + t1="%(trailers:key=one,key_value_separator=W,separator=X)" && 1574 + t2="%(trailers:key=two,key_value_separator=Y,separator=Z)" && 1575 + git for-each-ref --format="$t1%0a$t2" refs/tags/tag-with-trailers >actual && 1576 + cat >expect <<-\EOF && 1577 + oneWfooXoneWbar 1578 + twoYbazZtwoYqux 1579 + EOF 1580 + test_cmp expect actual 1581 + ' 1582 + 1563 1583 test_failing_trailer_option () { 1564 1584 title=$1 option=$2 1565 1585 cat >expect ··· 1835 1855 sig_crlf=${sig_crlf%dummy} 1836 1856 test_atom refs/tags/fake-sig-crlf contents:signature "$sig_crlf" 1837 1857 1858 + test_expect_success 'set up tag with signature and trailers' ' 1859 + git tag -F - fake-sig-trailer <<-\EOF 1860 + this is the subject 1861 + 1862 + this is the body 1863 + 1864 + My-Trailer: foo 1865 + -----BEGIN PGP SIGNATURE----- 1866 + 1867 + not a real signature, but we just care about the 1868 + subject/body/trailer parsing. 1869 + -----END PGP SIGNATURE----- 1870 + EOF 1871 + ' 1872 + 1873 + # use "separator=" here to suppress the terminating newline 1874 + test_atom refs/tags/fake-sig-trailer trailers:separator= 'My-Trailer: foo' 1875 + 1838 1876 test_expect_success 'git for-each-ref --stdin: empty' ' 1839 1877 >in && 1840 1878 git for-each-ref --format="%(refname)" --stdin <in >actual && ··· 2003 2041 --format="$GRADE_FORMAT" >actual && 2004 2042 test_cmp expect actual 2005 2043 ' 2006 - test_expect_success GPGSSH 'show good signature with custom format 2007 - with ssh' ' 2044 + test_expect_success GPGSSH 'show good signature with custom format with ssh' ' 2008 2045 test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" && 2009 2046 FINGERPRINT=$(ssh-keygen -lf "${GPGSSH_KEY_PRIMARY}" | awk "{print \$2;}") && 2010 2047 cat >expect.tmpl <<-\EOF &&
+1
t/t6302-for-each-ref-filter.sh
··· 2 2 3 3 test_description='test for-each-refs usage of ref-filter APIs' 4 4 5 + TEST_PASSES_SANITIZE_LEAK=true 5 6 . ./test-lib.sh 6 7 . "$TEST_DIRECTORY"/lib-gpg.sh 7 8