[PATCH] string-list.h: Add STRING_LIST_INIT macro and make use of it.

[PATCH] string-list.h: Add STRING_LIST_INIT macro and make use of it.

[Thiago Farina]

Signed-off-by: Thiago Farina <tfransosi@gmail.com>
---
 builtin/fast-export.c  |    2 +-
 builtin/fetch.c        |    8 ++++----
 builtin/receive-pack.c |    2 +-
 builtin/remote.c       |    4 ++--
 builtin/show-ref.c     |    2 +-
 remote.c               |    4 ++--
 string-list.h          |    2 ++
 7 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index c6dd71a..89945bc 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -566,7 +566,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 {
 	struct rev_info revs;
 	struct object_array commits = { 0, 0, NULL };
-	struct string_list extra_refs = { NULL, 0, 0, 0 };
+	struct string_list extra_refs = STRING_LIST_INIT;
 	struct commit *commit;
 	char *export_filename = NULL, *import_filename = NULL;
 	struct option options[] = {
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 5cb369c..a16fb43 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -572,8 +572,8 @@ static void find_non_local_tags(struct transport *transport,
 			struct ref **head,
 			struct ref ***tail)
 {
-	struct string_list existing_refs = { NULL, 0, 0, 0 };
-	struct string_list remote_refs = { NULL, 0, 0, 0 };
+	struct string_list existing_refs = STRING_LIST_INIT;
+	struct string_list remote_refs = STRING_LIST_INIT;
 	struct tag_data data;
 	const struct ref *ref;
 	struct string_list_item *item = NULL;
@@ -667,7 +667,7 @@ static int truncate_fetch_head(void)
 static int do_fetch(struct transport *transport,
 		    struct refspec *refs, int ref_count)
 {
-	struct string_list existing_refs = { NULL, 0, 0, 0 };
+	struct string_list existing_refs = STRING_LIST_INIT;
 	struct string_list_item *peer_item = NULL;
 	struct ref *ref_map;
 	struct ref *rm;
@@ -890,7 +890,7 @@ static int fetch_one(struct remote *remote, int argc, const char **argv)
 int cmd_fetch(int argc, const char **argv, const char *prefix)
 {
 	int i;
-	struct string_list list = { NULL, 0, 0, 0 };
+	struct string_list list = STRING_LIST_INIT;
 	struct remote *remote;
 	int result = 0;
 
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 29bc8d5..c40a4ba 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -530,7 +530,7 @@ static void check_aliased_update(struct command *cmd, struct string_list *list)
 static void check_aliased_updates(struct command *commands)
 {
 	struct command *cmd;
-	struct string_list ref_list = { NULL, 0, 0, 0 };
+	struct string_list ref_list = STRING_LIST_INIT;
 
 	for (cmd = commands; cmd; cmd = cmd->next) {
 		struct string_list_item *item =
diff --git a/builtin/remote.c b/builtin/remote.c
index 0a52667..9d41792 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -596,7 +596,7 @@ static int mv(int argc, const char **argv)
 	};
 	struct remote *oldremote, *newremote;
 	struct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT, buf3 = STRBUF_INIT;
-	struct string_list remote_branches = { NULL, 0, 0, 0 };
+	struct string_list remote_branches = STRING_LIST_INIT;
 	struct rename_info rename;
 	int i;
 
@@ -1044,7 +1044,7 @@ static int show(int argc, const char **argv)
 		OPT_END()
 	};
 	struct ref_states states;
-	struct string_list info_list = { NULL, 0, 0, 0 };
+	struct string_list info_list = STRING_LIST_INIT;
 	struct show_info info;
 
 	argc = parse_options(argc, argv, NULL, options, builtin_remote_show_usage,
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 17ada88..25c280a 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -120,7 +120,7 @@ static int add_existing(const char *refname, const unsigned char *sha1, int flag
  */
 static int exclude_existing(const char *match)
 {
-	static struct string_list existing_refs = { NULL, 0, 0, 0 };
+	static struct string_list existing_refs = STRING_LIST_INIT;
 	char buf[1024];
 	int matchlen = match ? strlen(match) : 0;
 
diff --git a/remote.c b/remote.c
index e51cd22..059d757 100644
--- a/remote.c
+++ b/remote.c
@@ -754,7 +754,7 @@ int for_each_remote(each_remote_fn fn, void *priv)
 
 void ref_remove_duplicates(struct ref *ref_map)
 {
-	struct string_list refs = { NULL, 0, 0, 0 };
+	struct string_list refs = STRING_LIST_INIT;
 	struct string_list_item *item = NULL;
 	struct ref *prev = NULL, *next = NULL;
 	for (; ref_map; prev = ref_map, ref_map = next) {
@@ -1704,7 +1704,7 @@ static int get_stale_heads_cb(const char *refname,
 struct ref *get_stale_heads(struct remote *remote, struct ref *fetch_map)
 {
 	struct ref *ref, *stale_refs = NULL;
-	struct string_list ref_names = { NULL, 0, 0, 0 };
+	struct string_list ref_names = STRING_LIST_INIT;
 	struct stale_heads_info info;
 	info.remote = remote;
 	info.ref_names = &ref_names;
diff --git a/string-list.h b/string-list.h
index 63b69c8..037d058 100644
--- a/string-list.h
+++ b/string-list.h
@@ -12,6 +12,8 @@ struct string_list
 	unsigned int strdup_strings:1;
 };
 
+#define STRING_LIST_INIT { NULL, 0, 0, 0 }
+
 void print_string_list(const char *text, const struct string_list *p);
 void string_list_clear(struct string_list *list, int free_util);
 
-- 
1.7.0.4

Re: [PATCH] string-list.h: Add STRING_LIST_INIT macro and make use of it.

[Sverre Rabbelier]

Heya,

On Sun, Jun 27, 2010 at 00:34, Thiago Farina <tfransosi@gmail.com> wrote:
>        struct object_array commits = { 0, 0, NULL };

Looks like object_array could use the same treatment?

-- 
Cheers,

Sverre Rabbelier

Re: [PATCH] string-list.h: Add STRING_LIST_INIT macro and make use of it.

[Jonathan Nieder]

Thiago Farina wrote:

>  builtin/fast-export.c  |    2 +-
>  builtin/fetch.c        |    8 ++++----
>  builtin/receive-pack.c |    2 +-
>  builtin/remote.c       |    4 ++--
>  builtin/show-ref.c     |    2 +-
>  remote.c               |    4 ++--
>  string-list.h          |    2 ++
>  7 files changed, 13 insertions(+), 11 deletions(-)

 $ git grep -e 'string_list.*=' origin/master | grep { | grep -v 1 |
 > wc -l
 16
 $ git grep -e 'string_list.*=' origin/master | grep { | grep -v 1 |
 > cut -d: -f2 | uniq
 builtin/fast-export.c
 builtin/fetch.c
 builtin/mv.c
 builtin/receive-pack.c
 builtin/remote.c
 builtin/show-ref.c
 merge-recursive.c
 remote.c
 transport-helper.c

Looks like you missed a few.  Still,

Acked-by: Jonathan Nieder <jrnieder@gmail.com>

[PATCH] string-list.h: Add STRING_LIST_INIT macro and make use of it.

[Thiago Farina]

Signed-off-by: Thiago Farina <tfransosi@gmail.com>
---
 builtin/fast-export.c  |    2 +-
 builtin/fetch.c        |    8 ++++----
 builtin/receive-pack.c |    2 +-
 builtin/remote.c       |    4 ++--
 builtin/show-ref.c     |    2 +-
 merge-recursive.c      |    3 ++-
 remote.c               |    4 ++--
 string-list.h          |    2 ++
 8 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index c6dd71a..89945bc 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -566,7 +566,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 {
 	struct rev_info revs;
 	struct object_array commits = { 0, 0, NULL };
-	struct string_list extra_refs = { NULL, 0, 0, 0 };
+	struct string_list extra_refs = STRING_LIST_INIT;
 	struct commit *commit;
 	char *export_filename = NULL, *import_filename = NULL;
 	struct option options[] = {
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 5cb369c..a16fb43 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -572,8 +572,8 @@ static void find_non_local_tags(struct transport *transport,
 			struct ref **head,
 			struct ref ***tail)
 {
-	struct string_list existing_refs = { NULL, 0, 0, 0 };
-	struct string_list remote_refs = { NULL, 0, 0, 0 };
+	struct string_list existing_refs = STRING_LIST_INIT;
+	struct string_list remote_refs = STRING_LIST_INIT;
 	struct tag_data data;
 	const struct ref *ref;
 	struct string_list_item *item = NULL;
@@ -667,7 +667,7 @@ static int truncate_fetch_head(void)
 static int do_fetch(struct transport *transport,
 		    struct refspec *refs, int ref_count)
 {
-	struct string_list existing_refs = { NULL, 0, 0, 0 };
+	struct string_list existing_refs = STRING_LIST_INIT;
 	struct string_list_item *peer_item = NULL;
 	struct ref *ref_map;
 	struct ref *rm;
@@ -890,7 +890,7 @@ static int fetch_one(struct remote *remote, int argc, const char **argv)
 int cmd_fetch(int argc, const char **argv, const char *prefix)
 {
 	int i;
-	struct string_list list = { NULL, 0, 0, 0 };
+	struct string_list list = STRING_LIST_INIT;
 	struct remote *remote;
 	int result = 0;
 
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 29bc8d5..c40a4ba 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -530,7 +530,7 @@ static void check_aliased_update(struct command *cmd, struct string_list *list)
 static void check_aliased_updates(struct command *commands)
 {
 	struct command *cmd;
-	struct string_list ref_list = { NULL, 0, 0, 0 };
+	struct string_list ref_list = STRING_LIST_INIT;
 
 	for (cmd = commands; cmd; cmd = cmd->next) {
 		struct string_list_item *item =
diff --git a/builtin/remote.c b/builtin/remote.c
index 0a52667..9d41792 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -596,7 +596,7 @@ static int mv(int argc, const char **argv)
 	};
 	struct remote *oldremote, *newremote;
 	struct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT, buf3 = STRBUF_INIT;
-	struct string_list remote_branches = { NULL, 0, 0, 0 };
+	struct string_list remote_branches = STRING_LIST_INIT;
 	struct rename_info rename;
 	int i;
 
@@ -1044,7 +1044,7 @@ static int show(int argc, const char **argv)
 		OPT_END()
 	};
 	struct ref_states states;
-	struct string_list info_list = { NULL, 0, 0, 0 };
+	struct string_list info_list = STRING_LIST_INIT;
 	struct show_info info;
 
 	argc = parse_options(argc, argv, NULL, options, builtin_remote_show_usage,
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 17ada88..25c280a 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -120,7 +120,7 @@ static int add_existing(const char *refname, const unsigned char *sha1, int flag
  */
 static int exclude_existing(const char *match)
 {
-	static struct string_list existing_refs = { NULL, 0, 0, 0 };
+	static struct string_list existing_refs = STRING_LIST_INIT;
 	char buf[1024];
 	int matchlen = match ? strlen(match) : 0;
 
diff --git a/merge-recursive.c b/merge-recursive.c
index 206c103..e9a5513 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -806,7 +806,8 @@ static int process_renames(struct merge_options *o,
 			   struct string_list *b_renames)
 {
 	int clean_merge = 1, i, j;
-	struct string_list a_by_dst = {NULL, 0, 0, 0}, b_by_dst = {NULL, 0, 0, 0};
+	struct string_list a_by_dst = STRING_LIST_INIT;
+        struct string_list b_by_dst = STRING_LIST_INIT;
 	const struct rename *sre;
 
 	for (i = 0; i < a_renames->nr; i++) {
diff --git a/remote.c b/remote.c
index e51cd22..059d757 100644
--- a/remote.c
+++ b/remote.c
@@ -754,7 +754,7 @@ int for_each_remote(each_remote_fn fn, void *priv)
 
 void ref_remove_duplicates(struct ref *ref_map)
 {
-	struct string_list refs = { NULL, 0, 0, 0 };
+	struct string_list refs = STRING_LIST_INIT;
 	struct string_list_item *item = NULL;
 	struct ref *prev = NULL, *next = NULL;
 	for (; ref_map; prev = ref_map, ref_map = next) {
@@ -1704,7 +1704,7 @@ static int get_stale_heads_cb(const char *refname,
 struct ref *get_stale_heads(struct remote *remote, struct ref *fetch_map)
 {
 	struct ref *ref, *stale_refs = NULL;
-	struct string_list ref_names = { NULL, 0, 0, 0 };
+	struct string_list ref_names = STRING_LIST_INIT;
 	struct stale_heads_info info;
 	info.remote = remote;
 	info.ref_names = &ref_names;
diff --git a/string-list.h b/string-list.h
index 63b69c8..037d058 100644
--- a/string-list.h
+++ b/string-list.h
@@ -12,6 +12,8 @@ struct string_list
 	unsigned int strdup_strings:1;
 };
 
+#define STRING_LIST_INIT { NULL, 0, 0, 0 }
+
 void print_string_list(const char *text, const struct string_list *p);
 void string_list_clear(struct string_list *list, int free_util);
 
-- 
1.7.1.576.gef57c

Re: [PATCH] string-list.h: Add STRING_LIST_INIT macro and make use of it.

[Thiago Farina]

On Sat, Jun 26, 2010 at 8:04 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Thiago Farina wrote:
>
>>  builtin/fast-export.c  |    2 +-
>>  builtin/fetch.c        |    8 ++++----
>>  builtin/receive-pack.c |    2 +-
>>  builtin/remote.c       |    4 ++--
>>  builtin/show-ref.c     |    2 +-
>>  remote.c               |    4 ++--
>>  string-list.h          |    2 ++
>>  7 files changed, 13 insertions(+), 11 deletions(-)
>
>  $ git grep -e 'string_list.*=' origin/master | grep { | grep -v 1 |
>  > wc -l
>  16
>  $ git grep -e 'string_list.*=' origin/master | grep { | grep -v 1 |
>  > cut -d: -f2 | uniq
>  builtin/fast-export.c
>  builtin/fetch.c
>  builtin/mv.c
>  builtin/receive-pack.c
>  builtin/remote.c
>  builtin/show-ref.c
>  merge-recursive.c
>  remote.c
>  transport-helper.c
>
> Looks like you missed a few.  Still,
>
> Acked-by: Jonathan Nieder <jrnieder@gmail.com>

Thanks Jonathan. I think the other entries you found have different
initializations.

[PATCH] string-list.h: Add STRING_LIST_INIT macro and make use of it.

[Thiago Farina]

Acked-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Thiago Farina <tfransosi@gmail.com>
---
 builtin/fast-export.c  |    2 +-
 builtin/fetch.c        |    8 ++++----
 builtin/receive-pack.c |    2 +-
 builtin/remote.c       |    4 ++--
 builtin/show-ref.c     |    2 +-
 merge-recursive.c      |    3 ++-
 remote.c               |    4 ++--
 string-list.h          |    2 ++
 8 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index c6dd71a..89945bc 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -566,7 +566,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 {
 	struct rev_info revs;
 	struct object_array commits = { 0, 0, NULL };
-	struct string_list extra_refs = { NULL, 0, 0, 0 };
+	struct string_list extra_refs = STRING_LIST_INIT;
 	struct commit *commit;
 	char *export_filename = NULL, *import_filename = NULL;
 	struct option options[] = {
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 5cb369c..a16fb43 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -572,8 +572,8 @@ static void find_non_local_tags(struct transport *transport,
 			struct ref **head,
 			struct ref ***tail)
 {
-	struct string_list existing_refs = { NULL, 0, 0, 0 };
-	struct string_list remote_refs = { NULL, 0, 0, 0 };
+	struct string_list existing_refs = STRING_LIST_INIT;
+	struct string_list remote_refs = STRING_LIST_INIT;
 	struct tag_data data;
 	const struct ref *ref;
 	struct string_list_item *item = NULL;
@@ -667,7 +667,7 @@ static int truncate_fetch_head(void)
 static int do_fetch(struct transport *transport,
 		    struct refspec *refs, int ref_count)
 {
-	struct string_list existing_refs = { NULL, 0, 0, 0 };
+	struct string_list existing_refs = STRING_LIST_INIT;
 	struct string_list_item *peer_item = NULL;
 	struct ref *ref_map;
 	struct ref *rm;
@@ -890,7 +890,7 @@ static int fetch_one(struct remote *remote, int argc, const char **argv)
 int cmd_fetch(int argc, const char **argv, const char *prefix)
 {
 	int i;
-	struct string_list list = { NULL, 0, 0, 0 };
+	struct string_list list = STRING_LIST_INIT;
 	struct remote *remote;
 	int result = 0;
 
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 29bc8d5..c40a4ba 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -530,7 +530,7 @@ static void check_aliased_update(struct command *cmd, struct string_list *list)
 static void check_aliased_updates(struct command *commands)
 {
 	struct command *cmd;
-	struct string_list ref_list = { NULL, 0, 0, 0 };
+	struct string_list ref_list = STRING_LIST_INIT;
 
 	for (cmd = commands; cmd; cmd = cmd->next) {
 		struct string_list_item *item =
diff --git a/builtin/remote.c b/builtin/remote.c
index 0a52667..9d41792 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -596,7 +596,7 @@ static int mv(int argc, const char **argv)
 	};
 	struct remote *oldremote, *newremote;
 	struct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT, buf3 = STRBUF_INIT;
-	struct string_list remote_branches = { NULL, 0, 0, 0 };
+	struct string_list remote_branches = STRING_LIST_INIT;
 	struct rename_info rename;
 	int i;
 
@@ -1044,7 +1044,7 @@ static int show(int argc, const char **argv)
 		OPT_END()
 	};
 	struct ref_states states;
-	struct string_list info_list = { NULL, 0, 0, 0 };
+	struct string_list info_list = STRING_LIST_INIT;
 	struct show_info info;
 
 	argc = parse_options(argc, argv, NULL, options, builtin_remote_show_usage,
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 17ada88..25c280a 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -120,7 +120,7 @@ static int add_existing(const char *refname, const unsigned char *sha1, int flag
  */
 static int exclude_existing(const char *match)
 {
-	static struct string_list existing_refs = { NULL, 0, 0, 0 };
+	static struct string_list existing_refs = STRING_LIST_INIT;
 	char buf[1024];
 	int matchlen = match ? strlen(match) : 0;
 
diff --git a/merge-recursive.c b/merge-recursive.c
index 206c103..e9a5513 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -806,7 +806,8 @@ static int process_renames(struct merge_options *o,
 			   struct string_list *b_renames)
 {
 	int clean_merge = 1, i, j;
-	struct string_list a_by_dst = {NULL, 0, 0, 0}, b_by_dst = {NULL, 0, 0, 0};
+	struct string_list a_by_dst = STRING_LIST_INIT;
+        struct string_list b_by_dst = STRING_LIST_INIT;
 	const struct rename *sre;
 
 	for (i = 0; i < a_renames->nr; i++) {
diff --git a/remote.c b/remote.c
index e51cd22..059d757 100644
--- a/remote.c
+++ b/remote.c
@@ -754,7 +754,7 @@ int for_each_remote(each_remote_fn fn, void *priv)
 
 void ref_remove_duplicates(struct ref *ref_map)
 {
-	struct string_list refs = { NULL, 0, 0, 0 };
+	struct string_list refs = STRING_LIST_INIT;
 	struct string_list_item *item = NULL;
 	struct ref *prev = NULL, *next = NULL;
 	for (; ref_map; prev = ref_map, ref_map = next) {
@@ -1704,7 +1704,7 @@ static int get_stale_heads_cb(const char *refname,
 struct ref *get_stale_heads(struct remote *remote, struct ref *fetch_map)
 {
 	struct ref *ref, *stale_refs = NULL;
-	struct string_list ref_names = { NULL, 0, 0, 0 };
+	struct string_list ref_names = STRING_LIST_INIT;
 	struct stale_heads_info info;
 	info.remote = remote;
 	info.ref_names = &ref_names;
diff --git a/string-list.h b/string-list.h
index 63b69c8..037d058 100644
--- a/string-list.h
+++ b/string-list.h
@@ -12,6 +12,8 @@ struct string_list
 	unsigned int strdup_strings:1;
 };
 
+#define STRING_LIST_INIT { NULL, 0, 0, 0 }
+
 void print_string_list(const char *text, const struct string_list *p);
 void string_list_clear(struct string_list *list, int free_util);
 
-- 
1.7.1.576.gef57c

Re: [PATCH] string-list.h: Add STRING_LIST_INIT macro and make use of it.

[Junio C Hamano]

I think you need the attached on top to be more complete.

But I wonder if this is worth it (not this "more complete", but your
patch).  We convert 16 or so instances of initialization for "no strdup"
list, but there are about the same number of "strdup" instances still
spelled out:

    $ git grep -e 'struct string_list [^ ]* = {.*'
    $ git grep -e 'struct string_list [^ ]* = {.*1'

Wouldn't it be more sensible to use this instead?

    #define STRING_LIST_INIT(pleasedup) { NULL, 0, 0, (pleasedup) }

---
 builtin/mv.c       |    2 +-
 builtin/remote.c   |    4 ++--
 transport-helper.c |    2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/builtin/mv.c b/builtin/mv.c
index c07f53b..e2f0279 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -63,7 +63,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 	const char **source, **destination, **dest_path;
 	enum update_mode { BOTH = 0, WORKING_DIRECTORY, INDEX } *modes;
 	struct stat st;
-	struct string_list src_for_dst = {NULL, 0, 0, 0};
+	struct string_list src_for_dst = STRING_LIST_INIT;
 
 	git_config(git_default_config, NULL);
 
diff --git a/builtin/remote.c b/builtin/remote.c
index 9d41792..5ff2b5b 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -134,7 +134,7 @@ static int add_branch(const char *key, const char *branchname,
 static int add(int argc, const char **argv)
 {
 	int fetch = 0, mirror = 0, fetch_tags = TAGS_DEFAULT;
-	struct string_list track = { NULL, 0, 0 };
+	struct string_list track = STRING_LIST_INIT;
 	const char *master = NULL;
 	struct remote *remote;
 	struct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT;
@@ -1483,7 +1483,7 @@ static int get_one_entry(struct remote *remote, void *priv)
 
 static int show_all(void)
 {
-	struct string_list list = { NULL, 0, 0 };
+	struct string_list list = STRING_LIST_INIT;
 	int result;
 
 	list.strdup_strings = 1;
diff --git a/transport-helper.c b/transport-helper.c
index 0381de5..86a79b9 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -689,7 +689,7 @@ static int push_refs_with_export(struct transport *transport,
 	struct child_process *helper, exporter;
 	struct helper_data *data = transport->data;
 	char *export_marks = NULL, *import_marks = NULL;
-	struct string_list revlist_args = { NULL, 0, 0 };
+	struct string_list revlist_args = STRING_LIST_INIT;
 	struct strbuf buf = STRBUF_INIT;
 
 	helper = get_helper(transport);

Re: [PATCH] string-list.h: Add STRING_LIST_INIT macro and make use of it.

[Alex Riesen]

On Sun, Jun 27, 2010 at 18:44, Junio C Hamano <gitster@pobox.com> wrote:
> I think you need the attached on top to be more complete.
>
> But I wonder if this is worth it (not this "more complete", but your
> patch).  We convert 16 or so instances of initialization for "no strdup"
> list, but there are about the same number of "strdup" instances still
> spelled out:
>
>    $ git grep -e 'struct string_list [^ ]* = {.*'
>    $ git grep -e 'struct string_list [^ ]* = {.*1'
>
> Wouldn't it be more sensible to use this instead?
>
>    #define STRING_LIST_INIT(pleasedup) { NULL, 0, 0, (pleasedup) }
>

This begs for using strdup(string-to-dup) in the macro argument, which
will not compile with ancient compilers which don't allow code in an
initializer.

Re: [PATCH] string-list.h: Add STRING_LIST_INIT macro and make use of it.

[Junio C Hamano]

Alex Riesen <raa.lkml@gmail.com> writes:

>> But I wonder if this is worth it (not this "more complete", but your
>> patch). We convert 16 or so instances of initialization for "no strdup"
>> list, but there are about the same number of "strdup" instances still
>> spelled out:
>>
>>    $ git grep -e 'struct string_list [^ ]* = {.*'
>>    $ git grep -e 'struct string_list [^ ]* = {.*1'
>>
>> Wouldn't it be more sensible to use this instead?
>>
>>    #define STRING_LIST_INIT(pleasedup) { NULL, 0, 0, (pleasedup) }
>
> This begs for using strdup(string-to-dup) in the macro argument, which
> will not compile with ancient compilers which don't allow code in an
> initializer.

Err, one of us must be confused.

I was suggesting to rewrite things like these, found in builtin/remote.c
(add and rm):

	struct string_list track = { NULL, 0, 0, 0 };
	struct string_list branches = { NULL, 0, 0, 1 };

like so:

	#define STRING_LIST_INIT(pleasedup) { NULL, 0, 0, (pleasedup) }
	struct string_list track = STRING_LIST_INIT(0);
        struct string_list branches = STRING_LIST_INIT(1);

I don't see strdup in macro argument anywhere.

The original patch in this thread proposed to do this instead:

	#define STRING_LIST_INIT { NULL, 0, 0, 0 }
	struct string_list track = STRING_LIST_INIT;
        struct string_list branches = { NULL, 0, 0, 1 };

converting about half of initializers to the macro, and leaving the other
half spelled out.

RE: [PATCH] string-list.h: Add STRING_LIST_INIT macro and make use of it.

[Peter Kjellerstedt]

> -----Original Message-----
> From: git-owner@vger.kernel.org [mailto:git-owner@vger.kernel.org] On
> Behalf Of Junio C Hamano
> Sent: den 29 juni 2010 07:43
> To: Alex Riesen
> Cc: Thiago Farina; git@vger.kernel.org; jrnieder@gmail.com;
> srabbelier@gmail.com
> Subject: Re: [PATCH] string-list.h: Add STRING_LIST_INIT macro and make
> use of it.
> 
> Alex Riesen <raa.lkml@gmail.com> writes:
> 
> >> But I wonder if this is worth it (not this "more complete", but 
> >> your patch). We convert 16 or so instances of initialization for
> >> "no strdup" list, but there are about the same number of "strdup" 
> >> instances still spelled out:
> >>
> >>    $ git grep -e 'struct string_list [^ ]* = {.*'
> >>    $ git grep -e 'struct string_list [^ ]* = {.*1'
> >>
> >> Wouldn't it be more sensible to use this instead?
> >>
> >>    #define STRING_LIST_INIT(pleasedup) { NULL, 0, 0, (pleasedup) }
> >
> > This begs for using strdup(string-to-dup) in the macro argument,
> > which will not compile with ancient compilers which don't allow 
> > code in an initializer.
> 
> Err, one of us must be confused.
> 
> I was suggesting to rewrite things like these, found in
> builtin/remote.c (add and rm):
> 
> 	struct string_list track = { NULL, 0, 0, 0 };
> 	struct string_list branches = { NULL, 0, 0, 1 };
> 
> like so:
> 
> 	#define STRING_LIST_INIT(pleasedup) { NULL, 0, 0, (pleasedup) }
> 	struct string_list track = STRING_LIST_INIT(0);
>         struct string_list branches = STRING_LIST_INIT(1);
> 
> I don't see strdup in macro argument anywhere.
> 
> The original patch in this thread proposed to do this instead:
> 
> 	#define STRING_LIST_INIT { NULL, 0, 0, 0 }
> 	struct string_list track = STRING_LIST_INIT;
>         struct string_list branches = { NULL, 0, 0, 1 };
> 
> converting about half of initializers to the macro, and leaving the
> other half spelled out.

How about using two macros instead:

#define STRING_LIST_INIT { NULL, 0, 0, 0 }
#define STRING_LIST_INIT_DUP { NULL, 0, 0, 1 }

That way there is less confusion to what the 0/1 argument
to STRING_LIST_INIT() means. (I guess STRING_LIST_INIT could 
be STRING_LIST_INIT_NODUP instead if one wants more clarity.)

//Peter

Re: [PATCH] string-list.h: Add STRING_LIST_INIT macro and make use of it.

[Alex Riesen]

On Tue, Jun 29, 2010 at 07:43, Junio C Hamano <gitster@pobox.com> wrote:
>>>
>>>    #define STRING_LIST_INIT(pleasedup) { NULL, 0, 0, (pleasedup) }
>>
>> This begs for using strdup(string-to-dup) in the macro argument, which
>> will not compile with ancient compilers which don't allow code in an
>> initializer.
>
> Err, one of us must be confused.
>
> I was suggesting to rewrite things like these, found in builtin/remote.c
> (add and rm):
>
>        struct string_list track = { NULL, 0, 0, 0 };
>        struct string_list branches = { NULL, 0, 0, 1 };
>

That must be me, again. Didn't bother to check the string_list definition
and naively assumed the pleasedup was for an initialization string. I
thought that depending on usage the caller may wish to either copy it
or leave the string_list referncing the original. Which cannot be the case
with string_list, indeed.

BTW, now that I took a look at it... The iteration over string_list
items looks a little overengineered. At least from the point of
view of the existing users of the feature. Wouldn't a simple loop
be just as simple to use (if not simplier) and faster (no uninlineable
function calls and argument preparation and passing needed)?

#define string_list_foreach(item,list) \
	for (item = (list)->items; item < (list)->items + (list)->nr; ++item)

[PATCH] string_list: Add STRING_LIST_INIT macro and make use of it.

[Thiago Farina]

Acked-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Thiago Farina <tfransosi@gmail.com>
---
 builtin/fast-export.c   |    2 +-
 builtin/fetch.c         |    8 ++++----
 builtin/fmt-merge-msg.c |    6 +++---
 builtin/grep.c          |    2 +-
 builtin/mailsplit.c     |    2 +-
 builtin/mv.c            |    2 +-
 builtin/receive-pack.c  |    2 +-
 builtin/remote.c        |   12 ++++++------
 builtin/rerere.c        |    4 ++--
 builtin/show-ref.c      |    2 +-
 diff-no-index.c         |    3 ++-
 merge-recursive.c       |    3 ++-
 remote.c                |    4 ++--
 rerere.c                |   10 +++++-----
 string-list.h           |    2 ++
 transport-helper.c      |    2 +-
 16 files changed, 35 insertions(+), 31 deletions(-)

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 9fe25ff..44d538f 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -566,7 +566,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 {
 	struct rev_info revs;
 	struct object_array commits = { 0, 0, NULL };
-	struct string_list extra_refs = { NULL, 0, 0, 0 };
+	struct string_list extra_refs = STRING_LIST_INIT(0);
 	struct commit *commit;
 	char *export_filename = NULL, *import_filename = NULL;
 	struct option options[] = {
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 6eb1dfe..b0b2526 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -572,8 +572,8 @@ static void find_non_local_tags(struct transport *transport,
 			struct ref **head,
 			struct ref ***tail)
 {
-	struct string_list existing_refs = { NULL, 0, 0, 0 };
-	struct string_list remote_refs = { NULL, 0, 0, 0 };
+	struct string_list existing_refs = STRING_LIST_INIT(0);
+	struct string_list remote_refs = STRING_LIST_INIT(0);
 	struct tag_data data;
 	const struct ref *ref;
 	struct string_list_item *item = NULL;
@@ -667,7 +667,7 @@ static int truncate_fetch_head(void)
 static int do_fetch(struct transport *transport,
 		    struct refspec *refs, int ref_count)
 {
-	struct string_list existing_refs = { NULL, 0, 0, 0 };
+	struct string_list existing_refs = STRING_LIST_INIT(0);
 	struct string_list_item *peer_item = NULL;
 	struct ref *ref_map;
 	struct ref *rm;
@@ -890,7 +890,7 @@ static int fetch_one(struct remote *remote, int argc, const char **argv)
 int cmd_fetch(int argc, const char **argv, const char *prefix)
 {
 	int i;
-	struct string_list list = { NULL, 0, 0, 0 };
+	struct string_list list = STRING_LIST_INIT(0);
 	struct remote *remote;
 	int result = 0;
 
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index bc3c5e6..7133f9f 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -38,8 +38,8 @@ void init_src_data(struct src_data *data)
 	data->generic.strdup_strings = 1;
 }
 
-static struct string_list srcs = { NULL, 0, 0, 1 };
-static struct string_list origins = { NULL, 0, 0, 1 };
+static struct string_list srcs = STRING_LIST_INIT(1);
+static struct string_list origins = STRING_LIST_INIT(1);
 
 static int handle_line(char *line)
 {
@@ -146,7 +146,7 @@ static void shortlog(const char *name, unsigned char *sha1,
 	int i, count = 0;
 	struct commit *commit;
 	struct object *branch;
-	struct string_list subjects = { NULL, 0, 0, 1 };
+	struct string_list subjects = STRING_LIST_INIT(1);
 	int flags = UNINTERESTING | TREESAME | SEEN | SHOWN | ADDED;
 	struct strbuf sb = STRBUF_INIT;
 
diff --git a/builtin/grep.c b/builtin/grep.c
index 232cd1c..9b01145 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -836,7 +836,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	struct grep_opt opt;
 	struct object_array list = { 0, 0, NULL };
 	const char **paths = NULL;
-	struct string_list path_list = { NULL, 0, 0, 0 };
+	struct string_list path_list = STRING_LIST_INIT(0);
 	int i;
 	int dummy;
 	int nongit = 0, use_index = 1;
diff --git a/builtin/mailsplit.c b/builtin/mailsplit.c
index e4560da..c95e17a 100644
--- a/builtin/mailsplit.c
+++ b/builtin/mailsplit.c
@@ -137,7 +137,7 @@ static int split_maildir(const char *maildir, const char *dir,
 	char name[PATH_MAX];
 	int ret = -1;
 	int i;
-	struct string_list list = {NULL, 0, 0, 1};
+	struct string_list list = STRING_LIST_INIT(1);
 
 	if (populate_maildir_list(&list, maildir) < 0)
 		goto out;
diff --git a/builtin/mv.c b/builtin/mv.c
index 38574b8..c7832dd 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -63,7 +63,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 	const char **source, **destination, **dest_path;
 	enum update_mode { BOTH = 0, WORKING_DIRECTORY, INDEX } *modes;
 	struct stat st;
-	struct string_list src_for_dst = {NULL, 0, 0, 0};
+	struct string_list src_for_dst = STRING_LIST_INIT(0);
 
 	git_config(git_default_config, NULL);
 
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index d634b5a..190157a 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -530,7 +530,7 @@ static void check_aliased_update(struct command *cmd, struct string_list *list)
 static void check_aliased_updates(struct command *commands)
 {
 	struct command *cmd;
-	struct string_list ref_list = { NULL, 0, 0, 0 };
+	struct string_list ref_list = STRING_LIST_INIT(0);
 
 	for (cmd = commands; cmd; cmd = cmd->next) {
 		struct string_list_item *item =
diff --git a/builtin/remote.c b/builtin/remote.c
index 6699bc5..e2c2102 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -134,7 +134,7 @@ static int add_branch(const char *key, const char *branchname,
 static int add(int argc, const char **argv)
 {
 	int fetch = 0, mirror = 0, fetch_tags = TAGS_DEFAULT;
-	struct string_list track = { NULL, 0, 0 };
+	struct string_list track = STRING_LIST_INIT(0);
 	const char *master = NULL;
 	struct remote *remote;
 	struct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT;
@@ -596,7 +596,7 @@ static int mv(int argc, const char **argv)
 	};
 	struct remote *oldremote, *newremote;
 	struct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT, buf3 = STRBUF_INIT;
-	struct string_list remote_branches = { NULL, 0, 0, 0 };
+	struct string_list remote_branches = STRING_LIST_INIT(0);
 	struct rename_info rename;
 	int i;
 
@@ -734,8 +734,8 @@ static int rm(int argc, const char **argv)
 	struct remote *remote;
 	struct strbuf buf = STRBUF_INIT;
 	struct known_remotes known_remotes = { NULL, NULL };
-	struct string_list branches = { NULL, 0, 0, 1 };
-	struct string_list skipped = { NULL, 0, 0, 1 };
+	struct string_list branches = STRING_LIST_INIT(1);
+	struct string_list skipped = STRING_LIST_INIT(1);
 	struct branches_for_remote cb_data;
 	int i, result;
 
@@ -1044,7 +1044,7 @@ static int show(int argc, const char **argv)
 		OPT_END()
 	};
 	struct ref_states states;
-	struct string_list info_list = { NULL, 0, 0, 0 };
+	struct string_list info_list = STRING_LIST_INIT(0);
 	struct show_info info;
 
 	argc = parse_options(argc, argv, NULL, options, builtin_remote_show_usage,
@@ -1483,7 +1483,7 @@ static int get_one_entry(struct remote *remote, void *priv)
 
 static int show_all(void)
 {
-	struct string_list list = { NULL, 0, 0 };
+	struct string_list list = STRING_LIST_INIT(0);
 	int result;
 
 	list.strdup_strings = 1;
diff --git a/builtin/rerere.c b/builtin/rerere.c
index 980d542..9995388 100644
--- a/builtin/rerere.c
+++ b/builtin/rerere.c
@@ -40,7 +40,7 @@ static int git_rerere_gc_config(const char *var, const char *value, void *cb)
 
 static void garbage_collect(struct string_list *rr)
 {
-	struct string_list to_remove = { NULL, 0, 0, 1 };
+	struct string_list to_remove = STRING_LIST_INIT(1);
 	DIR *dir;
 	struct dirent *e;
 	int i, cutoff;
@@ -102,7 +102,7 @@ static int diff_two(const char *file1, const char *label1,
 
 int cmd_rerere(int argc, const char **argv, const char *prefix)
 {
-	struct string_list merge_rr = { NULL, 0, 0, 1 };
+	struct string_list merge_rr = STRING_LIST_INIT(1);
 	int i, fd, flags = 0;
 
 	if (2 < argc) {
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 0b2a9ad..41b7eb5 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -120,7 +120,7 @@ static int add_existing(const char *refname, const unsigned char *sha1, int flag
  */
 static int exclude_existing(const char *match)
 {
-	static struct string_list existing_refs = { NULL, 0, 0, 0 };
+	static struct string_list existing_refs = STRING_LIST_INIT(0);
 	char buf[1024];
 	int matchlen = match ? strlen(match) : 0;
 
diff --git a/diff-no-index.c b/diff-no-index.c
index 43aeeba..2d09e37 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -64,7 +64,8 @@ static int queue_diff(struct diff_options *o,
 
 	if (S_ISDIR(mode1) || S_ISDIR(mode2)) {
 		char buffer1[PATH_MAX], buffer2[PATH_MAX];
-		struct string_list p1 = {NULL, 0, 0, 1}, p2 = {NULL, 0, 0, 1};
+		struct string_list p1 = STRING_LIST_INIT(1);
+                struct string_list p2 = STRING_LIST_INIT(1);
 		int len1 = 0, len2 = 0, i1, i2, ret = 0;
 
 		if (name1 && read_directory(name1, &p1))
diff --git a/merge-recursive.c b/merge-recursive.c
index 856e98c..6311b53 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -806,7 +806,8 @@ static int process_renames(struct merge_options *o,
 			   struct string_list *b_renames)
 {
 	int clean_merge = 1, i, j;
-	struct string_list a_by_dst = {NULL, 0, 0, 0}, b_by_dst = {NULL, 0, 0, 0};
+	struct string_list a_by_dst = STRING_LIST_INIT(0);
+        struct string_list b_by_dst = STRING_LIST_INIT(0);
 	const struct rename *sre;
 
 	for (i = 0; i < a_renames->nr; i++) {
diff --git a/remote.c b/remote.c
index afbba47..9f3dc52 100644
--- a/remote.c
+++ b/remote.c
@@ -754,7 +754,7 @@ int for_each_remote(each_remote_fn fn, void *priv)
 
 void ref_remove_duplicates(struct ref *ref_map)
 {
-	struct string_list refs = { NULL, 0, 0, 0 };
+	struct string_list refs = STRING_LIST_INIT(0);
 	struct string_list_item *item = NULL;
 	struct ref *prev = NULL, *next = NULL;
 	for (; ref_map; prev = ref_map, ref_map = next) {
@@ -1704,7 +1704,7 @@ static int get_stale_heads_cb(const char *refname,
 struct ref *get_stale_heads(struct remote *remote, struct ref *fetch_map)
 {
 	struct ref *ref, *stale_refs = NULL;
-	struct string_list ref_names = { NULL, 0, 0, 0 };
+	struct string_list ref_names = STRING_LIST_INIT(0);
 	struct stale_heads_info info;
 	info.remote = remote;
 	info.ref_names = &ref_names;
diff --git a/rerere.c b/rerere.c
index d03a696..1424dbb 100644
--- a/rerere.c
+++ b/rerere.c
@@ -426,8 +426,8 @@ static int update_paths(struct string_list *update)
 
 static int do_plain_rerere(struct string_list *rr, int fd)
 {
-	struct string_list conflict = { NULL, 0, 0, 1 };
-	struct string_list update = { NULL, 0, 0, 1 };
+	struct string_list conflict = STRING_LIST_INIT(1);
+	struct string_list update = STRING_LIST_INIT(1);
 	int i;
 
 	find_conflict(&conflict);
@@ -547,7 +547,7 @@ int setup_rerere(struct string_list *merge_rr, int flags)
 
 int rerere(int flags)
 {
-	struct string_list merge_rr = { NULL, 0, 0, 1 };
+	struct string_list merge_rr = STRING_LIST_INIT(1);
 	int fd;
 
 	fd = setup_rerere(&merge_rr, flags);
@@ -585,8 +585,8 @@ static int rerere_forget_one_path(const char *path, struct string_list *rr)
 int rerere_forget(const char **pathspec)
 {
 	int i, fd;
-	struct string_list conflict = { NULL, 0, 0, 1 };
-	struct string_list merge_rr = { NULL, 0, 0, 1 };
+	struct string_list conflict = STRING_LIST_INIT(1);
+	struct string_list merge_rr = STRING_LIST_INIT(1);
 
 	if (read_cache() < 0)
 		return error("Could not read index");
diff --git a/string-list.h b/string-list.h
index 680d600..21969d3 100644
--- a/string-list.h
+++ b/string-list.h
@@ -12,6 +12,8 @@ struct string_list
 	unsigned int strdup_strings:1;
 };
 
+#define STRING_LIST_INIT(dup_string) { NULL, 0, 0, (dup_string) }
+
 void print_string_list(const struct string_list *p, const char *text);
 void string_list_clear(struct string_list *list, int free_util);
 
diff --git a/transport-helper.c b/transport-helper.c
index 191fbf7..054576b 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -689,7 +689,7 @@ static int push_refs_with_export(struct transport *transport,
 	struct child_process *helper, exporter;
 	struct helper_data *data = transport->data;
 	char *export_marks = NULL, *import_marks = NULL;
-	struct string_list revlist_args = { NULL, 0, 0 };
+	struct string_list revlist_args = STRING_LIST_INIT(0);
 	struct strbuf buf = STRBUF_INIT;
 
 	helper = get_helper(transport);
-- 
1.7.1.576.gafae7.dirty

Re: [PATCH] string_list: Add STRING_LIST_INIT macro and make use of it.

[Jonathan Nieder]

Thiago Farina wrote:

> Acked-by: Jonathan Nieder <jrnieder@gmail.com>

Um, I am not so fond of this one.

The argument to STRING_LIST_INIT is hard to interpret without
familiarity.  What did you think of Peter’s suggestion to use an a
different macro for the strdup_strings case?  Something like
STRING_LIST_INIT_DUP_ON_APPEND, but perhaps shorter.

It would also be nice to update
Documentation/technical/api-string-list.txt with whatever you choose. 

That said, I do like the goal of making it easier to reorganize
struct string_list later if needed.

Re: [PATCH] string_list: Add STRING_LIST_INIT macro and make use of it.

[Thiago Farina]

On Fri, Jul 2, 2010 at 4:30 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Thiago Farina wrote:
>
>> Acked-by: Jonathan Nieder <jrnieder@gmail.com>
>
> Um, I am not so fond of this one.
>
> The argument to STRING_LIST_INIT is hard to interpret without
> familiarity.  What did you think of Peter’s suggestion to use an a
> different macro for the strdup_strings case?  Something like
> STRING_LIST_INIT_DUP_ON_APPEND, but perhaps shorter.
>
If we are fine, would be the following?

#define STRING_LIST_INIT_NODUP { NULL, 0, 0, 0 }
#define STRING_LIST_INIT_DUP { NULL, 0, 0, 1 }

> It would also be nice to update
> Documentation/technical/api-string-list.txt with whatever you choose.
>
Sure, I will update it.

> That said, I do like the goal of making it easier to reorganize
> struct string_list later if needed.
>

Re: [PATCH] string_list: Add STRING_LIST_INIT macro and make use of it.

[Jonathan Nieder]

Thiago Farina wrote:

> #define STRING_LIST_INIT_NODUP { NULL, 0, 0, 0 }
> #define STRING_LIST_INIT_DUP { NULL, 0, 0, 1 }

That looks fine to me.  Thanks, and sorry to nitpick.

[PATCH] string_list: Add STRING_LIST_INIT macro and make use of it.

[Thiago Farina]

Acked-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Thiago Farina <tfransosi@gmail.com>
---
 builtin/fast-export.c   |    2 +-
 builtin/fetch.c         |    8 ++++----
 builtin/fmt-merge-msg.c |    6 +++---
 builtin/grep.c          |    2 +-
 builtin/mailsplit.c     |    2 +-
 builtin/mv.c            |    2 +-
 builtin/receive-pack.c  |    2 +-
 builtin/remote.c        |   12 ++++++------
 builtin/rerere.c        |    4 ++--
 builtin/show-ref.c      |    2 +-
 diff-no-index.c         |    3 ++-
 merge-recursive.c       |    3 ++-
 remote.c                |    4 ++--
 rerere.c                |   10 +++++-----
 string-list.h           |    3 +++
 transport-helper.c      |    2 +-
 16 files changed, 36 insertions(+), 31 deletions(-)

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 9fe25ff..834ec8b 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -566,7 +566,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 {
 	struct rev_info revs;
 	struct object_array commits = { 0, 0, NULL };
-	struct string_list extra_refs = { NULL, 0, 0, 0 };
+	struct string_list extra_refs = STRING_LIST_INIT_NODUP;
 	struct commit *commit;
 	char *export_filename = NULL, *import_filename = NULL;
 	struct option options[] = {
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 6eb1dfe..9f40511 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -572,8 +572,8 @@ static void find_non_local_tags(struct transport *transport,
 			struct ref **head,
 			struct ref ***tail)
 {
-	struct string_list existing_refs = { NULL, 0, 0, 0 };
-	struct string_list remote_refs = { NULL, 0, 0, 0 };
+	struct string_list existing_refs = STRING_LIST_INIT_NODUP;
+	struct string_list remote_refs = STRING_LIST_INIT_NODUP;
 	struct tag_data data;
 	const struct ref *ref;
 	struct string_list_item *item = NULL;
@@ -667,7 +667,7 @@ static int truncate_fetch_head(void)
 static int do_fetch(struct transport *transport,
 		    struct refspec *refs, int ref_count)
 {
-	struct string_list existing_refs = { NULL, 0, 0, 0 };
+	struct string_list existing_refs = STRING_LIST_INIT_NODUP;
 	struct string_list_item *peer_item = NULL;
 	struct ref *ref_map;
 	struct ref *rm;
@@ -890,7 +890,7 @@ static int fetch_one(struct remote *remote, int argc, const char **argv)
 int cmd_fetch(int argc, const char **argv, const char *prefix)
 {
 	int i;
-	struct string_list list = { NULL, 0, 0, 0 };
+	struct string_list list = STRING_LIST_INIT_NODUP;
 	struct remote *remote;
 	int result = 0;
 
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index bc3c5e6..a76cd4e 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -38,8 +38,8 @@ void init_src_data(struct src_data *data)
 	data->generic.strdup_strings = 1;
 }
 
-static struct string_list srcs = { NULL, 0, 0, 1 };
-static struct string_list origins = { NULL, 0, 0, 1 };
+static struct string_list srcs = STRING_LIST_INIT_DUP;
+static struct string_list origins = STRING_LIST_INIT_DUP;
 
 static int handle_line(char *line)
 {
@@ -146,7 +146,7 @@ static void shortlog(const char *name, unsigned char *sha1,
 	int i, count = 0;
 	struct commit *commit;
 	struct object *branch;
-	struct string_list subjects = { NULL, 0, 0, 1 };
+	struct string_list subjects = STRING_LIST_INIT_DUP;
 	int flags = UNINTERESTING | TREESAME | SEEN | SHOWN | ADDED;
 	struct strbuf sb = STRBUF_INIT;
 
diff --git a/builtin/grep.c b/builtin/grep.c
index 232cd1c..103f64a 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -836,7 +836,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	struct grep_opt opt;
 	struct object_array list = { 0, 0, NULL };
 	const char **paths = NULL;
-	struct string_list path_list = { NULL, 0, 0, 0 };
+	struct string_list path_list = STRING_LIST_INIT_NODUP;
 	int i;
 	int dummy;
 	int nongit = 0, use_index = 1;
diff --git a/builtin/mailsplit.c b/builtin/mailsplit.c
index e4560da..99654d0 100644
--- a/builtin/mailsplit.c
+++ b/builtin/mailsplit.c
@@ -137,7 +137,7 @@ static int split_maildir(const char *maildir, const char *dir,
 	char name[PATH_MAX];
 	int ret = -1;
 	int i;
-	struct string_list list = {NULL, 0, 0, 1};
+	struct string_list list = STRING_LIST_INIT_DUP;
 
 	if (populate_maildir_list(&list, maildir) < 0)
 		goto out;
diff --git a/builtin/mv.c b/builtin/mv.c
index 38574b8..cdbb094 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -63,7 +63,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 	const char **source, **destination, **dest_path;
 	enum update_mode { BOTH = 0, WORKING_DIRECTORY, INDEX } *modes;
 	struct stat st;
-	struct string_list src_for_dst = {NULL, 0, 0, 0};
+	struct string_list src_for_dst = STRING_LIST_INIT_NODUP;
 
 	git_config(git_default_config, NULL);
 
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index d634b5a..760817d 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -530,7 +530,7 @@ static void check_aliased_update(struct command *cmd, struct string_list *list)
 static void check_aliased_updates(struct command *commands)
 {
 	struct command *cmd;
-	struct string_list ref_list = { NULL, 0, 0, 0 };
+	struct string_list ref_list = STRING_LIST_INIT_NODUP;
 
 	for (cmd = commands; cmd; cmd = cmd->next) {
 		struct string_list_item *item =
diff --git a/builtin/remote.c b/builtin/remote.c
index 6699bc5..48e0a6b 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -134,7 +134,7 @@ static int add_branch(const char *key, const char *branchname,
 static int add(int argc, const char **argv)
 {
 	int fetch = 0, mirror = 0, fetch_tags = TAGS_DEFAULT;
-	struct string_list track = { NULL, 0, 0 };
+	struct string_list track = STRING_LIST_INIT_NODUP;
 	const char *master = NULL;
 	struct remote *remote;
 	struct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT;
@@ -596,7 +596,7 @@ static int mv(int argc, const char **argv)
 	};
 	struct remote *oldremote, *newremote;
 	struct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT, buf3 = STRBUF_INIT;
-	struct string_list remote_branches = { NULL, 0, 0, 0 };
+	struct string_list remote_branches = STRING_LIST_INIT_NODUP;
 	struct rename_info rename;
 	int i;
 
@@ -734,8 +734,8 @@ static int rm(int argc, const char **argv)
 	struct remote *remote;
 	struct strbuf buf = STRBUF_INIT;
 	struct known_remotes known_remotes = { NULL, NULL };
-	struct string_list branches = { NULL, 0, 0, 1 };
-	struct string_list skipped = { NULL, 0, 0, 1 };
+	struct string_list branches = STRING_LIST_INIT_DUP;
+	struct string_list skipped = STRING_LIST_INIT_DUP;
 	struct branches_for_remote cb_data;
 	int i, result;
 
@@ -1044,7 +1044,7 @@ static int show(int argc, const char **argv)
 		OPT_END()
 	};
 	struct ref_states states;
-	struct string_list info_list = { NULL, 0, 0, 0 };
+	struct string_list info_list = STRING_LIST_INIT_NODUP;
 	struct show_info info;
 
 	argc = parse_options(argc, argv, NULL, options, builtin_remote_show_usage,
@@ -1483,7 +1483,7 @@ static int get_one_entry(struct remote *remote, void *priv)
 
 static int show_all(void)
 {
-	struct string_list list = { NULL, 0, 0 };
+	struct string_list list = STRING_LIST_INIT_NODUP;
 	int result;
 
 	list.strdup_strings = 1;
diff --git a/builtin/rerere.c b/builtin/rerere.c
index 980d542..3c8e458 100644
--- a/builtin/rerere.c
+++ b/builtin/rerere.c
@@ -40,7 +40,7 @@ static int git_rerere_gc_config(const char *var, const char *value, void *cb)
 
 static void garbage_collect(struct string_list *rr)
 {
-	struct string_list to_remove = { NULL, 0, 0, 1 };
+	struct string_list to_remove = STRING_LIST_INIT_DUP;
 	DIR *dir;
 	struct dirent *e;
 	int i, cutoff;
@@ -102,7 +102,7 @@ static int diff_two(const char *file1, const char *label1,
 
 int cmd_rerere(int argc, const char **argv, const char *prefix)
 {
-	struct string_list merge_rr = { NULL, 0, 0, 1 };
+	struct string_list merge_rr = STRING_LIST_INIT_DUP;
 	int i, fd, flags = 0;
 
 	if (2 < argc) {
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 0b2a9ad..be9b512 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -120,7 +120,7 @@ static int add_existing(const char *refname, const unsigned char *sha1, int flag
  */
 static int exclude_existing(const char *match)
 {
-	static struct string_list existing_refs = { NULL, 0, 0, 0 };
+	static struct string_list existing_refs = STRING_LIST_INIT_NODUP;
 	char buf[1024];
 	int matchlen = match ? strlen(match) : 0;
 
diff --git a/diff-no-index.c b/diff-no-index.c
index 43aeeba..a0ad7ab 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -64,7 +64,8 @@ static int queue_diff(struct diff_options *o,
 
 	if (S_ISDIR(mode1) || S_ISDIR(mode2)) {
 		char buffer1[PATH_MAX], buffer2[PATH_MAX];
-		struct string_list p1 = {NULL, 0, 0, 1}, p2 = {NULL, 0, 0, 1};
+		struct string_list p1 = STRING_LIST_INIT_DUP;
+                struct string_list p2 = STRING_LIST_INIT_DUP;
 		int len1 = 0, len2 = 0, i1, i2, ret = 0;
 
 		if (name1 && read_directory(name1, &p1))
diff --git a/merge-recursive.c b/merge-recursive.c
index 856e98c..85eb3f0 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -806,7 +806,8 @@ static int process_renames(struct merge_options *o,
 			   struct string_list *b_renames)
 {
 	int clean_merge = 1, i, j;
-	struct string_list a_by_dst = {NULL, 0, 0, 0}, b_by_dst = {NULL, 0, 0, 0};
+	struct string_list a_by_dst = STRING_LIST_INIT_NODUP;
+        struct string_list b_by_dst = STRING_LIST_INIT_NODUP;
 	const struct rename *sre;
 
 	for (i = 0; i < a_renames->nr; i++) {
diff --git a/remote.c b/remote.c
index afbba47..9143ec7 100644
--- a/remote.c
+++ b/remote.c
@@ -754,7 +754,7 @@ int for_each_remote(each_remote_fn fn, void *priv)
 
 void ref_remove_duplicates(struct ref *ref_map)
 {
-	struct string_list refs = { NULL, 0, 0, 0 };
+	struct string_list refs = STRING_LIST_INIT_NODUP;
 	struct string_list_item *item = NULL;
 	struct ref *prev = NULL, *next = NULL;
 	for (; ref_map; prev = ref_map, ref_map = next) {
@@ -1704,7 +1704,7 @@ static int get_stale_heads_cb(const char *refname,
 struct ref *get_stale_heads(struct remote *remote, struct ref *fetch_map)
 {
 	struct ref *ref, *stale_refs = NULL;
-	struct string_list ref_names = { NULL, 0, 0, 0 };
+	struct string_list ref_names = STRING_LIST_INIT_NODUP;
 	struct stale_heads_info info;
 	info.remote = remote;
 	info.ref_names = &ref_names;
diff --git a/rerere.c b/rerere.c
index d03a696..78bbcf1 100644
--- a/rerere.c
+++ b/rerere.c
@@ -426,8 +426,8 @@ static int update_paths(struct string_list *update)
 
 static int do_plain_rerere(struct string_list *rr, int fd)
 {
-	struct string_list conflict = { NULL, 0, 0, 1 };
-	struct string_list update = { NULL, 0, 0, 1 };
+	struct string_list conflict = STRING_LIST_INIT_DUP;
+	struct string_list update = STRING_LIST_INIT_DUP;
 	int i;
 
 	find_conflict(&conflict);
@@ -547,7 +547,7 @@ int setup_rerere(struct string_list *merge_rr, int flags)
 
 int rerere(int flags)
 {
-	struct string_list merge_rr = { NULL, 0, 0, 1 };
+	struct string_list merge_rr = STRING_LIST_INIT_DUP;
 	int fd;
 
 	fd = setup_rerere(&merge_rr, flags);
@@ -585,8 +585,8 @@ static int rerere_forget_one_path(const char *path, struct string_list *rr)
 int rerere_forget(const char **pathspec)
 {
 	int i, fd;
-	struct string_list conflict = { NULL, 0, 0, 1 };
-	struct string_list merge_rr = { NULL, 0, 0, 1 };
+	struct string_list conflict = STRING_LIST_INIT_DUP;
+	struct string_list merge_rr = STRING_LIST_INIT_DUP;
 
 	if (read_cache() < 0)
 		return error("Could not read index");
diff --git a/string-list.h b/string-list.h
index 680d600..84cd09d 100644
--- a/string-list.h
+++ b/string-list.h
@@ -12,6 +12,9 @@ struct string_list
 	unsigned int strdup_strings:1;
 };
 
+#define STRING_LIST_INIT_NODUP { NULL, 0, 0, 0 }
+#define STRING_LIST_INIT_DUP   { NULL, 0, 0, 1 }
+
 void print_string_list(const struct string_list *p, const char *text);
 void string_list_clear(struct string_list *list, int free_util);
 
diff --git a/transport-helper.c b/transport-helper.c
index 191fbf7..acfc88e 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -689,7 +689,7 @@ static int push_refs_with_export(struct transport *transport,
 	struct child_process *helper, exporter;
 	struct helper_data *data = transport->data;
 	char *export_marks = NULL, *import_marks = NULL;
-	struct string_list revlist_args = { NULL, 0, 0 };
+	struct string_list revlist_args = STRING_LIST_INIT_NODUP;
 	struct strbuf buf = STRBUF_INIT;
 
 	helper = get_helper(transport);
-- 
1.7.1.576.gafae7.dirty

Re: [PATCH] string_list: Add STRING_LIST_INIT macro and make use of it.

[Jonathan Nieder]

Hi Thiago,

Thiago Farina wrote:

> I see that no one commentted in the patch, but I can't see it on
> http://git.kernel.org/?p=git/git.git;a=summary, so I'm not sure if it
> was accepted.
> 
> What is the easiest way to verify if the patch was committed?

Good question.  When no one comments on a patch, that is usually
a bad sign.  In this case, I think the patch is good and I was too
lazy to comment on it (sorry).

| $ git log --oneline --author=Thiago origin/pu
| 183113a string_list: Add STRING_LIST_INIT macro and make use of it.
| [...]
| $ git branch tf/string-list-init 183113a
| $ git branch -r --contains tf/string-list-init
| origin/pu
| $

So it has been queued in "pu" but not graduated to "next" yet.

Side note.  From the description of the "pu" branch in
origin/todo:MaintNotes

	The "pu" branch, and topic branches that are only in
	"pu", are subject to rebasing in general.  By the above
	definition of how "next" works, you can tell that this
	branch will contain quite experimental and obviously
	broken stuff.

it might sound like Junio thinks your patch is obviously broken!
I suspect that description of "pu" is intended for testers
(meaning: try this branch only if you found "next" too boring and
like writing bug reports) rather than patch authors[1].

Especially during a release candidate period (which this is),
promising new features are often queued to "pu" and only merged to
"next" when Junio has had time to look them over again.

HTH,
Jonathan

[1] Maybe it could be clarified: how about inserting "sometimes"
before "contain"?

	                           ... you can tell that this
	branch will sometimes contain quite experimental and
	obviously broken stuff.

Re: [PATCH] string_list: Add STRING_LIST_INIT macro and make use of it.

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> [1] Maybe it could be clarified: how about inserting "sometimes"
> before "contain"?

Sure, or "will also contain".

Re: [PATCH] string_list: Add STRING_LIST_INIT macro and make use of it.

[Thiago Farina]

Hi Jonathan,

On Wed, Jul 7, 2010 at 12:19 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Hi Thiago,
>
> Thiago Farina wrote:
>
>> I see that no one commentted in the patch, but I can't see it on
>> http://git.kernel.org/?p=git/git.git;a=summary, so I'm not sure if it
>> was accepted.
>>
>> What is the easiest way to verify if the patch was committed?
>
> Good question.  When no one comments on a patch, that is usually
> a bad sign.  In this case, I think the patch is good and I was too
> lazy to comment on it (sorry).
>
> | $ git log --oneline --author=Thiago origin/pu
> | 183113a string_list: Add STRING_LIST_INIT macro and make use of it.
> | [...]
> | $ git branch tf/string-list-init 183113a
How did you figure out the part "tf/string-list-init"? The first step
doesn't seem to give any hint about that.

> | $ git branch -r --contains tf/string-list-init
> | origin/pu
Now it shows origin/master, origin/next and origin/pu, so I assume
it's all these three branches.

> | $
>
> So it has been queued in "pu" but not graduated to "next" yet.
>
> Side note.  From the description of the "pu" branch in
> origin/todo:MaintNotes
>
>        The "pu" branch, and topic branches that are only in
>        "pu", are subject to rebasing in general.  By the above
>        definition of how "next" works, you can tell that this
>        branch will contain quite experimental and obviously
>        broken stuff.
>
> it might sound like Junio thinks your patch is obviously broken!
> I suspect that description of "pu" is intended for testers
> (meaning: try this branch only if you found "next" too boring and
> like writing bug reports) rather than patch authors[1].
>
> Especially during a release candidate period (which this is),
> promising new features are often queued to "pu" and only merged to
> "next" when Junio has had time to look them over again.
>
> HTH,
> Jonathan
>
> [1] Maybe it could be clarified: how about inserting "sometimes"
> before "contain"?
>
>                                   ... you can tell that this
>        branch will sometimes contain quite experimental and
>        obviously broken stuff.
>

Re: [PATCH] string_list: Add STRING_LIST_INIT macro and make use of it.

[Jonathan Nieder]

Thiago Farina wrote:
> On Wed, Jul 7, 2010 at 12:19 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:

>> | $ git log --oneline --author=Thiago origin/pu
>> | 183113a string_list: Add STRING_LIST_INIT macro and make use of it.
>> | [...]
>> | $ git branch tf/string-list-init 183113a
>
> How did you figure out the part "tf/string-list-init"?

I made it up.  Actually, I used

  git log --ancestry-path 183113a..origin/pu

for some unrelated purpose and remembered the name Junio used, but the
name is completely unimportant; my local branch can be named
tf/string-list-macro and it would still work.  Or I can use no local
branch at all, like this:

  git branch -r --contains 183113a

I am guessing you tried the equivalent of

  git branch -r --contains 0afcb5f7

which asks git a different question.

Re: [PATCH] string_list: Add STRING_LIST_INIT macro and make use of it.

[Thiago Farina]

On Fri, Jul 2, 2010 at 5:22 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Thiago Farina wrote:
>
>> #define STRING_LIST_INIT_NODUP { NULL, 0, 0, 0 }
>> #define STRING_LIST_INIT_DUP { NULL, 0, 0, 1 }
>
> That looks fine to me.  Thanks, and sorry to nitpick.
>
Why that was never merged to the master branch?

<fc60bb94eb4a4b54a0b3f0c075ca2e7141be06f5.1278272263.git.tfransosi@gmail.com>