{"thread":{"id":"58199","subject":"[PATCH 0/2] string_list API users: use alloc + init, not calloc + strdup_strings","startedAt":"2022-07-21T06:39:58Z","lastAt":"2022-07-21T12:01:27Z","messageCount":11,"participants":["Ævar Arnfjörð Bjarmason","Elijah Newren"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"459573","messageId":"cover-0.2-00000000000-20220721T063543Z-avarab@gmail.com","threadId":"58199","inReplyTo":null,"subject":"[PATCH 0/2] string_list API users: use alloc + init, not calloc + strdup_strings","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-21T06:39:43Z","receivedAt":"2022-07-21T06:39:58Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"As a small follow-up to Junio's\nhttps://lore.kernel.org/git/xmqq7d471dns.fsf@gitster.g/; This is a\nsmall set of fixes to use the string_list functions rather than\npeeking into its guts during initalization.\n\nI've been running my local git version with these changes for almost a\nyear, but for submision I wrote a coccinelle rule to cover most of it\nin 1/2, along with a tests using the new recently landed coccicheck\ntest support.\n\nThe 2/2 is then things I had to manually change still, which weren't\ncovered by the conservative rule.\n\nÆvar Arnfjörð Bjarmason (2):\n  string_list API users + cocci: use string_list_init_dup()\n  string-list API users: manually use string_list_init_*()\n\n contrib/coccinelle/string_list.cocci     | 8 ++++++++\n contrib/coccinelle/tests/string_list.c   | 7 +++++++\n contrib/coccinelle/tests/string_list.res | 7 +++++++\n notes-utils.c                            | 4 ++--\n reflog-walk.c                            | 2 +-\n refs.c                                   | 4 ++--\n resolve-undo.c                           | 8 ++++----\n revision.c                               | 4 ++--\n 8 files changed, 33 insertions(+), 11 deletions(-)\n create mode 100644 contrib/coccinelle/string_list.cocci\n create mode 100644 contrib/coccinelle/tests/string_list.c\n create mode 100644 contrib/coccinelle/tests/string_list.res\n\n-- \n2.37.1.1095.g64a1e8362fd\n\n"},{"id":"459574","messageId":"patch-1.2-c89758491e7-20220721T063543Z-avarab@gmail.com","threadId":"58199","inReplyTo":"cover-0.2-00000000000-20220721T063543Z-avarab@gmail.com","subject":"[PATCH 1/2] string_list API users + cocci: use string_list_init_dup()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-21T06:39:44Z","receivedAt":"2022-07-21T06:39:59Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add a coccinelle rule to detect a particular misuse of the \"struct\nstring_list\" API. We have the *_INIT macros, but this code assumed\nthat a zero'd out \"struct string_list\" with a \"strdup_string\" set\nwould be the same as string_list_init_dup().\n\nThat assumption happens to be right, but let's instead use the helper\nfunctions introduced in 183113a5ca9 (string_list: Add STRING_LIST_INIT\nmacro and make use of it., 2010-07-04).\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n contrib/coccinelle/string_list.cocci     | 8 ++++++++\n contrib/coccinelle/tests/string_list.c   | 7 +++++++\n contrib/coccinelle/tests/string_list.res | 7 +++++++\n refs.c                                   | 4 ++--\n resolve-undo.c                           | 8 ++++----\n revision.c                               | 4 ++--\n 6 files changed, 30 insertions(+), 8 deletions(-)\n create mode 100644 contrib/coccinelle/string_list.cocci\n create mode 100644 contrib/coccinelle/tests/string_list.c\n create mode 100644 contrib/coccinelle/tests/string_list.res\n\ndiff --git a/contrib/coccinelle/string_list.cocci b/contrib/coccinelle/string_list.cocci\nnew file mode 100644\nindex 00000000000..5d285d5732c\n--- /dev/null\n+++ b/contrib/coccinelle/string_list.cocci\n@@ -0,0 +1,8 @@\n+@@\n+struct string_list *P;\n+@@\n+- CALLOC_ARRAY(P, 1);\n++ ALLOC_ARRAY(P, 1);\n+... when != P\n+- (P)->strdup_strings = 1;\n++ string_list_init_dup(P);\ndiff --git a/contrib/coccinelle/tests/string_list.c b/contrib/coccinelle/tests/string_list.c\nnew file mode 100644\nindex 00000000000..e77822b7682\n--- /dev/null\n+++ b/contrib/coccinelle/tests/string_list.c\n@@ -0,0 +1,7 @@\n+int init(void)\n+{\n+\tstruct string_list *list;\n+\n+\tCALLOC_ARRAY(list, 1);\n+\tlist->strdup_strings = 1;\n+}\ndiff --git a/contrib/coccinelle/tests/string_list.res b/contrib/coccinelle/tests/string_list.res\nnew file mode 100644\nindex 00000000000..7e666f5bf48\n--- /dev/null\n+++ b/contrib/coccinelle/tests/string_list.res\n@@ -0,0 +1,7 @@\n+int init(void)\n+{\n+\tstruct string_list *list;\n+\n+\tALLOC_ARRAY(list, 1);\n+\tstring_list_init_dup(list);\n+}\ndiff --git a/refs.c b/refs.c\nindex 90bcb271687..83151a42b3a 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1313,8 +1313,8 @@ int parse_hide_refs_config(const char *var, const char *value, const char *secti\n \t\twhile (len && ref[len - 1] == '/')\n \t\t\tref[--len] = '\\0';\n \t\tif (!hide_refs) {\n-\t\t\tCALLOC_ARRAY(hide_refs, 1);\n-\t\t\thide_refs->strdup_strings = 1;\n+\t\t\tALLOC_ARRAY(hide_refs, 1);\n+\t\t\tstring_list_init_dup(hide_refs);\n \t\t}\n \t\tstring_list_append(hide_refs, ref);\n \t}\ndiff --git a/resolve-undo.c b/resolve-undo.c\nindex e81096e2d45..e66b8306fe0 100644\n--- a/resolve-undo.c\n+++ b/resolve-undo.c\n@@ -15,8 +15,8 @@ void record_resolve_undo(struct index_state *istate, struct cache_entry *ce)\n \t\treturn;\n \n \tif (!istate->resolve_undo) {\n-\t\tCALLOC_ARRAY(resolve_undo, 1);\n-\t\tresolve_undo->strdup_strings = 1;\n+\t\tALLOC_ARRAY(resolve_undo, 1);\n+\t\tstring_list_init_dup(resolve_undo);\n \t\tistate->resolve_undo = resolve_undo;\n \t}\n \tresolve_undo = istate->resolve_undo;\n@@ -57,8 +57,8 @@ struct string_list *resolve_undo_read(const char *data, unsigned long size)\n \tint i;\n \tconst unsigned rawsz = the_hash_algo->rawsz;\n \n-\tCALLOC_ARRAY(resolve_undo, 1);\n-\tresolve_undo->strdup_strings = 1;\n+\tALLOC_ARRAY(resolve_undo, 1);\n+\tstring_list_init_dup(resolve_undo);\n \n \twhile (size) {\n \t\tstruct string_list_item *lost;\ndiff --git a/revision.c b/revision.c\nindex 0c6e26cd9c8..e44af92cacc 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1578,8 +1578,8 @@ void clear_ref_exclusion(struct string_list **ref_excludes_p)\n void add_ref_exclusion(struct string_list **ref_excludes_p, const char *exclude)\n {\n \tif (!*ref_excludes_p) {\n-\t\tCALLOC_ARRAY(*ref_excludes_p, 1);\n-\t\t(*ref_excludes_p)->strdup_strings = 1;\n+\t\tALLOC_ARRAY(*ref_excludes_p, 1);\n+\t\tstring_list_init_dup(*ref_excludes_p);\n \t}\n \tstring_list_append(*ref_excludes_p, exclude);\n }\n-- \n2.37.1.1095.g64a1e8362fd\n\n"},{"id":"459575","messageId":"patch-2.2-5d8baa9cbc4-20220721T063543Z-avarab@gmail.com","threadId":"58199","inReplyTo":"cover-0.2-00000000000-20220721T063543Z-avarab@gmail.com","subject":"[PATCH 2/2] string-list API users: manually use string_list_init_*()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-21T06:39:45Z","receivedAt":"2022-07-21T06:40:06Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Convert various code that didn't use string_list_init_*() to do so, in\ncases where the only thing being allocated was the string list we can\nchange from CALLOC_ARRAY() to ALLOC_ARRAY(), the string_list_init_*()\nfunction will zero out the memory.\n\nThis covers cases that weren't matched by tho coccinelle rule in the\npreceding commit, which is conservative enough to care about the type\nof what we're modifying.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n notes-utils.c | 4 ++--\n reflog-walk.c | 2 +-\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/notes-utils.c b/notes-utils.c\nindex d7d18e30f5a..73559d24ec8 100644\n--- a/notes-utils.c\n+++ b/notes-utils.c\n@@ -129,8 +129,8 @@ struct notes_rewrite_cfg *init_copy_notes_for_rewrite(const char *cmd)\n \tc->cmd = cmd;\n \tc->enabled = 1;\n \tc->combine = combine_notes_concatenate;\n-\tCALLOC_ARRAY(c->refs, 1);\n-\tc->refs->strdup_strings = 1;\n+\tALLOC_ARRAY(c->refs, 1);\n+\tstring_list_init_dup(c->refs);\n \tc->refs_from_env = 0;\n \tc->mode_from_env = 0;\n \tif (rewrite_mode_env) {\ndiff --git a/reflog-walk.c b/reflog-walk.c\nindex 7aa6595a51f..2b17408f9a4 100644\n--- a/reflog-walk.c\n+++ b/reflog-walk.c\n@@ -120,7 +120,7 @@ struct reflog_walk_info {\n void init_reflog_walk(struct reflog_walk_info **info)\n {\n \tCALLOC_ARRAY(*info, 1);\n-\t(*info)->complete_reflogs.strdup_strings = 1;\n+\tstring_list_init_dup(&((*info)->complete_reflogs));\n }\n \n void reflog_walk_info_release(struct reflog_walk_info *info)\n-- \n2.37.1.1095.g64a1e8362fd\n\n"},{"id":"459586","messageId":"CABPp-BFiNXM_2kfOQ+BnC5bWVEVZTk2kiFNuPSPPsz+5owM9kA@mail.gmail.com","threadId":"58199","inReplyTo":"patch-2.2-5d8baa9cbc4-20220721T063543Z-avarab@gmail.com","subject":"Re: [PATCH 2/2] string-list API users: manually use string_list_init_*()","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-07-21T07:48:51Z","receivedAt":"2022-07-21T07:49:06Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Wed, Jul 20, 2022 at 11:39 PM Ævar Arnfjörð Bjarmason\n<avarab@gmail.com> wrote:\n>\n> Convert various code that didn't use string_list_init_*() to do so, in\n> cases where the only thing being allocated was the string list we can\n> change from CALLOC_ARRAY() to ALLOC_ARRAY(), the string_list_init_*()\n> function will zero out the memory.\n>\n> This covers cases that weren't matched by tho coccinelle rule in the\n\ns/tho/the/ ?\n\n> preceding commit, which is conservative enough to care about the type\n> of what we're modifying.\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n> ---\n>  notes-utils.c | 4 ++--\n>  reflog-walk.c | 2 +-\n>  2 files changed, 3 insertions(+), 3 deletions(-)\n>\n> diff --git a/notes-utils.c b/notes-utils.c\n> index d7d18e30f5a..73559d24ec8 100644\n> --- a/notes-utils.c\n> +++ b/notes-utils.c\n> @@ -129,8 +129,8 @@ struct notes_rewrite_cfg *init_copy_notes_for_rewrite(const char *cmd)\n>         c->cmd = cmd;\n>         c->enabled = 1;\n>         c->combine = combine_notes_concatenate;\n> -       CALLOC_ARRAY(c->refs, 1);\n> -       c->refs->strdup_strings = 1;\n> +       ALLOC_ARRAY(c->refs, 1);\n> +       string_list_init_dup(c->refs);\n\nBut c->refs is a struct string_list *; why doesn't cocci recognize\nthis one and convert it?\n\n>         c->refs_from_env = 0;\n>         c->mode_from_env = 0;\n>         if (rewrite_mode_env) {\n> diff --git a/reflog-walk.c b/reflog-walk.c\n> index 7aa6595a51f..2b17408f9a4 100644\n> --- a/reflog-walk.c\n> +++ b/reflog-walk.c\n> @@ -120,7 +120,7 @@ struct reflog_walk_info {\n>  void init_reflog_walk(struct reflog_walk_info **info)\n>  {\n>         CALLOC_ARRAY(*info, 1);\n> -       (*info)->complete_reflogs.strdup_strings = 1;\n> +       string_list_init_dup(&((*info)->complete_reflogs));\n\nMakes sense.\n\n>  }\n>\n>  void reflog_walk_info_release(struct reflog_walk_info *info)\n> --\n> 2.37.1.1095.g64a1e8362fd\n>\n"},{"id":"459608","messageId":"cover-v2-0.6-00000000000-20220721T111808Z-avarab@gmail.com","threadId":"58199","inReplyTo":"cover-0.2-00000000000-20220721T063543Z-avarab@gmail.com","subject":"[PATCH v2 0/6] string-list API user: fix API use, some with coccinelle","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-21T12:00:47Z","receivedAt":"2022-07-21T12:01:18Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"A larger v2, this:\n\n * Renames string_list.cocci to string-list.cocci (to be consistent\n   with the string-list.[ch])\n\n * Answers Elijah's question about why the previous 2/2 left out one\n   case, it's due to --disable-worth-trying-opt. That's now split out\n   into its own commit.\n\n   FWIW it's the only case with \"make coccicheck\" that we've \"missed\",\n   including existing rules, but we just got lucky there.\n\n   I have some follow-up patches to fix various such bugs in our\n   coccicheck, but let's leave the larger fixes for now.\n\n * The new 4/6 duplicates Junio's\n   https://lore.kernel.org/git/xmqq7d471dns.fsf@gitster.g/, but now\n   we've got a coccinelle rule for it and similar inverse cases. It's\n   still the only in-tree case, but it's probably good to keep the\n   rule around regardless.\n\n   I could also eject that *.C change and say this is based on\n   Junio's, or both could make the same change and \"merge\" will sort\n   it out. I do think 4/6 is a bit easier to read with an actual case\n   where we change things, but then again there's the added tests...\n\n * A new 5/6 tweaks some \"strdup_strings\" dancing to a simpler\n   pattern, this is adjusted from my local WIP branch of larger\n   string_list API fixes.\n\n * A new 6/6 fixes a confusing case of dup v.s. nodup to use a more\n   obvious callback pattern where we don't flip-flop.\n\nÆvar Arnfjörð Bjarmason (6):\n  string_list API users + cocci: use string_list_init_dup()\n  cocci: apply string_list.cocci with --disable-worth-trying-opt\n  reflog-walk.c: use string_list_init_dup()\n  cocci: add \"string_list\" rule to swap \"DUP\" <-> \"NODUP\"\n  string-list API users: don't tweak \"strdup_strings\" to free dupes\n  notes.c: make \"struct string_list display_notes_refs\" non-static\n\n bisect.c                                 |  7 +++---\n builtin/remote.c                         |  3 +--\n contrib/coccinelle/string-list.cocci     | 26 ++++++++++++++++++++++\n contrib/coccinelle/tests/string-list.c   | 20 +++++++++++++++++\n contrib/coccinelle/tests/string-list.res | 18 +++++++++++++++\n notes-utils.c                            |  4 ++--\n notes.c                                  | 28 ++++++++++++++----------\n reflog-walk.c                            |  2 +-\n refs.c                                   |  4 ++--\n resolve-undo.c                           |  8 +++----\n revision.c                               |  4 ++--\n 11 files changed, 95 insertions(+), 29 deletions(-)\n create mode 100644 contrib/coccinelle/string-list.cocci\n create mode 100644 contrib/coccinelle/tests/string-list.c\n create mode 100644 contrib/coccinelle/tests/string-list.res\n\nRange-diff against v1:\n1:  c89758491e7 ! 1:  61a62bdf8e9 string_list API users + cocci: use string_list_init_dup()\n    @@ Commit message\n     \n         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n     \n    - ## contrib/coccinelle/string_list.cocci (new) ##\n    + ## contrib/coccinelle/string-list.cocci (new) ##\n     @@\n     +@@\n     +struct string_list *P;\n    @@ contrib/coccinelle/string_list.cocci (new)\n     +- (P)->strdup_strings = 1;\n     ++ string_list_init_dup(P);\n     \n    - ## contrib/coccinelle/tests/string_list.c (new) ##\n    + ## contrib/coccinelle/tests/string-list.c (new) ##\n     @@\n     +int init(void)\n     +{\n    @@ contrib/coccinelle/tests/string_list.c (new)\n     +\tlist->strdup_strings = 1;\n     +}\n     \n    - ## contrib/coccinelle/tests/string_list.res (new) ##\n    + ## contrib/coccinelle/tests/string-list.res (new) ##\n     @@\n     +int init(void)\n     +{\n2:  5d8baa9cbc4 ! 2:  33e551a2f4c string-list API users: manually use string_list_init_*()\n    @@ Metadata\n     Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n     \n      ## Commit message ##\n    -    string-list API users: manually use string_list_init_*()\n    +    cocci: apply string_list.cocci with --disable-worth-trying-opt\n     \n    -    Convert various code that didn't use string_list_init_*() to do so, in\n    -    cases where the only thing being allocated was the string list we can\n    -    change from CALLOC_ARRAY() to ALLOC_ARRAY(), the string_list_init_*()\n    -    function will zero out the memory.\n    +    Apply the new string-list.cocci added in the preceding commit with\n    +    --disable-worth-trying-opt. For optimization purposes we run spatch in\n    +    a mode where even though we run it with --all-includes we'll miss some\n    +    changes because we don't use --disable-worth-trying-opt.\n     \n    -    This covers cases that weren't matched by tho coccinelle rule in the\n    -    preceding commit, which is conservative enough to care about the type\n    -    of what we're modifying.\n    +    This is because without that option it'll take a look at\n    +    notes-utils.c, and conclude that it doesn't need to process\n    +    it (irrelevant output excluded with \"[...]\"):\n    +\n    +            $ spatch --sp-file contrib/coccinelle/string-list.cocci --patch . notes-utils.c\n    +            [...]\n    +            (ONCE) Expected tokens string_list strdup_strings CALLOC_ARRAY\n    +            Skipping: notes-utils.c\n    +\n    +    This is just one of the known (and probably some unknown) issues where\n    +    our \"make coccicheck\" fails to include changes for whatever\n    +    reason. That should be fixed more generally, but let's just fix this\n    +    manually for now.\n     \n         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n     \n    @@ notes-utils.c: struct notes_rewrite_cfg *init_copy_notes_for_rewrite(const char\n      \tc->refs_from_env = 0;\n      \tc->mode_from_env = 0;\n      \tif (rewrite_mode_env) {\n    -\n    - ## reflog-walk.c ##\n    -@@ reflog-walk.c: struct reflog_walk_info {\n    - void init_reflog_walk(struct reflog_walk_info **info)\n    - {\n    - \tCALLOC_ARRAY(*info, 1);\n    --\t(*info)->complete_reflogs.strdup_strings = 1;\n    -+\tstring_list_init_dup(&((*info)->complete_reflogs));\n    - }\n    - \n    - void reflog_walk_info_release(struct reflog_walk_info *info)\n-:  ----------- > 3:  62aab32ae77 reflog-walk.c: use string_list_init_dup()\n-:  ----------- > 4:  2d858c49243 cocci: add \"string_list\" rule to swap \"DUP\" <-> \"NODUP\"\n-:  ----------- > 5:  8c0ac6cbd96 string-list API users: don't tweak \"strdup_strings\" to free dupes\n-:  ----------- > 6:  b0de7a63d1c notes.c: make \"struct string_list display_notes_refs\" non-static\n-- \n2.37.1.1095.g64a1e8362fd\n\n"},{"id":"459609","messageId":"patch-v2-1.6-61a62bdf8e9-20220721T111808Z-avarab@gmail.com","threadId":"58199","inReplyTo":"cover-v2-0.6-00000000000-20220721T111808Z-avarab@gmail.com","subject":"[PATCH v2 1/6] string_list API users + cocci: use string_list_init_dup()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-21T12:00:48Z","receivedAt":"2022-07-21T12:01:19Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add a coccinelle rule to detect a particular misuse of the \"struct\nstring_list\" API. We have the *_INIT macros, but this code assumed\nthat a zero'd out \"struct string_list\" with a \"strdup_string\" set\nwould be the same as string_list_init_dup().\n\nThat assumption happens to be right, but let's instead use the helper\nfunctions introduced in 183113a5ca9 (string_list: Add STRING_LIST_INIT\nmacro and make use of it., 2010-07-04).\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n contrib/coccinelle/string-list.cocci     | 8 ++++++++\n contrib/coccinelle/tests/string-list.c   | 7 +++++++\n contrib/coccinelle/tests/string-list.res | 7 +++++++\n refs.c                                   | 4 ++--\n resolve-undo.c                           | 8 ++++----\n revision.c                               | 4 ++--\n 6 files changed, 30 insertions(+), 8 deletions(-)\n create mode 100644 contrib/coccinelle/string-list.cocci\n create mode 100644 contrib/coccinelle/tests/string-list.c\n create mode 100644 contrib/coccinelle/tests/string-list.res\n\ndiff --git a/contrib/coccinelle/string-list.cocci b/contrib/coccinelle/string-list.cocci\nnew file mode 100644\nindex 00000000000..5d285d5732c\n--- /dev/null\n+++ b/contrib/coccinelle/string-list.cocci\n@@ -0,0 +1,8 @@\n+@@\n+struct string_list *P;\n+@@\n+- CALLOC_ARRAY(P, 1);\n++ ALLOC_ARRAY(P, 1);\n+... when != P\n+- (P)->strdup_strings = 1;\n++ string_list_init_dup(P);\ndiff --git a/contrib/coccinelle/tests/string-list.c b/contrib/coccinelle/tests/string-list.c\nnew file mode 100644\nindex 00000000000..e77822b7682\n--- /dev/null\n+++ b/contrib/coccinelle/tests/string-list.c\n@@ -0,0 +1,7 @@\n+int init(void)\n+{\n+\tstruct string_list *list;\n+\n+\tCALLOC_ARRAY(list, 1);\n+\tlist->strdup_strings = 1;\n+}\ndiff --git a/contrib/coccinelle/tests/string-list.res b/contrib/coccinelle/tests/string-list.res\nnew file mode 100644\nindex 00000000000..7e666f5bf48\n--- /dev/null\n+++ b/contrib/coccinelle/tests/string-list.res\n@@ -0,0 +1,7 @@\n+int init(void)\n+{\n+\tstruct string_list *list;\n+\n+\tALLOC_ARRAY(list, 1);\n+\tstring_list_init_dup(list);\n+}\ndiff --git a/refs.c b/refs.c\nindex 90bcb271687..83151a42b3a 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1313,8 +1313,8 @@ int parse_hide_refs_config(const char *var, const char *value, const char *secti\n \t\twhile (len && ref[len - 1] == '/')\n \t\t\tref[--len] = '\\0';\n \t\tif (!hide_refs) {\n-\t\t\tCALLOC_ARRAY(hide_refs, 1);\n-\t\t\thide_refs->strdup_strings = 1;\n+\t\t\tALLOC_ARRAY(hide_refs, 1);\n+\t\t\tstring_list_init_dup(hide_refs);\n \t\t}\n \t\tstring_list_append(hide_refs, ref);\n \t}\ndiff --git a/resolve-undo.c b/resolve-undo.c\nindex e81096e2d45..e66b8306fe0 100644\n--- a/resolve-undo.c\n+++ b/resolve-undo.c\n@@ -15,8 +15,8 @@ void record_resolve_undo(struct index_state *istate, struct cache_entry *ce)\n \t\treturn;\n \n \tif (!istate->resolve_undo) {\n-\t\tCALLOC_ARRAY(resolve_undo, 1);\n-\t\tresolve_undo->strdup_strings = 1;\n+\t\tALLOC_ARRAY(resolve_undo, 1);\n+\t\tstring_list_init_dup(resolve_undo);\n \t\tistate->resolve_undo = resolve_undo;\n \t}\n \tresolve_undo = istate->resolve_undo;\n@@ -57,8 +57,8 @@ struct string_list *resolve_undo_read(const char *data, unsigned long size)\n \tint i;\n \tconst unsigned rawsz = the_hash_algo->rawsz;\n \n-\tCALLOC_ARRAY(resolve_undo, 1);\n-\tresolve_undo->strdup_strings = 1;\n+\tALLOC_ARRAY(resolve_undo, 1);\n+\tstring_list_init_dup(resolve_undo);\n \n \twhile (size) {\n \t\tstruct string_list_item *lost;\ndiff --git a/revision.c b/revision.c\nindex 0c6e26cd9c8..e44af92cacc 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1578,8 +1578,8 @@ void clear_ref_exclusion(struct string_list **ref_excludes_p)\n void add_ref_exclusion(struct string_list **ref_excludes_p, const char *exclude)\n {\n \tif (!*ref_excludes_p) {\n-\t\tCALLOC_ARRAY(*ref_excludes_p, 1);\n-\t\t(*ref_excludes_p)->strdup_strings = 1;\n+\t\tALLOC_ARRAY(*ref_excludes_p, 1);\n+\t\tstring_list_init_dup(*ref_excludes_p);\n \t}\n \tstring_list_append(*ref_excludes_p, exclude);\n }\n-- \n2.37.1.1095.g64a1e8362fd\n\n"},{"id":"459610","messageId":"patch-v2-2.6-33e551a2f4c-20220721T111808Z-avarab@gmail.com","threadId":"58199","inReplyTo":"cover-v2-0.6-00000000000-20220721T111808Z-avarab@gmail.com","subject":"[PATCH v2 2/6] cocci: apply string_list.cocci with --disable-worth-trying-opt","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-21T12:00:49Z","receivedAt":"2022-07-21T12:01:20Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Apply the new string-list.cocci added in the preceding commit with\n--disable-worth-trying-opt. For optimization purposes we run spatch in\na mode where even though we run it with --all-includes we'll miss some\nchanges because we don't use --disable-worth-trying-opt.\n\nThis is because without that option it'll take a look at\nnotes-utils.c, and conclude that it doesn't need to process\nit (irrelevant output excluded with \"[...]\"):\n\n\t$ spatch --sp-file contrib/coccinelle/string-list.cocci --patch . notes-utils.c\n\t[...]\n\t(ONCE) Expected tokens string_list strdup_strings CALLOC_ARRAY\n\tSkipping: notes-utils.c\n\nThis is just one of the known (and probably some unknown) issues where\nour \"make coccicheck\" fails to include changes for whatever\nreason. That should be fixed more generally, but let's just fix this\nmanually for now.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n notes-utils.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/notes-utils.c b/notes-utils.c\nindex d7d18e30f5a..73559d24ec8 100644\n--- a/notes-utils.c\n+++ b/notes-utils.c\n@@ -129,8 +129,8 @@ struct notes_rewrite_cfg *init_copy_notes_for_rewrite(const char *cmd)\n \tc->cmd = cmd;\n \tc->enabled = 1;\n \tc->combine = combine_notes_concatenate;\n-\tCALLOC_ARRAY(c->refs, 1);\n-\tc->refs->strdup_strings = 1;\n+\tALLOC_ARRAY(c->refs, 1);\n+\tstring_list_init_dup(c->refs);\n \tc->refs_from_env = 0;\n \tc->mode_from_env = 0;\n \tif (rewrite_mode_env) {\n-- \n2.37.1.1095.g64a1e8362fd\n\n"},{"id":"459611","messageId":"patch-v2-3.6-62aab32ae77-20220721T111808Z-avarab@gmail.com","threadId":"58199","inReplyTo":"cover-v2-0.6-00000000000-20220721T111808Z-avarab@gmail.com","subject":"[PATCH v2 3/6] reflog-walk.c: use string_list_init_dup()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-21T12:00:50Z","receivedAt":"2022-07-21T12:01:22Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change init_reflog_walk() to use the helper function added in\n183113a5ca9 (string_list: Add STRING_LIST_INIT macro and make use of\nit., 2010-07-04) rather than assuming that the \"struct string_list\"\npart of \"struct reflog_walk_info\" can be memset() to 0, followed by\nflipping \".strdup_strings = 1\".\n\nAs explained in a preceding commit that can be assumed now if we peek\nbehind the guts of the \"struct string_list\", but there's no reason to\nbreak the encapsulation here.\n\nThis does not match the newly added \"struct string_list\" rule for\nchanging CALLOC_ARRAY() followed by \".strdup_strings = 1\" to\n\"ALLOC_ARRAY()\" and \"string_list_init_dup()\" because here the\n\"CALLOC_ARRAY()\" is allocating more than just the \"struct\nstring_list\".\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n reflog-walk.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/reflog-walk.c b/reflog-walk.c\nindex 7aa6595a51f..2b17408f9a4 100644\n--- a/reflog-walk.c\n+++ b/reflog-walk.c\n@@ -120,7 +120,7 @@ struct reflog_walk_info {\n void init_reflog_walk(struct reflog_walk_info **info)\n {\n \tCALLOC_ARRAY(*info, 1);\n-\t(*info)->complete_reflogs.strdup_strings = 1;\n+\tstring_list_init_dup(&((*info)->complete_reflogs));\n }\n \n void reflog_walk_info_release(struct reflog_walk_info *info)\n-- \n2.37.1.1095.g64a1e8362fd\n\n"},{"id":"459612","messageId":"patch-v2-5.6-8c0ac6cbd96-20220721T111808Z-avarab@gmail.com","threadId":"58199","inReplyTo":"cover-v2-0.6-00000000000-20220721T111808Z-avarab@gmail.com","subject":"[PATCH v2 5/6] string-list API users: don't tweak \"strdup_strings\" to free dupes","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-21T12:00:52Z","receivedAt":"2022-07-21T12:01:23Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change code added in [1] and [2] used in notes.c and bisect.c to\ninitialize a \"struct string_list\" as \"DUP\" and append with \"nodup\",\nrather than doing it the other way around.\n\nSettings up a \"struct string_list\" as \"NODUP\" and then manually\nfreeing its items by flipping the \"strdup_strings\" breaks the\nencapsulation of the \"struct string_list\". It's also both more verbose\nthan the alternative, and not as safe. If we miss one of the codepaths\nthat appends to the list we'll end up freeing a constant string.\n\nIt's better to declare it as \"dup\", and then when we insert into the\nlist declare that the particular string we're inserting should be\nowned by the \"struct string_list\" with string_list_append_nodup(). The\nworst case with that API use is that we'll miss a caller, end up\ndouble-dup-ing the string, and have a memory leak as a result.\n\n1. 92e0d42539a (revision.c: make --no-notes reset --notes list,\n   2011-03-29)\n2. fb71a329964 (bisect--helper: `bisect_clean_state` shell function in\n   C, 2017-09-29)\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n bisect.c | 7 +++----\n notes.c  | 8 ++------\n 2 files changed, 5 insertions(+), 10 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex b63669cc9d7..8412feb1f69 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -1159,7 +1159,7 @@ static int mark_for_removal(const char *refname, const struct object_id *oid,\n {\n \tstruct string_list *refs = cb_data;\n \tchar *ref = xstrfmt(\"refs/bisect%s\", refname);\n-\tstring_list_append(refs, ref);\n+\tstring_list_append_nodup(refs, ref);\n \treturn 0;\n }\n \n@@ -1168,11 +1168,10 @@ int bisect_clean_state(void)\n \tint result = 0;\n \n \t/* There may be some refs packed during bisection */\n-\tstruct string_list refs_for_removal = STRING_LIST_INIT_NODUP;\n+\tstruct string_list refs_for_removal = STRING_LIST_INIT_DUP;\n \tfor_each_ref_in(\"refs/bisect\", mark_for_removal, (void *) &refs_for_removal);\n-\tstring_list_append(&refs_for_removal, xstrdup(\"BISECT_HEAD\"));\n+\tstring_list_append_nodup(&refs_for_removal, xstrdup(\"BISECT_HEAD\"));\n \tresult = delete_refs(\"bisect: remove\", &refs_for_removal, REF_NO_DEREF);\n-\trefs_for_removal.strdup_strings = 1;\n \tstring_list_clear(&refs_for_removal, 0);\n \tunlink_or_warn(git_path_bisect_expected_rev());\n \tunlink_or_warn(git_path_bisect_ancestors_ok());\ndiff --git a/notes.c b/notes.c\nindex 7452e71cc8d..acc35b580b6 100644\n--- a/notes.c\n+++ b/notes.c\n@@ -1064,19 +1064,15 @@ void enable_ref_display_notes(struct display_notes_opt *opt, int *show_notes,\n \tstruct strbuf buf = STRBUF_INIT;\n \tstrbuf_addstr(&buf, ref);\n \texpand_notes_ref(&buf);\n-\tstring_list_append(&opt->extra_notes_refs,\n-\t\t\tstrbuf_detach(&buf, NULL));\n+\tstring_list_append_nodup(&opt->extra_notes_refs,\n+\t\t\t\t strbuf_detach(&buf, NULL));\n \t*show_notes = 1;\n }\n \n void disable_display_notes(struct display_notes_opt *opt, int *show_notes)\n {\n \topt->use_default_notes = -1;\n-\t/* we have been strdup'ing ourselves, so trick\n-\t * string_list into free()ing strings */\n-\topt->extra_notes_refs.strdup_strings = 1;\n \tstring_list_clear(&opt->extra_notes_refs, 0);\n-\topt->extra_notes_refs.strdup_strings = 0;\n \t*show_notes = 0;\n }\n \n-- \n2.37.1.1095.g64a1e8362fd\n\n"},{"id":"459613","messageId":"patch-v2-4.6-2d858c49243-20220721T111808Z-avarab@gmail.com","threadId":"58199","inReplyTo":"cover-v2-0.6-00000000000-20220721T111808Z-avarab@gmail.com","subject":"[PATCH v2 4/6] cocci: add \"string_list\" rule to swap \"DUP\" <-> \"NODUP\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-21T12:00:51Z","receivedAt":"2022-07-21T12:01:24Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add a coccinelle rule to swap \"NODUP\" and \"DUP\" initialization in\ncases such as [1], which as this change shows produces an identical\nchange.\n\nWe happened to have only one change in-tree that matched this\ncriteria, but now we're more certain of that, and will convert these\nsorts of cases in the future.\n\n1. https://lore.kernel.org/git/xmqq7d471dns.fsf@gitster.g/\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/remote.c                         |  3 +--\n contrib/coccinelle/string-list.cocci     | 18 ++++++++++++++++++\n contrib/coccinelle/tests/string-list.c   | 13 +++++++++++++\n contrib/coccinelle/tests/string-list.res | 11 +++++++++++\n 4 files changed, 43 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex d9b8746cb3c..c713463d89d 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -1229,10 +1229,9 @@ static int get_one_entry(struct remote *remote, void *priv)\n \n static int show_all(void)\n {\n-\tstruct string_list list = STRING_LIST_INIT_NODUP;\n+\tstruct string_list list = STRING_LIST_INIT_DUP;\n \tint result;\n \n-\tlist.strdup_strings = 1;\n \tresult = for_each_remote(get_one_entry, &list);\n \n \tif (!result) {\ndiff --git a/contrib/coccinelle/string-list.cocci b/contrib/coccinelle/string-list.cocci\nindex 5d285d5732c..63bb2abc93a 100644\n--- a/contrib/coccinelle/string-list.cocci\n+++ b/contrib/coccinelle/string-list.cocci\n@@ -6,3 +6,21 @@ struct string_list *P;\n ... when != P\n - (P)->strdup_strings = 1;\n + string_list_init_dup(P);\n+\n+@@\n+type T;\n+identifier I;\n+constant INIT_NODUP =~ \"^STRING_LIST_INIT_NODUP$\";\n+constant INIT_DUP =~ \"^STRING_LIST_INIT_DUP$\";\n+@@\n+(\n+- T I = INIT_NODUP;\n++ T I = STRING_LIST_INIT_DUP;\n+... when != &I\n+- I.strdup_strings = 1;\n+|\n+- T I = INIT_DUP;\n++ T I = STRING_LIST_INIT_NODUP;\n+... when != &I\n+- I.strdup_strings = 0;\n+)\ndiff --git a/contrib/coccinelle/tests/string-list.c b/contrib/coccinelle/tests/string-list.c\nindex e77822b7682..1821ed4ebb4 100644\n--- a/contrib/coccinelle/tests/string-list.c\n+++ b/contrib/coccinelle/tests/string-list.c\n@@ -1,7 +1,20 @@\n int init(void)\n {\n \tstruct string_list *list;\n+\tstruct string_list list2 = STRING_LIST_INIT_NODUP;\n+\tstruct string_list list3 = STRING_LIST_INIT_DUP;\n+\tstruct string_list list4 = STRING_LIST_INIT_NODUP;\n+\tstruct string_list list5 = STRING_LIST_INIT_DUP;\n \n \tCALLOC_ARRAY(list, 1);\n+\n+\t/* Exclude these */\n+\tstring_list_append(&list4, \"str\");\n+\tstring_list_append_nodup(&list5, xstrdup(\"str\"));\n+\n \tlist->strdup_strings = 1;\n+\tlist2.strdup_strings = 1;\n+\tlist3.strdup_strings = 0;\n+\tlist4.strdup_strings = 1;\n+\tlist5.strdup_strings = 0;\n }\ndiff --git a/contrib/coccinelle/tests/string-list.res b/contrib/coccinelle/tests/string-list.res\nindex 7e666f5bf48..58b3733dec2 100644\n--- a/contrib/coccinelle/tests/string-list.res\n+++ b/contrib/coccinelle/tests/string-list.res\n@@ -1,7 +1,18 @@\n int init(void)\n {\n \tstruct string_list *list;\n+\tstruct string_list list2 = STRING_LIST_INIT_DUP;\n+\tstruct string_list list3 = STRING_LIST_INIT_NODUP;\n+\tstruct string_list list4 = STRING_LIST_INIT_NODUP;\n+\tstruct string_list list5 = STRING_LIST_INIT_DUP;\n \n \tALLOC_ARRAY(list, 1);\n+\n+\t/* Exclude these */\n+\tstring_list_append(&list4, \"str\");\n+\tstring_list_append_nodup(&list5, xstrdup(\"str\"));\n+\n \tstring_list_init_dup(list);\n+\tlist4.strdup_strings = 1;\n+\tlist5.strdup_strings = 0;\n }\n-- \n2.37.1.1095.g64a1e8362fd\n\n"},{"id":"459614","messageId":"patch-v2-6.6-b0de7a63d1c-20220721T111808Z-avarab@gmail.com","threadId":"58199","inReplyTo":"cover-v2-0.6-00000000000-20220721T111808Z-avarab@gmail.com","subject":"[PATCH v2 6/6] notes.c: make \"struct string_list display_notes_refs\" non-static","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-21T12:00:53Z","receivedAt":"2022-07-21T12:01:27Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the \"struct string_list\" added in 894a9d333e9 (Support showing\nnotes from more than one notes tree, 2010-03-12) to be non-static. Now\nwe don't need to flip-flop from \"NODUP\" to \".strdup_strings = 1\".\n\nIn 2721ce21e43 (use string_list initializer consistently, 2016-06-13)\nthe statically initialized variable was made to use\n\"STRING_LIST_INIT_NODUP\". That was correct in the narrow sense, but\ndidn't make much sense when this code was viewed as a whole. We'd set\nit to \"NODUP\" just to set it to \"DUP\" when we'd use it.\n\nThis change could be smaller in just changing the \"NODUP\" to a \"DUP\",\nbut let's prove to ourselves and the compiler that this data is only\never used when we enter load_display_notes(). We can just allocate it\nin that function, and pass it to the notes_display_config() callback.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n notes.c | 20 ++++++++++++++------\n 1 file changed, 14 insertions(+), 6 deletions(-)\n\ndiff --git a/notes.c b/notes.c\nindex acc35b580b6..272fcb270a1 100644\n--- a/notes.c\n+++ b/notes.c\n@@ -75,7 +75,6 @@ struct non_note {\n \n struct notes_tree default_notes_tree;\n \n-static struct string_list display_notes_refs = STRING_LIST_INIT_NODUP;\n static struct notes_tree **display_notes_trees;\n \n static void load_subtree(struct notes_tree *t, struct leaf_node *subtree,\n@@ -967,14 +966,19 @@ void string_list_add_refs_from_colon_sep(struct string_list *list,\n \tfree(globs_copy);\n }\n \n+struct notes_display_config_cb {\n+\tstruct string_list *list;\n+\tint *load_refs;\n+};\n+\n static int notes_display_config(const char *k, const char *v, void *cb)\n {\n-\tint *load_refs = cb;\n+\tstruct notes_display_config_cb *data = cb;\n \n-\tif (*load_refs && !strcmp(k, \"notes.displayref\")) {\n+\tif (*data->load_refs && !strcmp(k, \"notes.displayref\")) {\n \t\tif (!v)\n \t\t\treturn config_error_nonbool(k);\n-\t\tstring_list_add_refs_by_glob(&display_notes_refs, v);\n+\t\tstring_list_add_refs_by_glob(data->list, v);\n \t}\n \n \treturn 0;\n@@ -1080,7 +1084,11 @@ void load_display_notes(struct display_notes_opt *opt)\n {\n \tchar *display_ref_env;\n \tint load_config_refs = 0;\n-\tdisplay_notes_refs.strdup_strings = 1;\n+\tstruct string_list display_notes_refs = STRING_LIST_INIT_DUP;\n+\tstruct notes_display_config_cb cb_data = {\n+\t\t.list = &display_notes_refs,\n+\t\t.load_refs = &load_config_refs,\n+\t};\n \n \tassert(!display_notes_trees);\n \n@@ -1096,7 +1104,7 @@ void load_display_notes(struct display_notes_opt *opt)\n \t\t\tload_config_refs = 1;\n \t}\n \n-\tgit_config(notes_display_config, &load_config_refs);\n+\tgit_config(notes_display_config, &cb_data);\n \n \tif (opt) {\n \t\tstruct string_list_item *item;\n-- \n2.37.1.1095.g64a1e8362fd\n\n"}]}