| Message ID | RFC-cover-v2-0.4-00000000000-20211020T131627Z-avarab@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | for-each-ref: delay parsing of --sort=<atom> options + linux-leaks fixes | expand |
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > I tried to structure this as somethnig that could be merged or queued > up separately, but the semantic conflict in "branch.c" was difficult, > as Junio's patch changes how those variables are managed. I tend to agree that it would have been simpler if you did 1, 2 and 4 first and then rebased 3 on top, possibly as an "after the dust settles" separate topic. The releasing of these allocated pieces of memory does make sense. Thanks.
As noted in <211020.864k9boo0f.gmgdl@evledraar.gmail.com> the upthread patch breaks the linux-leaks job, since simple invocations of "git tag" start leaking memory: https://github.com/git/git/runs/3934925278 I tried to structure this as somethnig that could be merged or queued up separately, but the semantic conflict in "branch.c" was difficult, as Junio's patch changes how those variables are managed. Junio C Hamano (1): for-each-ref: delay parsing of --sort=<atom> options Ævar Arnfjörð Bjarmason (3): tag: use a "goto cleanup" pattern, leak less memory ref-filter API user: add and use a ref_sorting_release() branch: use ref_sorting_release() builtin/branch.c | 24 ++++++++++++----------- builtin/for-each-ref.c | 10 +++++----- builtin/ls-remote.c | 13 ++++++++----- builtin/tag.c | 42 ++++++++++++++++++++++------------------- ref-filter.c | 40 ++++++++++++++++++++++++++++++--------- ref-filter.h | 30 ++++++++++++----------------- t/t3200-branch.sh | 12 +++++++++++- t/t6300-for-each-ref.sh | 5 +++++ 8 files changed, 108 insertions(+), 68 deletions(-) Range-diff against v1: -: ----------- > 1: fc776c3f1cd tag: use a "goto cleanup" pattern, leak less memory -: ----------- > 2: 0ae71c19ab7 ref-filter API user: add and use a ref_sorting_release() 1: 21a1f4d3b08 ! 3: 7abbbe4468c for-each-ref: delay parsing of --sort=<atom> options @@ builtin/ls-remote.c: int cmd_ls_remote(int argc, const char **argv, const char * - if (sorting) + if (sorting_options.nr) { + struct ref_sorting *sorting; -+ UNLEAK(sorting); + + sorting = ref_sorting_options(&sorting_options); ref_array_sort(sorting, &ref_array); ++ ref_sorting_release(sorting); + } for (i = 0; i < ref_array.nr; i++) { @@ ref-filter.c: void parse_ref_sorting(struct ref_sorting **sorting_tail, const ch + return sorting; } - int parse_opt_merge_filter(const struct option *opt, const char *arg, int unset) + void ref_sorting_release(struct ref_sorting *sorting) ## ref-filter.h ## @@ @@ ref-filter.h: int format_ref_array_item(struct ref_array_item *info, -struct ref_sorting *ref_default_sorting(void); +/* Convert list of sort options into ref_sorting */ +struct ref_sorting *ref_sorting_options(struct string_list *); + /* Release a "struct ref_sorting" */ + void ref_sorting_release(struct ref_sorting *); /* Function to parse --merged and --no-merged options */ - int parse_opt_merge_filter(const struct option *opt, const char *arg, int unset); - /* Get the current HEAD's description */ ## t/t3200-branch.sh ## @@ t/t3200-branch.sh: test_expect_success 'invalid sort parameter in configuration' ' -: ----------- > 4: f7d87aea384 branch: use ref_sorting_release()