{"thread":{"id":"24989","subject":"[PATCH] Documentation: document the string-list macros.","startedAt":"2010-09-05T17:51:17Z","lastAt":"2010-09-05T23:19:15Z","messageCount":6,"participants":["Thiago Farina","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"149983","messageId":"1283709077-5438-1-git-send-email-tfransosi@gmail.com","threadId":"24989","inReplyTo":null,"subject":"[PATCH] Documentation: document the string-list macros.","fromName":"Thiago Farina","fromEmail":"tfransosi@gmail.com","sentAt":"2010-09-05T17:51:17Z","receivedAt":"2010-09-05T17:51:17Z","isPatch":true,"sender":{"key":"tfransosi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/970071?v=4"},"body":"Add basic documentation about the string-list.h macros\nthat can be used to initialize the string_list structure.\n\nSigned-off-by: Thiago Farina <tfransosi@gmail.com>\n---\n Documentation/technical/api-string-list.txt |   12 ++++++++++++\n 1 files changed, 12 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/technical/api-string-list.txt b/Documentation/technical/api-string-list.txt\nindex 3f575bd..cec11b5 100644\n--- a/Documentation/technical/api-string-list.txt\n+++ b/Documentation/technical/api-string-list.txt\n@@ -52,6 +52,18 @@ However, if you use the list to check if a certain string was added\n already, you should not do that (using unsorted_string_list_has_string()),\n because the complexity would be quadratic again (but with a worse factor).\n \n+Macros\n+------\n+\n+`STRING_LIST_INIT_NODUP`::\n+\n+\tInitialize the members and set the `strdup_strings` member to 0.\n+\n+`STRING_LIST_INIT_DUP`::\n+\n+\tInitialize the members and set the `strdup_strings` member to 1.\n+\n+\n Functions\n ---------\n \n-- \n1.7.2.3.313.gcd15\n"},{"id":"149991","messageId":"20100905200323.GA14497@burratino","threadId":"24989","inReplyTo":"1283709077-5438-1-git-send-email-tfransosi@gmail.com","subject":"[demo/patch 0/3] Re: [PATCH] Documentation: document the string-list macros.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-09-05T20:03:23Z","receivedAt":"2010-09-05T20:03:23Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Thiago Farina wrote:\n\n> --- a/Documentation/technical/api-string-list.txt\n> +++ b/Documentation/technical/api-string-list.txt\n> @@ -52,6 +52,18 @@ However, if you use the list to check if a certain string was added\n>  already, you should not do that (using unsorted_string_list_has_string()),\n>  because the complexity would be quadratic again (but with a worse factor).\n>  \n> +Macros\n> +------\n> +\n> +`STRING_LIST_INIT_NODUP`::\n> +\n> +\tInitialize the members and set the `strdup_strings` member to 0.\n> +\n> +`STRING_LIST_INIT_DUP`::\n> +\n> +\tInitialize the members and set the `strdup_strings` member to 1.\n\nAfter reading that, one might be tempted to write\n\n\tstruct string_list x;\n\tSTRING_LIST_INIT_NODUP(x);\n\n, no?  In other words, I don't find the text very clear.\n\nIf you like working by example (like I do) then api-strbuf.txt might\ngive a good indication of how this sort of thing can be helpfully\ndocumented.\n\nMaybe something in this direction?\n\nPatch #3 in particular is very rough and ought to be split up for\neasier review.  This is not meant for application, just to give an\nidea.\n\nJonathan Nieder (3):\n  string-list: introduce string_list_init()\n  string-list: document ...\n  Make initialization of string_lists more consistent\n\n Documentation/technical/api-string-list.txt |   18 +++++++++------\n builtin/apply.c                             |    8 +++---\n builtin/blame.c                             |    4 +-\n builtin/clean.c                             |    2 +-\n builtin/commit.c                            |    4 +-\n builtin/fetch.c                             |   13 ++++-------\n builtin/fmt-merge-msg.c                     |   13 ++++++-----\n builtin/log.c                               |    9 ++-----\n builtin/mailsplit.c                         |    1 +\n builtin/notes.c                             |    4 +-\n builtin/remote.c                            |   30 +++++++++++++-------------\n builtin/shortlog.c                          |   25 ++++++++++++---------\n diff-no-index.c                             |    1 +\n mailmap.c                                   |   17 +++++++++-----\n mailmap.h                                   |    2 +-\n merge-recursive.c                           |   16 ++++++++------\n notes.c                                     |    4 +-\n pretty.c                                    |    5 ++-\n reflog-walk.c                               |    1 +\n resolve-undo.c                              |    8 +++---\n revision.c                                  |    7 ++++-\n string-list.c                               |   28 +++++++++++++++++++++---\n string-list.h                               |    4 +++\n submodule.c                                 |    4 +-\n wt-status.c                                 |    6 ++--\n 25 files changed, 137 insertions(+), 97 deletions(-)\n\n-- \n1.7.2.3\n"},{"id":"149992","messageId":"20100905200429.GB14497@burratino","threadId":"24989","inReplyTo":"20100905200323.GA14497@burratino","subject":"[demo/PATCH 1/3] string-list: introduce string_list_init()","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-09-05T20:04:30Z","receivedAt":"2010-09-05T20:04:30Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Instead of asking callers to\n\n\tmemset(&list, 0, sizeof(list));\n\tlist.strdup_strings = strdup_strings;\n\nwe can take care of that ourselves, providing the flexibility\nto change the details of string_list layout later if wanted.\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\n string-list.c |    7 +++++++\n string-list.h |    1 +\n 2 files changed, 8 insertions(+), 0 deletions(-)\n\ndiff --git a/string-list.c b/string-list.c\nindex 9b023a2..8e992a7 100644\n--- a/string-list.c\n+++ b/string-list.c\n@@ -102,6 +102,13 @@ int for_each_string_list(struct string_list *list,\n \treturn ret;\n }\n \n+void string_list_init(struct string_list *list, int strdup_strings)\n+{\n+\tlist->items = NULL;\n+\tlist->nr = list->alloc = 0;\n+\tlist->strdup_strings = strdup_strings ? 1 : 0;\n+}\n+\n void string_list_clear(struct string_list *list, int free_util)\n {\n \tif (list->items) {\ndiff --git a/string-list.h b/string-list.h\nindex 4946938..07e075c 100644\n--- a/string-list.h\n+++ b/string-list.h\n@@ -15,6 +15,7 @@ struct string_list\n #define STRING_LIST_INIT_NODUP { NULL, 0, 0, 0 }\n #define STRING_LIST_INIT_DUP   { NULL, 0, 0, 1 }\n \n+void string_list_init(struct string_list *list, int strdup_strings);\n void print_string_list(const struct string_list *p, const char *text);\n void string_list_clear(struct string_list *list, int free_util);\n \n-- \n1.7.2.3\n"},{"id":"149993","messageId":"20100905200634.GC14497@burratino","threadId":"24989","inReplyTo":"20100905200323.GA14497@burratino","subject":"[demo/PATCH 1/3] string-list: Document STRING_LIST_INIT_* and string_list_init()","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-09-05T20:06:34Z","receivedAt":"2010-09-05T20:06:34Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Clarify the modern ways to initialize a string_list.  Text roughly\nbased on the analogous passage from api-strbuf.txt.\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\n Documentation/technical/api-string-list.txt |   18 +++++++++++-------\n 1 files changed, 11 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/technical/api-string-list.txt b/Documentation/technical/api-string-list.txt\nindex 3f575bd..0f0e579 100644\n--- a/Documentation/technical/api-string-list.txt\n+++ b/Documentation/technical/api-string-list.txt\n@@ -9,12 +9,17 @@ because it is not specific to paths.\n \n The caller:\n \n-. Allocates and clears a `struct string_list` variable.\n+. Allocates a `struct string_list` variable\n \n-. Initializes the members. You might want to set the flag `strdup_strings`\n-  if the strings should be strdup()ed. For example, this is necessary\n-  when you add something like git_path(\"...\"), since that function returns\n-  a static buffer that will change with the next call to git_path().\n+. Initializes the members. A string_list has to be initialized by\n+  `string_list_init()` or by `= STRING_LIST_INIT_DUP` or\n+  `= STRING_LIST_INIT_NODUP` before it can be used.\n++\n+Strings in lists initialized with the _DUP variant will be\n+automatically strdup()ed on insertion and free()ed on removal.\n+For example, this is necessary when you add something like\n+git_path(\"...\"), since that function returns a static buffer\n+that will change with the next call to git_path().\n +\n If you need something advanced, you can manually malloc() the `items`\n member (you need this if you add things later) and you should set the\n@@ -34,10 +39,9 @@ member (you need this if you add things later) and you should set the\n Example:\n \n ----\n-struct string_list list;\n+struct string_list list STRING_LIST_INIT_NODUP;\n int i;\n \n-memset(&list, 0, sizeof(struct string_list));\n string_list_append(&list, \"foo\");\n string_list_append(&list, \"bar\");\n for (i = 0; i < list.nr; i++)\n-- \n1.7.2.3\n"},{"id":"149995","messageId":"20100905200833.GD14497@burratino","threadId":"24989","inReplyTo":"20100905200323.GA14497@burratino","subject":"[demo/PATCH 3/3] Make initialization of string_lists more consistent","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-09-05T20:08:33Z","receivedAt":"2010-09-05T20:08:33Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"The strdup_strings member of a string_list should be constant\nfrom when it is initialized until the string_list_clear() call,\nor there can be memory leaks and double-free()ing.  Clarify\nsome calling conventions to ensure this.\n\nActually it might be even better to have a completely distinct\nstring_list_dup type, so the compiler could catch that kind of\nerror, but this patch doesn't do that.\n\nPossible portability hazard: this casts function pointers in\nweird ways.\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\nbut not intended for application yet\n---\n builtin/apply.c         |    8 ++++----\n builtin/blame.c         |    4 ++--\n builtin/clean.c         |    2 +-\n builtin/commit.c        |    4 ++--\n builtin/fetch.c         |   13 +++++--------\n builtin/fmt-merge-msg.c |   13 +++++++------\n builtin/log.c           |    9 +++------\n builtin/mailsplit.c     |    1 +\n builtin/notes.c         |    4 ++--\n builtin/remote.c        |   30 +++++++++++++++---------------\n builtin/shortlog.c      |   25 ++++++++++++++-----------\n diff-no-index.c         |    1 +\n mailmap.c               |   17 +++++++++++------\n mailmap.h               |    2 +-\n merge-recursive.c       |   16 +++++++++-------\n notes.c                 |    4 ++--\n pretty.c                |    5 +++--\n reflog-walk.c           |    1 +\n resolve-undo.c          |    8 ++++----\n revision.c              |    7 +++++--\n string-list.c           |   21 +++++++++++++++++----\n string-list.h           |    3 +++\n submodule.c             |    4 ++--\n wt-status.c             |    6 +++---\n 24 files changed, 118 insertions(+), 90 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex 23c18c5..e9f16ce 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -223,7 +223,7 @@ struct image {\n  * the case where more than one patches touch the same file.\n  */\n \n-static struct string_list fn_table;\n+static struct string_list fn_table = STRING_LIST_INIT_NODUP;\n \n static uint32_t hash_line(const char *cp, size_t len)\n {\n@@ -3560,7 +3560,7 @@ static int write_out_results(struct patch *list, int skipped_patch)\n \n static struct lock_file lock_file;\n \n-static struct string_list limit_by_name;\n+static struct string_list limit_by_name = STRING_LIST_INIT_NODUP;\n static int has_include;\n static void add_name_limit(const char *name, int exclude)\n {\n@@ -3635,8 +3635,8 @@ static int apply_patch(int fd, const char *filename, int options)\n \tstruct patch *list = NULL, **listp = &list;\n \tint skipped_patch = 0;\n \n-\t/* FIXME - memory leak when using multiple patch files as inputs */\n-\tmemset(&fn_table, 0, sizeof(struct string_list));\n+\tstring_list_clear(&fn_table, 0);\n+\tstring_list_init(&fn_table, 0);\n \tpatch_input_file = filename;\n \tread_patch_file(&buf, fd);\n \toffset = 0;\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 1015354..bc89a48 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -45,7 +45,7 @@ static int xdl_opts;\n static enum date_mode blame_date_mode = DATE_ISO8601;\n static size_t blame_date_width;\n \n-static struct string_list mailmap;\n+static struct string_list mailmap = STRING_LIST_INIT_DUP;\n \n #ifndef DEBUG\n #define DEBUG 0\n@@ -2498,7 +2498,7 @@ parse_done:\n \tsb.ent = ent;\n \tsb.path = path;\n \n-\tread_mailmap(&mailmap, NULL);\n+\tmailmap_read(&mailmap, NULL);\n \n \tif (!incremental)\n \t\tsetup_pager();\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex b508d2c..c8798f5 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -44,7 +44,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \tstruct dir_struct dir;\n \tstatic const char **pathspec;\n \tstruct strbuf buf = STRBUF_INIT;\n-\tstruct string_list exclude_list = { NULL, 0, 0, 0 };\n+\tstruct string_list exclude_list = STRING_LIST_INIT_NODUP;\n \tconst char *qname;\n \tchar *seen = NULL;\n \tstruct option options[] = {\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 66fdd22..ab9d974 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -205,6 +205,7 @@ static int list_paths(struct string_list *list, const char *with_tree,\n \tint i;\n \tchar *m;\n \n+\tassert(list->strdup_strings);\n \tfor (i = 0; pattern[i]; i++)\n \t\t;\n \tm = xcalloc(1, i);\n@@ -379,8 +380,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int\n \tif (in_merge)\n \t\tdie(\"cannot do a partial commit during a merge.\");\n \n-\tmemset(&partial, 0, sizeof(partial));\n-\tpartial.strdup_strings = 1;\n+\tstring_list_init(&partial, 1);\n \tif (list_paths(&partial, initial_commit ? NULL : \"HEAD\", prefix, pathspec))\n \t\texit(1);\n \ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex fab3fce..9a1c46c 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -732,7 +732,7 @@ static int get_one_remote_for_fetch(struct remote *remote, void *priv)\n {\n \tstruct string_list *list = priv;\n \tif (!remote->skip_default_update)\n-\t\tstring_list_append(list, remote->name);\n+\t\tstring_list_append_take_ownership(list, (char *) remote->name);\n \treturn 0;\n }\n \n@@ -751,8 +751,8 @@ static int get_remote_group(const char *key, const char *value, void *priv)\n \t\tint space = strcspn(value, \" \\t\\n\");\n \t\twhile (*value) {\n \t\t\tif (space > 1) {\n-\t\t\t\tstring_list_append(g->list,\n-\t\t\t\t\t\t   xstrndup(value, space));\n+\t\t\t\tstring_list_append_take_ownership(\n+\t\t\t\t\tg->list, xstrndup(value, space));\n \t\t\t}\n \t\t\tvalue += space + (value[space] != '\\0');\n \t\t\tspace = strcspn(value, \" \\t\\n\");\n@@ -774,7 +774,7 @@ static int add_remote_or_group(const char *name, struct string_list *list)\n \t\tif (!remote_is_configured(name))\n \t\t\treturn 0;\n \t\tremote = remote_get(name);\n-\t\tstring_list_append(list, remote->name);\n+\t\tstring_list_append_take_ownership(list, (char *) remote->name);\n \t}\n \treturn 1;\n }\n@@ -877,7 +877,7 @@ static int fetch_one(struct remote *remote, int argc, const char **argv)\n int cmd_fetch(int argc, const char **argv, const char *prefix)\n {\n \tint i;\n-\tstruct string_list list = STRING_LIST_INIT_NODUP;\n+\tstruct string_list list = STRING_LIST_INIT_DUP;\n \tstruct remote *remote;\n \tint result = 0;\n \n@@ -921,9 +921,6 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)\n \t\t}\n \t}\n \n-\t/* All names were strdup()ed or strndup()ed */\n-\tlist.strdup_strings = 1;\n \tstring_list_clear(&list, 0);\n-\n \treturn result;\n }\ndiff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c\nindex e7e12ee..621c074 100644\n--- a/builtin/fmt-merge-msg.c\n+++ b/builtin/fmt-merge-msg.c\n@@ -30,12 +30,13 @@ struct src_data {\n \tint head_status;\n };\n \n-void init_src_data(struct src_data *data)\n+static void init_src_data(struct src_data *data)\n {\n-\tdata->branch.strdup_strings = 1;\n-\tdata->tag.strdup_strings = 1;\n-\tdata->r_branch.strdup_strings = 1;\n-\tdata->generic.strdup_strings = 1;\n+\tstring_list_init(&data->branch, 1);\n+\tstring_list_init(&data->tag, 1);\n+\tstring_list_init(&data->r_branch, 1);\n+\tstring_list_init(&data->generic, 1);\n+\tdata->head_status = 0;\n }\n \n static struct string_list srcs = STRING_LIST_INIT_DUP;\n@@ -83,7 +84,7 @@ static int handle_line(char *line)\n \titem = unsorted_string_list_lookup(&srcs, src);\n \tif (!item) {\n \t\titem = string_list_append(&srcs, src);\n-\t\titem->util = xcalloc(1, sizeof(struct src_data));\n+\t\titem->util = xmalloc(sizeof(struct src_data));\n \t\tinit_src_data(item->util);\n \t}\n \tsrc_data = item->util;\ndiff --git a/builtin/log.c b/builtin/log.c\nindex eaa1ee0..7e8f0e2 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -521,9 +521,9 @@ static int auto_number = 1;\n \n static char *default_attach = NULL;\n \n-static struct string_list extra_hdr;\n-static struct string_list extra_to;\n-static struct string_list extra_cc;\n+static struct string_list extra_hdr = STRING_LIST_INIT_DUP;\n+static struct string_list extra_to = STRING_LIST_INIT_DUP;\n+static struct string_list extra_cc = STRING_LIST_INIT_DUP;\n \n static void add_header(const char *value)\n {\n@@ -1048,9 +1048,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tOPT_END()\n \t};\n \n-\textra_hdr.strdup_strings = 1;\n-\textra_to.strdup_strings = 1;\n-\textra_cc.strdup_strings = 1;\n \tgit_config(git_format_config, NULL);\n \tinit_revisions(&rev, prefix);\n \trev.commit_format = CMIT_FMT_EMAIL;\ndiff --git a/builtin/mailsplit.c b/builtin/mailsplit.c\nindex 99654d0..17f247b 100644\n--- a/builtin/mailsplit.c\n+++ b/builtin/mailsplit.c\n@@ -108,6 +108,7 @@ static int populate_maildir_list(struct string_list *list, const char *path)\n \tchar *subs[] = { \"cur\", \"new\", NULL };\n \tchar **sub;\n \n+\tassert(list->strdup_strings);\n \tfor (sub = subs; *sub; ++sub) {\n \t\tsnprintf(name, sizeof(name), \"%s/%s\", path, *sub);\n \t\tif ((dir = opendir(name)) == NULL) {\ndiff --git a/builtin/notes.c b/builtin/notes.c\nindex fbc347c..35e139f 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -363,8 +363,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-\tc->refs = xcalloc(1, sizeof(struct string_list));\n-\tc->refs->strdup_strings = 1;\n+\tc->refs = xmalloc(sizeof(struct string_list));\n+\tstring_list_init(c->refs, 1);\n \tc->refs_from_env = 0;\n \tc->mode_from_env = 0;\n \tif (rewrite_mode_env) {\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex 48e0a6b..9ab36ed 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -230,7 +230,7 @@ struct branch_info {\n \tint rebase;\n };\n \n-static struct string_list branch_list;\n+static struct string_list branch_list = STRING_LIST_INIT_NODUP;\n \n static const char *abbrev_ref(const char *name, const char *prefix)\n {\n@@ -302,6 +302,10 @@ struct ref_states {\n \tint queried;\n };\n \n+#define REF_STATES_INIT { NULL, \\\n+\tSTRING_LIST_INIT_DUP, STRING_LIST_INIT_DUP, STRING_LIST_INIT_DUP, STRING_LIST_INIT_DUP, \\\n+\tSTRING_LIST_INIT_DUP, 0 }\n+\n static int get_ref_states(const struct ref *remote_refs, struct ref_states *states)\n {\n \tstruct ref *fetch_map = NULL, **tail = &fetch_map;\n@@ -313,9 +317,9 @@ static int get_ref_states(const struct ref *remote_refs, struct ref_states *stat\n \t\t\tdie(\"Could not get fetch map for refspec %s\",\n \t\t\t\tstates->remote->fetch_refspec[i]);\n \n-\tstates->new.strdup_strings = 1;\n-\tstates->tracked.strdup_strings = 1;\n-\tstates->stale.strdup_strings = 1;\n+\tassert(states->new.strdup_strings);\n+\tassert(states->tracked.strdup_strings);\n+\tassert(states->stale.strdup_strings);\n \tfor (ref = fetch_map; ref; ref = ref->next) {\n \t\tunsigned char sha1[20];\n \t\tif (!ref->peer_ref || read_ref(ref->peer_ref->name, sha1))\n@@ -366,7 +370,7 @@ static int get_push_ref_states(const struct ref *remote_refs,\n \tmatch_refs(local_refs, &push_map, remote->push_refspec_nr,\n \t\t   remote->push_refspec, MATCH_REFS_NONE);\n \n-\tstates->push.strdup_strings = 1;\n+\tassert(states->push.strdup_strings);\n \tfor (ref = push_map; ref; ref = ref->next) {\n \t\tstruct string_list_item *item;\n \t\tstruct push_info *info;\n@@ -409,7 +413,7 @@ static int get_push_ref_states_noquery(struct ref_states *states)\n \tif (remote->mirror)\n \t\treturn 0;\n \n-\tstates->push.strdup_strings = 1;\n+\tassert(states->push.strdup_strings);\n \tif (!remote->push_refspec_nr) {\n \t\titem = string_list_append(&states->push, \"(matching)\");\n \t\tinfo = item->util = xcalloc(sizeof(struct push_info), 1);\n@@ -442,7 +446,7 @@ static int get_head_names(const struct ref *remote_refs, struct ref_states *stat\n \trefspec.force = 0;\n \trefspec.pattern = 1;\n \trefspec.src = refspec.dst = \"refs/heads/*\";\n-\tstates->heads.strdup_strings = 1;\n+\tassert(states->heads.strdup_strings);\n \tget_fetch_map(remote_refs, &refspec, &fetch_map_tail, 0);\n \tmatches = guess_remote_head(find_ref_by_name(remote_refs, \"HEAD\"),\n \t\t\t\t    fetch_map, 1);\n@@ -1043,7 +1047,7 @@ static int show(int argc, const char **argv)\n \t\tOPT_BOOLEAN('n', NULL, &no_query, \"do not query remotes\"),\n \t\tOPT_END()\n \t};\n-\tstruct ref_states states;\n+\tstruct ref_states states = REF_STATES_INIT;\n \tstruct string_list info_list = STRING_LIST_INIT_NODUP;\n \tstruct show_info info;\n \n@@ -1056,7 +1060,6 @@ static int show(int argc, const char **argv)\n \tif (!no_query)\n \t\tquery_flag = (GET_REF_STATES | GET_HEAD_NAMES | GET_PUSH_REF_STATES);\n \n-\tmemset(&states, 0, sizeof(states));\n \tmemset(&info, 0, sizeof(info));\n \tinfo.states = &states;\n \tinfo.list = &info_list;\n@@ -1158,8 +1161,7 @@ static int set_head(int argc, const char **argv)\n \tif (!opt_a && !opt_d && argc == 2) {\n \t\thead_name = xstrdup(argv[1]);\n \t} else if (opt_a && !opt_d && argc == 1) {\n-\t\tstruct ref_states states;\n-\t\tmemset(&states, 0, sizeof(states));\n+\t\tstruct ref_states states = REF_STATES_INIT;\n \t\tget_remote_ref_states(argv[0], &states, GET_HEAD_NAMES);\n \t\tif (!states.heads.nr)\n \t\t\tresult |= error(\"Cannot determine remote HEAD\");\n@@ -1219,12 +1221,11 @@ static int prune(int argc, const char **argv)\n static int prune_remote(const char *remote, int dry_run)\n {\n \tint result = 0, i;\n-\tstruct ref_states states;\n+\tstruct ref_states states = REF_STATES_INIT;\n \tconst char *dangling_msg = dry_run\n \t\t? \" %s will become dangling!\\n\"\n \t\t: \" %s has become dangling!\\n\";\n \n-\tmemset(&states, 0, sizeof(states));\n \tget_remote_ref_states(remote, &states, GET_REF_STATES);\n \n \tif (states.stale.nr) {\n@@ -1483,10 +1484,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/builtin/shortlog.c b/builtin/shortlog.c\nindex 2135b0d..5843c0a 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -16,9 +16,9 @@ static char const * const shortlog_usage[] = {\n \tNULL\n };\n \n-static int compare_by_number(const void *a1, const void *a2)\n+static int compare_by_number(const struct string_list_item *i1,\n+\t\t\t     const struct string_list_item *i2)\n {\n-\tconst struct string_list_item *i1 = a1, *i2 = a2;\n \tconst struct string_list *l1 = i1->util, *l2 = i2->util;\n \n \tif (l1->nr < l2->nr)\n@@ -84,9 +84,12 @@ static void insert_one_record(struct shortlog *log,\n \t\tsnprintf(namebuf + len, room, \" <%.*s>\", maillen, emailbuf);\n \t}\n \n+\tassert(log->list.strdup_strings);\n \titem = string_list_insert(&log->list, namebuf);\n-\tif (item->util == NULL)\n-\t\titem->util = xcalloc(1, sizeof(struct string_list));\n+\tif (!item->util) {\n+\t\titem->util = xmalloc(sizeof(struct string_list));\n+\t\tstring_list_init(item->util, 1);\n+\t}\n \n \t/* Skip any leading whitespace, including any blank lines. */\n \twhile (*oneline && isspace(*oneline))\n@@ -115,7 +118,7 @@ static void insert_one_record(struct shortlog *log,\n \t\t}\n \t}\n \n-\tstring_list_append(item->util, buffer);\n+\tstring_list_append_take_ownership(item->util, buffer);\n }\n \n static void read_from_stdin(struct shortlog *log)\n@@ -236,10 +239,11 @@ static int parse_wrap_args(const struct option *opt, const char *arg, int unset)\n void shortlog_init(struct shortlog *log)\n {\n \tmemset(log, 0, sizeof(*log));\n+\tstring_list_init(&log->list, 1);\n+\tstring_list_init(&log->mailmap, 1);\n \n-\tread_mailmap(&log->mailmap, &log->common_repo_prefix);\n+\tmailmap_read(&log->mailmap, &log->common_repo_prefix);\n \n-\tlog->list.strdup_strings = 1;\n \tlog->wrap = DEFAULT_WRAPLEN;\n \tlog->in1 = DEFAULT_INDENT1;\n \tlog->in2 = DEFAULT_INDENT2;\n@@ -320,8 +324,7 @@ void shortlog_output(struct shortlog *log)\n \tstruct strbuf sb = STRBUF_INIT;\n \n \tif (log->sort_by_number)\n-\t\tqsort(log->list.items, log->list.nr, sizeof(struct string_list_item),\n-\t\t\tcompare_by_number);\n+\t\tsort_string_list_by(&log->list, compare_by_number);\n \tfor (i = 0; i < log->list.nr; i++) {\n \t\tstruct string_list *onelines = log->list.items[i].util;\n \n@@ -343,14 +346,14 @@ void shortlog_output(struct shortlog *log)\n \t\t\tputchar('\\n');\n \t\t}\n \n-\t\tonelines->strdup_strings = 1;\n+\t\tassert(onelines->strdup_strings);\n \t\tstring_list_clear(onelines, 0);\n \t\tfree(onelines);\n \t\tlog->list.items[i].util = NULL;\n \t}\n \n \tstrbuf_release(&sb);\n-\tlog->list.strdup_strings = 1;\n+\tassert(log->list.strdup_strings);\n \tstring_list_clear(&log->list, 1);\n \tclear_mailmap(&log->mailmap);\n }\ndiff --git a/diff-no-index.c b/diff-no-index.c\nindex ce9e783..68da01a 100644\n--- a/diff-no-index.c\n+++ b/diff-no-index.c\n@@ -24,6 +24,7 @@ static int read_directory(const char *path, struct string_list *list)\n \tif (!(dir = opendir(path)))\n \t\treturn error(\"Could not open directory %s\", path);\n \n+\tassert(list->strdup_strings);\n \twhile ((e = readdir(dir)))\n \t\tif (strcmp(\".\", e->d_name) && strcmp(\"..\", e->d_name))\n \t\t\tstring_list_insert(list, e->d_name);\ndiff --git a/mailmap.c b/mailmap.c\nindex f80b701..b7f2796 100644\n--- a/mailmap.c\n+++ b/mailmap.c\n@@ -33,6 +33,12 @@ static void free_mailmap_info(void *p, const char *s)\n \tfree(mi->email);\n }\n \n+static void init_mailmap_entry(struct mailmap_entry *p)\n+{\n+\tp->name = p->email = NULL;\n+\tstring_list_init(&p->namemap, 1);\n+}\n+\n static void free_mailmap_entry(void *p, const char *s)\n {\n \tstruct mailmap_entry *me = (struct mailmap_entry *)p;\n@@ -41,7 +47,7 @@ static void free_mailmap_entry(void *p, const char *s)\n \tfree(me->name);\n \tfree(me->email);\n \n-\tme->namemap.strdup_strings = 1;\n+\tassert(me->namemap.strdup_strings);\n \tstring_list_clear_func(&me->namemap, free_mailmap_info);\n }\n \n@@ -71,8 +77,7 @@ static void add_mapping(struct string_list *map,\n \t\t/* create mailmap entry */\n \t\tstruct string_list_item *item = string_list_insert_at_index(map, index, old_email);\n \t\titem->util = xmalloc(sizeof(struct mailmap_entry));\n-\t\tmemset(item->util, 0, sizeof(struct mailmap_entry));\n-\t\t((struct mailmap_entry *)item->util)->namemap.strdup_strings = 1;\n+\t\tinit_mailmap_entry((struct mailmap_entry *)item->util);\n \t}\n \tme = (struct mailmap_entry *)map->items[index].util;\n \n@@ -170,9 +175,9 @@ static int read_single_mailmap(struct string_list *map, const char *filename, ch\n \treturn 0;\n }\n \n-int read_mailmap(struct string_list *map, char **repo_abbrev)\n+int mailmap_read(struct string_list *map, char **repo_abbrev)\n {\n-\tmap->strdup_strings = 1;\n+\tassert(map->strdup_strings);\n \t/* each failure returns 1, so >1 means both calls failed */\n \treturn read_single_mailmap(map, \".mailmap\", repo_abbrev) +\n \t       read_single_mailmap(map, git_mailmap_file, repo_abbrev) > 1;\n@@ -181,7 +186,7 @@ int read_mailmap(struct string_list *map, char **repo_abbrev)\n void clear_mailmap(struct string_list *map)\n {\n \tdebug_mm(\"mailmap: clearing %d entries...\\n\", map->nr);\n-\tmap->strdup_strings = 1;\n+\tassert(map->strdup_strings);\n \tstring_list_clear_func(map, free_mailmap_entry);\n \tdebug_mm(\"mailmap: cleared\\n\");\n }\ndiff --git a/mailmap.h b/mailmap.h\nindex d5c3664..f0b3861 100644\n--- a/mailmap.h\n+++ b/mailmap.h\n@@ -1,7 +1,7 @@\n #ifndef MAILMAP_H\n #define MAILMAP_H\n \n-int read_mailmap(struct string_list *map, char **repo_abbrev);\n+int mailmap_read(struct string_list *map, char **repo_abbrev);\n void clear_mailmap(struct string_list *map);\n \n int map_user(struct string_list *mailmap,\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 20e1779..7a44e25 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -232,6 +232,9 @@ static int save_files_dirs(const unsigned char *sha1,\n \tmemcpy(newpath + baselen, path, len);\n \tnewpath[baselen + len] = '\\0';\n \n+\tassert(o->current_directory_set.strdup_strings);\n+\tassert(o->current_file_set.strdup_strings);\n+\n \tif (S_ISDIR(mode))\n \t\tstring_list_insert(&o->current_directory_set, newpath);\n \telse\n@@ -277,10 +280,10 @@ static struct stage_data *insert_stage_data(const char *path,\n  */\n static struct string_list *get_unmerged(void)\n {\n-\tstruct string_list *unmerged = xcalloc(1, sizeof(struct string_list));\n+\tstruct string_list *unmerged = xmalloc(sizeof(struct string_list));\n \tint i;\n \n-\tunmerged->strdup_strings = 1;\n+\tstring_list_init(unmerged, 1);\n \n \tfor (i = 0; i < active_nr; i++) {\n \t\tstruct string_list_item *item;\n@@ -327,7 +330,8 @@ static struct string_list *get_renames(struct merge_options *o,\n \tstruct string_list *renames;\n \tstruct diff_options opts;\n \n-\trenames = xcalloc(1, sizeof(struct string_list));\n+\trenames = xmalloc(sizeof(struct string_list));\n+\tstring_list_init(renames, 0);\n \tdiff_setup(&opts);\n \tDIFF_OPT_SET(&opts, RECURSIVE);\n \topts.detect_rename = DIFF_DETECT_RENAME;\n@@ -1539,8 +1543,6 @@ void init_merge_options(struct merge_options *o)\n \tif (o->verbosity >= 5)\n \t\to->buffer_output = 0;\n \tstrbuf_init(&o->obuf, 0);\n-\tmemset(&o->current_file_set, 0, sizeof(struct string_list));\n-\to->current_file_set.strdup_strings = 1;\n-\tmemset(&o->current_directory_set, 0, sizeof(struct string_list));\n-\to->current_directory_set.strdup_strings = 1;\n+\tstring_list_init(&o->current_file_set, 1);\n+\tstring_list_init(&o->current_directory_set, 1);\n }\ndiff --git a/notes.c b/notes.c\nindex 7fd2035..2ab0bb1 100644\n--- a/notes.c\n+++ b/notes.c\n@@ -70,7 +70,7 @@ struct non_note {\n \n struct notes_tree default_notes_tree;\n \n-static struct string_list display_notes_refs;\n+static struct string_list display_notes_refs = STRING_LIST_INIT_DUP;\n static struct notes_tree **display_notes_trees;\n \n static void load_subtree(struct notes_tree *t, struct leaf_node *subtree,\n@@ -958,8 +958,8 @@ void init_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 \n+\tassert(display_notes_refs.strdup_strings);\n \tassert(!display_notes_trees);\n \n \tif (!opt || !opt->suppress_default_notes) {\ndiff --git a/pretty.c b/pretty.c\nindex f85444b..89b37eb 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -434,8 +434,9 @@ static int mailmap_name(char *email, int email_len, char *name, int name_len)\n {\n \tstatic struct string_list *mail_map;\n \tif (!mail_map) {\n-\t\tmail_map = xcalloc(1, sizeof(*mail_map));\n-\t\tread_mailmap(mail_map, NULL);\n+\t\tmail_map = xmalloc(sizeof(*mail_map));\n+\t\tstring_list_init(mail_map, 1);\n+\t\tmailmap_read(mail_map, NULL);\n \t}\n \treturn mail_map->nr && map_user(mail_map, email, email_len, name, name_len);\n }\ndiff --git a/reflog-walk.c b/reflog-walk.c\nindex 4879615..a2ccdea 100644\n--- a/reflog-walk.c\n+++ b/reflog-walk.c\n@@ -135,6 +135,7 @@ struct reflog_walk_info {\n void init_reflog_walk(struct reflog_walk_info** info)\n {\n \t*info = xcalloc(sizeof(struct reflog_walk_info), 1);\n+\tstring_list_init(&(*info)->complete_reflogs, 0);\n }\n \n int add_reflog_for_walk(struct reflog_walk_info *info,\ndiff --git a/resolve-undo.c b/resolve-undo.c\nindex 72b4612..6ba8538 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\tresolve_undo = xcalloc(1, sizeof(*resolve_undo));\n-\t\tresolve_undo->strdup_strings = 1;\n+\t\tresolve_undo = xmalloc(sizeof(*resolve_undo));\n+\t\tstring_list_init(resolve_undo, 1);\n \t\tistate->resolve_undo = resolve_undo;\n \t}\n \tresolve_undo = istate->resolve_undo;\n@@ -56,8 +56,8 @@ struct string_list *resolve_undo_read(const char *data, unsigned long size)\n \tchar *endptr;\n \tint i;\n \n-\tresolve_undo = xcalloc(1, sizeof(*resolve_undo));\n-\tresolve_undo->strdup_strings = 1;\n+\tresolve_undo = xmalloc(sizeof(*resolve_undo));\n+\tstring_list_init(resolve_undo, 1);\n \n \twhile (size) {\n \t\tstruct string_list_item *lost;\ndiff --git a/revision.c b/revision.c\nindex b1c1890..e18522c 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1319,8 +1319,10 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\tstruct strbuf buf = STRBUF_INIT;\n \t\trevs->show_notes = 1;\n \t\trevs->show_notes_given = 1;\n-\t\tif (!revs->notes_opt.extra_notes_refs)\n-\t\t\trevs->notes_opt.extra_notes_refs = xcalloc(1, sizeof(struct string_list));\n+\t\tif (!revs->notes_opt.extra_notes_refs) {\n+\t\t\trevs->notes_opt.extra_notes_refs = xmalloc(sizeof(struct string_list));\n+\t\t\tstring_list_init(revs->notes_opt.extra_notes_refs, 0);\n+\t\t}\n \t\tif (!prefixcmp(arg+13, \"refs/\"))\n \t\t\t/* happy */;\n \t\telse if (!prefixcmp(arg+13, \"notes/\"))\n@@ -1328,6 +1330,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\telse\n \t\t\tstrbuf_addstr(&buf, \"refs/notes/\");\n \t\tstrbuf_addstr(&buf, arg+13);\n+\t\t/* NEEDSWORK: leak. */\n \t\tstring_list_append(revs->notes_opt.extra_notes_refs,\n \t\t\t\t   strbuf_detach(&buf, NULL));\n \t} else if (!strcmp(arg, \"--no-notes\")) {\ndiff --git a/string-list.c b/string-list.c\nindex 8e992a7..9684819 100644\n--- a/string-list.c\n+++ b/string-list.c\n@@ -163,16 +163,29 @@ struct string_list_item *string_list_append(struct string_list *list, const char\n \treturn list->items + list->nr++;\n }\n \n-static int cmp_items(const void *a, const void *b)\n+struct string_list_item *string_list_append_take_ownership(struct string_list *list, char *string)\n+{\n+\tassert(list->strdup_strings);\n+\tALLOC_GROW(list->items, list->nr + 1, list->alloc);\n+\tlist->items[list->nr].string = string;\n+\treturn list->items + list->nr++;\n+}\n+\n+\n+static int cmp_items(const struct string_list_item *one, const struct string_list_item *two)\n {\n-\tconst struct string_list_item *one = a;\n-\tconst struct string_list_item *two = b;\n \treturn strcmp(one->string, two->string);\n }\n \n void sort_string_list(struct string_list *list)\n {\n-\tqsort(list->items, list->nr, sizeof(*list->items), cmp_items);\n+\tsort_string_list_by(list, cmp_items);\n+}\n+\n+void sort_string_list_by(struct string_list *list, string_list_compare_func compare)\n+{\n+\tqsort(list->items, list->nr, sizeof(*list->items),\n+\t\t\t\t(int(*)(const void *, const void *)) compare);\n }\n \n struct string_list_item *unsorted_string_list_lookup(struct string_list *list,\ndiff --git a/string-list.h b/string-list.h\nindex 07e075c..d14f3e0 100644\n--- a/string-list.h\n+++ b/string-list.h\n@@ -42,7 +42,10 @@ struct string_list_item *string_list_lookup(struct string_list *list, const char\n \n /* Use these functions only on unsorted lists: */\n struct string_list_item *string_list_append(struct string_list *list, const char *string);\n+struct string_list_item *string_list_append_take_ownership(struct string_list *list, char *string);\n void sort_string_list(struct string_list *list);\n+typedef int (*string_list_compare_func)(const struct string_list_item *a, const struct string_list_item *b);\n+void sort_string_list_by(struct string_list *list, string_list_compare_func compare);\n int unsorted_string_list_has_string(struct string_list *list, const char *string);\n struct string_list_item *unsorted_string_list_lookup(struct string_list *list,\n \t\t\t\t\t\t     const char *string);\ndiff --git a/submodule.c b/submodule.c\nindex 91a4758..1c7ab8c 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -9,8 +9,8 @@\n #include \"refs.h\"\n #include \"string-list.h\"\n \n-struct string_list config_name_for_path;\n-struct string_list config_ignore_for_name;\n+struct string_list config_name_for_path = STRING_LIST_INIT_NODUP;\n+struct string_list config_ignore_for_name = STRING_LIST_INIT_NODUP;\n \n static int add_submodule_odb(const char *path)\n {\ndiff --git a/wt-status.c b/wt-status.c\nindex 54b6b03..0e137b8 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -44,9 +44,9 @@ void wt_status_prepare(struct wt_status *s)\n \ts->reference = \"HEAD\";\n \ts->fp = stdout;\n \ts->index_file = get_index_file();\n-\ts->change.strdup_strings = 1;\n-\ts->untracked.strdup_strings = 1;\n-\ts->ignored.strdup_strings = 1;\n+\tstring_list_init(&s->change, 1);\n+\tstring_list_init(&s->untracked, 1);\n+\tstring_list_init(&s->ignored, 1);\n }\n \n static void wt_status_print_unmerged_header(struct wt_status *s)\n-- \n1.7.2.3\n"},{"id":"150007","messageId":"AANLkTinP1XNsVCnyL+dnn_+up1Oi6aSxiaA_JjdKDGje@mail.gmail.com","threadId":"24989","inReplyTo":"20100905200323.GA14497@burratino","subject":"Re: [demo/patch 0/3] Re: [PATCH] Documentation: document the string-list macros.","fromName":"Thiago Farina","fromEmail":"tfransosi@gmail.com","sentAt":"2010-09-05T23:19:15Z","receivedAt":"2010-09-05T23:19:15Z","isPatch":true,"sender":{"key":"tfransosi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/970071?v=4"},"body":"On Sun, Sep 5, 2010 at 5:03 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Thiago Farina wrote:\n>\n>> --- a/Documentation/technical/api-string-list.txt\n>> +++ b/Documentation/technical/api-string-list.txt\n>> @@ -52,6 +52,18 @@ However, if you use the list to check if a certain string was added\n>>  already, you should not do that (using unsorted_string_list_has_string()),\n>>  because the complexity would be quadratic again (but with a worse factor).\n>>\n>> +Macros\n>> +------\n>> +\n>> +`STRING_LIST_INIT_NODUP`::\n>> +\n>> +     Initialize the members and set the `strdup_strings` member to 0.\n>> +\n>> +`STRING_LIST_INIT_DUP`::\n>> +\n>> +     Initialize the members and set the `strdup_strings` member to 1.\n>\n> After reading that, one might be tempted to write\n>\n>        struct string_list x;\n>        STRING_LIST_INIT_NODUP(x);\n>\n> , no?  In other words, I don't find the text very clear.\n>\nYeah, you are right.\n\n> If you like working by example (like I do) then api-strbuf.txt might\n> give a good indication of how this sort of thing can be helpfully\n> documented.\n>\nI have looked into it :-)\n\n> Maybe something in this direction?\n>\n> Patch #3 in particular is very rough\nOh yeah :)\n\n>  This is not meant for application, just to give an\n> idea.\n>\nI like the idea. I will improve the Documentation based on or version.\n\nThanks.\n"}]}