[PATCH 0/6] Non-trivial designated initializer conversion

[PATCH 0/6] Non-trivial designated initializer conversion

[Ævar Arnfjörð Bjarmason]

A sister series to the just-submitted series of trivial designated
initalizer cleanups:
https://lore.kernel.org/git/cover-0.5-00000000000-20210927T003330Z-avarab@gmail.com/

This doesn't conflict semantically or textually with that series, but
is migrating to easier to manage initialization patterns for the same
reasons.

The code changes are all rather small, the diff in 6/6 looks scary,
but as --word-diff will show most of it schanging x->y to an &x->y and
the like, as two struct members were changed from being pointers.

Ævar Arnfjörð Bjarmason (6):
  daemon.c: refactor hostinfo_init() to HOSTINFO_INIT macro
  builtin/blame.c: refactor commit_info_init() to COMMIT_INFO_INIT macro
  shortlog: use designated initializer for "struct shortlog"
  urlmatch.[ch]: add and use URLMATCH_CONFIG_INIT
  builtin/remote.c: add and use a REF_STATES_INIT
  builtin/remote.c: add and use SHOW_INFO_INIT

 builtin/blame.c    |  30 ++++++------
 builtin/config.c   |   3 +-
 builtin/remote.c   | 111 ++++++++++++++++++++++-----------------------
 builtin/shortlog.c |   2 +-
 credential.c       |   4 +-
 daemon.c           |  19 +++-----
 http.c             |   4 +-
 shortlog.h         |   4 ++
 urlmatch.h         |   4 ++
 9 files changed, 87 insertions(+), 94 deletions(-)

-- 
2.33.0.1316.gb2e9b3ba3ae

[PATCH 1/6] daemon.c: refactor hostinfo_init() to HOSTINFO_INIT macro

[Ævar Arnfjörð Bjarmason]

Remove the hostinfo_init() function added in 01cec54e135 (daemon:
deglobalize hostname information, 2015-03-07) and instead initialize
the "struct hostinfo" with a macro.

This is the more idiomatic pattern in the codebase, and doesn't leave
us wondering when we see the *_init() function if this struct needs
more complex initialization than a macro can provide.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 daemon.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/daemon.c b/daemon.c
index 5c4cbad62d0..d80d009d1a1 100644
--- a/daemon.c
+++ b/daemon.c
@@ -63,6 +63,12 @@ struct hostinfo {
 	unsigned int hostname_lookup_done:1;
 	unsigned int saw_extended_args:1;
 };
+#define HOSTINFO_INIT { \
+	.hostname = STRBUF_INIT, \
+	.canon_hostname = STRBUF_INIT, \
+	.ip_address = STRBUF_INIT, \
+	.tcp_port = STRBUF_INIT, \
+}
 
 static void lookup_hostname(struct hostinfo *hi);
 
@@ -727,15 +733,6 @@ static void lookup_hostname(struct hostinfo *hi)
 	}
 }
 
-static void hostinfo_init(struct hostinfo *hi)
-{
-	memset(hi, 0, sizeof(*hi));
-	strbuf_init(&hi->hostname, 0);
-	strbuf_init(&hi->canon_hostname, 0);
-	strbuf_init(&hi->ip_address, 0);
-	strbuf_init(&hi->tcp_port, 0);
-}
-
 static void hostinfo_clear(struct hostinfo *hi)
 {
 	strbuf_release(&hi->hostname);
@@ -760,11 +757,9 @@ static int execute(void)
 	char *line = packet_buffer;
 	int pktlen, len, i;
 	char *addr = getenv("REMOTE_ADDR"), *port = getenv("REMOTE_PORT");
-	struct hostinfo hi;
+	struct hostinfo hi = HOSTINFO_INIT;
 	struct strvec env = STRVEC_INIT;
 
-	hostinfo_init(&hi);
-
 	if (addr)
 		loginfo("Connection from %s:%s", addr, port);
 
-- 
2.33.0.1316.gb2e9b3ba3ae

[PATCH 2/6] builtin/blame.c: refactor commit_info_init() to COMMIT_INFO_INIT macro

[Ævar Arnfjörð Bjarmason]

Remove the commit_info_init() function addded in ea02ffa3857 (mailmap:
simplify map_user() interface, 2013-01-05) and instead initialize the
"struct commit_info" with a macro.

This is the more idiomatic pattern in the codebase, and doesn't leave
us wondering when we see the *_init() function if this struct needs
more complex initialization than a macro can provide.

The get_commit_info() function is only called by the three callers
being changed here immediately after initializing the struct with the
macros, so by moving the initialization to the callers we don't need
to do it in get_commit_info() anymore.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/blame.c | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 641523ff9af..1c31a996403 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -101,6 +101,16 @@ struct commit_info {
 	struct strbuf summary;
 };
 
+#define COMMIT_INFO_INIT { \
+	.author = STRBUF_INIT, \
+	.author_mail = STRBUF_INIT, \
+	.author_tz = STRBUF_INIT, \
+	.committer = STRBUF_INIT, \
+	.committer_mail = STRBUF_INIT, \
+	.committer_tz = STRBUF_INIT, \
+	.summary = STRBUF_INIT, \
+}
+
 /*
  * Parse author/committer line in the commit object buffer
  */
@@ -160,18 +170,6 @@ static void get_ac_line(const char *inbuf, const char *what,
 	strbuf_add(name, namebuf, namelen);
 }
 
-static void commit_info_init(struct commit_info *ci)
-{
-
-	strbuf_init(&ci->author, 0);
-	strbuf_init(&ci->author_mail, 0);
-	strbuf_init(&ci->author_tz, 0);
-	strbuf_init(&ci->committer, 0);
-	strbuf_init(&ci->committer_mail, 0);
-	strbuf_init(&ci->committer_tz, 0);
-	strbuf_init(&ci->summary, 0);
-}
-
 static void commit_info_destroy(struct commit_info *ci)
 {
 
@@ -192,8 +190,6 @@ static void get_commit_info(struct commit *commit,
 	const char *subject, *encoding;
 	const char *message;
 
-	commit_info_init(ret);
-
 	encoding = get_log_output_encoding();
 	message = logmsg_reencode(commit, NULL, encoding);
 	get_ac_line(message, "\nauthor ",
@@ -246,7 +242,7 @@ static void write_filename_info(struct blame_origin *suspect)
  */
 static int emit_one_suspect_detail(struct blame_origin *suspect, int repeat)
 {
-	struct commit_info ci;
+	struct commit_info ci = COMMIT_INFO_INIT;
 
 	if (!repeat && (suspect->commit->object.flags & METAINFO_SHOWN))
 		return 0;
@@ -440,7 +436,7 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int
 	int cnt;
 	const char *cp;
 	struct blame_origin *suspect = ent->suspect;
-	struct commit_info ci;
+	struct commit_info ci = COMMIT_INFO_INIT;
 	char hex[GIT_MAX_HEXSZ + 1];
 	int show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP);
 	const char *default_color = NULL, *color = NULL, *reset = NULL;
@@ -630,7 +626,7 @@ static void find_alignment(struct blame_scoreboard *sb, int *option)
 		if (longest_file < num)
 			longest_file = num;
 		if (!(suspect->commit->object.flags & METAINFO_SHOWN)) {
-			struct commit_info ci;
+			struct commit_info ci = COMMIT_INFO_INIT;
 			suspect->commit->object.flags |= METAINFO_SHOWN;
 			get_commit_info(suspect->commit, &ci, 1);
 			if (*option & OUTPUT_SHOW_EMAIL)
-- 
2.33.0.1316.gb2e9b3ba3ae

[PATCH 3/6] shortlog: use designated initializer for "struct shortlog"

[Ævar Arnfjörð Bjarmason]

Change code added in 64093fc06a (blame,shortlog: don't make local
option variables static, 2016-06-13) to use a designated initializer
via a typical *_INIT macro pattern.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/shortlog.c | 2 +-
 shortlog.h         | 4 ++++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index 3e7ab1ca821..fa1f76cc51e 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -342,7 +342,7 @@ void shortlog_init(struct shortlog *log)
 
 int cmd_shortlog(int argc, const char **argv, const char *prefix)
 {
-	struct shortlog log = { STRING_LIST_INIT_NODUP };
+	struct shortlog log = SHORTLOG_INIT;
 	struct rev_info rev;
 	int nongit = !startup_info->have_repository;
 
diff --git a/shortlog.h b/shortlog.h
index 3f7e9aabcae..47892d6d604 100644
--- a/shortlog.h
+++ b/shortlog.h
@@ -28,6 +28,10 @@ struct shortlog {
 	FILE *file;
 };
 
+#define SHORTLOG_INIT { \
+	.list = STRING_LIST_INIT_NODUP, \
+}
+
 void shortlog_init(struct shortlog *log);
 
 void shortlog_add_commit(struct shortlog *log, struct commit *commit);
-- 
2.33.0.1316.gb2e9b3ba3ae

Re: [PATCH 3/6] shortlog: use designated initializer for "struct shortlog"

[Phillip Wood]

Hi Ævar

On 27/09/2021 01:53, Ævar Arnfjörð Bjarmason wrote:
> Change code added in 64093fc06a (blame,shortlog: don't make local
> option variables static, 2016-06-13) to use a designated initializer
> via a typical *_INIT macro pattern.
> 
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>   builtin/shortlog.c | 2 +-
>   shortlog.h         | 4 ++++
>   2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/builtin/shortlog.c b/builtin/shortlog.c
> index 3e7ab1ca821..fa1f76cc51e 100644
> --- a/builtin/shortlog.c
> +++ b/builtin/shortlog.c
> @@ -342,7 +342,7 @@ void shortlog_init(struct shortlog *log)
>   
>   int cmd_shortlog(int argc, const char **argv, const char *prefix)
>   {
> -	struct shortlog log = { STRING_LIST_INIT_NODUP };
> +	struct shortlog log = SHORTLOG_INIT;
>   	struct rev_info rev;
>   	int nongit = !startup_info->have_repository;
>   
> diff --git a/shortlog.h b/shortlog.h
> index 3f7e9aabcae..47892d6d604 100644
> --- a/shortlog.h
> +++ b/shortlog.h
> @@ -28,6 +28,10 @@ struct shortlog {
>   	FILE *file;
>   };
>   
> +#define SHORTLOG_INIT { \
> +	.list = STRING_LIST_INIT_NODUP, \
> +}
> +
>   void shortlog_init(struct shortlog *log);

looking at this wouldn't it be better follow the pattern in the first 
patch in this series and replace shortlog_init() with a designated 
initializer?

Best Wishes

Phillip

>   void shortlog_add_commit(struct shortlog *log, struct commit *commit);
> 

Re: [PATCH 3/6] shortlog: use designated initializer for "struct shortlog"

[Ævar Arnfjörð Bjarmason]

On Mon, Sep 27 2021, Phillip Wood wrote:

> Hi Ævar
>
> On 27/09/2021 01:53, Ævar Arnfjörð Bjarmason wrote:
>> Change code added in 64093fc06a (blame,shortlog: don't make local
>> option variables static, 2016-06-13) to use a designated initializer
>> via a typical *_INIT macro pattern.
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> ---
>>   builtin/shortlog.c | 2 +-
>>   shortlog.h         | 4 ++++
>>   2 files changed, 5 insertions(+), 1 deletion(-)
>> diff --git a/builtin/shortlog.c b/builtin/shortlog.c
>> index 3e7ab1ca821..fa1f76cc51e 100644
>> --- a/builtin/shortlog.c
>> +++ b/builtin/shortlog.c
>> @@ -342,7 +342,7 @@ void shortlog_init(struct shortlog *log)
>>     int cmd_shortlog(int argc, const char **argv, const char
>> *prefix)
>>   {
>> -	struct shortlog log = { STRING_LIST_INIT_NODUP };
>> +	struct shortlog log = SHORTLOG_INIT;
>>   	struct rev_info rev;
>>   	int nongit = !startup_info->have_repository;
>>   diff --git a/shortlog.h b/shortlog.h
>> index 3f7e9aabcae..47892d6d604 100644
>> --- a/shortlog.h
>> +++ b/shortlog.h
>> @@ -28,6 +28,10 @@ struct shortlog {
>>   	FILE *file;
>>   };
>>   +#define SHORTLOG_INIT { \
>> +	.list = STRING_LIST_INIT_NODUP, \
>> +}
>> +
>>   void shortlog_init(struct shortlog *log);
>
> looking at this wouldn't it be better follow the pattern in the first
> patch in this series and replace shortlog_init() with a designated 
> initializer?

I've ejected this patch from a WIP re-roll, I don't know what I was
thinking when I included it.

It's "correct", but one of those cases where we can't easily get rid of
the not-a-macro init() function, so having a *_INIT macro that doesn't
actually do the initialization for both users is just confusing.

Worse, that "nodup" is actually a like, the shortlog_init() function
sets it to "dup", that issue pre-dated by change here, but I shouldn't
have carried it forward.

Sorry!

[PATCH 4/6] urlmatch.[ch]: add and use URLMATCH_CONFIG_INIT

[Ævar Arnfjörð Bjarmason]

Change the initialization pattern of "struct urlmatch_config" to use
an *_INIT macro and designated initializers.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/config.c | 3 +--
 credential.c     | 4 +---
 http.c           | 4 +---
 urlmatch.h       | 4 ++++
 4 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 865fddd6ce8..1ea4f68b7de 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -575,11 +575,10 @@ static int get_urlmatch(const char *var, const char *url)
 	int ret;
 	char *section_tail;
 	struct string_list_item *item;
-	struct urlmatch_config config = { STRING_LIST_INIT_DUP };
+	struct urlmatch_config config = URLMATCH_CONFIG_INIT;
 	struct string_list values = STRING_LIST_INIT_DUP;
 
 	config.collect_fn = urlmatch_collect_fn;
-	config.cascade_fn = NULL;
 	config.cb = &values;
 
 	if (!url_normalize(url, &config.url))
diff --git a/credential.c b/credential.c
index 000ac7a8d43..c85db8a75cd 100644
--- a/credential.c
+++ b/credential.c
@@ -105,7 +105,7 @@ static int match_partial_url(const char *url, void *cb)
 static void credential_apply_config(struct credential *c)
 {
 	char *normalized_url;
-	struct urlmatch_config config = { STRING_LIST_INIT_DUP };
+	struct urlmatch_config config = URLMATCH_CONFIG_INIT;
 	struct strbuf url = STRBUF_INIT;
 
 	if (!c->host)
@@ -117,9 +117,7 @@ static void credential_apply_config(struct credential *c)
 		return;
 
 	config.section = "credential";
-	config.key = NULL;
 	config.collect_fn = credential_config_callback;
-	config.cascade_fn = NULL;
 	config.select_fn = select_all;
 	config.fallback_match_fn = match_partial_url;
 	config.cb = c;
diff --git a/http.c b/http.c
index d7c20493d7f..3d6ad5c7be8 100644
--- a/http.c
+++ b/http.c
@@ -990,13 +990,11 @@ void http_init(struct remote *remote, const char *url, int proactive_auth)
 	char *low_speed_limit;
 	char *low_speed_time;
 	char *normalized_url;
-	struct urlmatch_config config = { STRING_LIST_INIT_DUP };
+	struct urlmatch_config config = URLMATCH_CONFIG_INIT;
 
 	config.section = "http";
-	config.key = NULL;
 	config.collect_fn = http_options;
 	config.cascade_fn = git_default_config;
-	config.cb = NULL;
 
 	http_is_verbose = 0;
 	normalized_url = url_normalize(url, &config.url);
diff --git a/urlmatch.h b/urlmatch.h
index 6ff42f81b0c..34a3ba6d197 100644
--- a/urlmatch.h
+++ b/urlmatch.h
@@ -66,6 +66,10 @@ struct urlmatch_config {
 	int (*fallback_match_fn)(const char *url, void *cb);
 };
 
+#define URLMATCH_CONFIG_INIT { \
+	.vars = STRING_LIST_INIT_DUP, \
+}
+
 int urlmatch_config_entry(const char *var, const char *value, void *cb);
 
 #endif /* URL_MATCH_H */
-- 
2.33.0.1316.gb2e9b3ba3ae

[PATCH 5/6] builtin/remote.c: add and use a REF_STATES_INIT

[Ævar Arnfjörð Bjarmason]

Use a new REF_STATES_INIT designated initializer instead of assigning
to the "strdup_strings" member of the previously memzero()'d version
of this struct.

The pattern of assigning to "strdup_strings" dates back to
211c89682ee (Make git-remote a builtin, 2008-02-29) (when it was
"strdup_paths"), i.e. long before we used anything like our current
established *_INIT patterns consistently.

Then in e61e0cc6b70 (builtin-remote: teach show to display remote
HEAD, 2009-02-25) and e5dcbfd9ab7 (builtin-remote: new show output
style for push refspecs, 2009-02-25) we added some more of these.

As it turns out we only initialized this struct three times, all the
other uses were of pointers to those initialized structs. So let's
initialize it in those three places, skip the memset(), and pass those
structs down appropriately.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/remote.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/builtin/remote.c b/builtin/remote.c
index 7f88e6ce9de..160dd954f74 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -344,6 +344,14 @@ struct ref_states {
 	int queried;
 };
 
+#define REF_STATES_INIT { \
+	.new_refs = STRING_LIST_INIT_DUP, \
+	.stale = STRING_LIST_INIT_DUP, \
+	.tracked = STRING_LIST_INIT_DUP, \
+	.heads = STRING_LIST_INIT_DUP, \
+	.push = STRING_LIST_INIT_DUP, \
+}
+
 static int get_ref_states(const struct ref *remote_refs, struct ref_states *states)
 {
 	struct ref *fetch_map = NULL, **tail = &fetch_map;
@@ -355,9 +363,6 @@ static int get_ref_states(const struct ref *remote_refs, struct ref_states *stat
 			die(_("Could not get fetch map for refspec %s"),
 				states->remote->fetch.raw[i]);
 
-	states->new_refs.strdup_strings = 1;
-	states->tracked.strdup_strings = 1;
-	states->stale.strdup_strings = 1;
 	for (ref = fetch_map; ref; ref = ref->next) {
 		if (!ref->peer_ref || !ref_exists(ref->peer_ref->name))
 			string_list_append(&states->new_refs, abbrev_branch(ref->name));
@@ -406,7 +411,6 @@ static int get_push_ref_states(const struct ref *remote_refs,
 
 	match_push_refs(local_refs, &push_map, &remote->push, MATCH_REFS_NONE);
 
-	states->push.strdup_strings = 1;
 	for (ref = push_map; ref; ref = ref->next) {
 		struct string_list_item *item;
 		struct push_info *info;
@@ -449,7 +453,6 @@ static int get_push_ref_states_noquery(struct ref_states *states)
 	if (remote->mirror)
 		return 0;
 
-	states->push.strdup_strings = 1;
 	if (!remote->push.nr) {
 		item = string_list_append(&states->push, _("(matching)"));
 		info = item->util = xcalloc(1, sizeof(struct push_info));
@@ -483,7 +486,6 @@ static int get_head_names(const struct ref *remote_refs, struct ref_states *stat
 	refspec.force = 0;
 	refspec.pattern = 1;
 	refspec.src = refspec.dst = "refs/heads/*";
-	states->heads.strdup_strings = 1;
 	get_fetch_map(remote_refs, &refspec, &fetch_map_tail, 0);
 	matches = guess_remote_head(find_ref_by_name(remote_refs, "HEAD"),
 				    fetch_map, 1);
@@ -1212,7 +1214,7 @@ static int show(int argc, const char **argv)
 		OPT_BOOL('n', NULL, &no_query, N_("do not query remotes")),
 		OPT_END()
 	};
-	struct ref_states states;
+	struct ref_states states = REF_STATES_INIT;
 	struct string_list info_list = STRING_LIST_INIT_NODUP;
 	struct show_info info;
 
@@ -1225,7 +1227,6 @@ static int show(int argc, const char **argv)
 	if (!no_query)
 		query_flag = (GET_REF_STATES | GET_HEAD_NAMES | GET_PUSH_REF_STATES);
 
-	memset(&states, 0, sizeof(states));
 	memset(&info, 0, sizeof(info));
 	info.states = &states;
 	info.list = &info_list;
@@ -1334,8 +1335,7 @@ static int set_head(int argc, const char **argv)
 	if (!opt_a && !opt_d && argc == 2) {
 		head_name = xstrdup(argv[1]);
 	} else if (opt_a && !opt_d && argc == 1) {
-		struct ref_states states;
-		memset(&states, 0, sizeof(states));
+		struct ref_states states = REF_STATES_INIT;
 		get_remote_ref_states(argv[0], &states, GET_HEAD_NAMES);
 		if (!states.heads.nr)
 			result |= error(_("Cannot determine remote HEAD"));
@@ -1374,14 +1374,13 @@ static int set_head(int argc, const char **argv)
 static int prune_remote(const char *remote, int dry_run)
 {
 	int result = 0;
-	struct ref_states states;
+	struct ref_states states = REF_STATES_INIT;
 	struct string_list refs_to_prune = STRING_LIST_INIT_NODUP;
 	struct string_list_item *item;
 	const char *dangling_msg = dry_run
 		? _(" %s will become dangling!")
 		: _(" %s has become dangling!");
 
-	memset(&states, 0, sizeof(states));
 	get_remote_ref_states(remote, &states, GET_REF_STATES);
 
 	if (!states.stale.nr) {
-- 
2.33.0.1316.gb2e9b3ba3ae

[PATCH 6/6] builtin/remote.c: add and use SHOW_INFO_INIT

[Ævar Arnfjörð Bjarmason]

In the preceding commit we introduced REF_STATES_INIT, but did not
change the "struct show_info" to have a corresponding
initializer. Let's do that, and make it use "REF_STATES_INIT" and
"STRING_LIST_INIT_DUP", doing that requires changing "list" and
"states" away from being pointers.

The resulting end-state is simpler since we omit the local "info_list"
and "states" variables in show() as well as the memset().

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/remote.c | 90 ++++++++++++++++++++++++------------------------
 1 file changed, 45 insertions(+), 45 deletions(-)

diff --git a/builtin/remote.c b/builtin/remote.c
index 160dd954f74..deb48772ac5 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -972,26 +972,31 @@ static int get_remote_ref_states(const char *name,
 }
 
 struct show_info {
-	struct string_list *list;
-	struct ref_states *states;
+	struct string_list list;
+	struct ref_states states;
 	int width, width2;
 	int any_rebase;
 };
 
+#define SHOW_INFO_INIT { \
+	.list = STRING_LIST_INIT_DUP, \
+	.states = REF_STATES_INIT, \
+}
+
 static int add_remote_to_show_info(struct string_list_item *item, void *cb_data)
 {
 	struct show_info *info = cb_data;
 	int n = strlen(item->string);
 	if (n > info->width)
 		info->width = n;
-	string_list_insert(info->list, item->string);
+	string_list_insert(&info->list, item->string);
 	return 0;
 }
 
 static int show_remote_info_item(struct string_list_item *item, void *cb_data)
 {
 	struct show_info *info = cb_data;
-	struct ref_states *states = info->states;
+	struct ref_states *states = &info->states;
 	const char *name = item->string;
 
 	if (states->queried) {
@@ -1018,7 +1023,7 @@ static int show_remote_info_item(struct string_list_item *item, void *cb_data)
 static int add_local_to_show_info(struct string_list_item *branch_item, void *cb_data)
 {
 	struct show_info *show_info = cb_data;
-	struct ref_states *states = show_info->states;
+	struct ref_states *states = &show_info->states;
 	struct branch_info *branch_info = branch_item->util;
 	struct string_list_item *item;
 	int n;
@@ -1031,7 +1036,7 @@ static int add_local_to_show_info(struct string_list_item *branch_item, void *cb
 	if (branch_info->rebase >= REBASE_TRUE)
 		show_info->any_rebase = 1;
 
-	item = string_list_insert(show_info->list, branch_item->string);
+	item = string_list_insert(&show_info->list, branch_item->string);
 	item->util = branch_info;
 
 	return 0;
@@ -1086,7 +1091,7 @@ static int add_push_to_show_info(struct string_list_item *push_item, void *cb_da
 		show_info->width = n;
 	if ((n = strlen(push_info->dest)) > show_info->width2)
 		show_info->width2 = n;
-	item = string_list_append(show_info->list, push_item->string);
+	item = string_list_append(&show_info->list, push_item->string);
 	item->util = push_item->util;
 	return 0;
 }
@@ -1214,9 +1219,7 @@ static int show(int argc, const char **argv)
 		OPT_BOOL('n', NULL, &no_query, N_("do not query remotes")),
 		OPT_END()
 	};
-	struct ref_states states = REF_STATES_INIT;
-	struct string_list info_list = STRING_LIST_INIT_NODUP;
-	struct show_info info;
+	struct show_info info = SHOW_INFO_INIT;
 
 	argc = parse_options(argc, argv, NULL, options, builtin_remote_show_usage,
 			     0);
@@ -1227,25 +1230,22 @@ static int show(int argc, const char **argv)
 	if (!no_query)
 		query_flag = (GET_REF_STATES | GET_HEAD_NAMES | GET_PUSH_REF_STATES);
 
-	memset(&info, 0, sizeof(info));
-	info.states = &states;
-	info.list = &info_list;
 	for (; argc; argc--, argv++) {
 		int i;
 		const char **url;
 		int url_nr;
 
-		get_remote_ref_states(*argv, &states, query_flag);
+		get_remote_ref_states(*argv, &info.states, query_flag);
 
 		printf_ln(_("* remote %s"), *argv);
-		printf_ln(_("  Fetch URL: %s"), states.remote->url_nr > 0 ?
-		       states.remote->url[0] : _("(no URL)"));
-		if (states.remote->pushurl_nr) {
-			url = states.remote->pushurl;
-			url_nr = states.remote->pushurl_nr;
+		printf_ln(_("  Fetch URL: %s"), info.states.remote->url_nr > 0 ?
+		       info.states.remote->url[0] : _("(no URL)"));
+		if (info.states.remote->pushurl_nr) {
+			url = info.states.remote->pushurl;
+			url_nr = info.states.remote->pushurl_nr;
 		} else {
-			url = states.remote->url;
-			url_nr = states.remote->url_nr;
+			url = info.states.remote->url;
+			url_nr = info.states.remote->url_nr;
 		}
 		for (i = 0; i < url_nr; i++)
 			/*
@@ -1258,57 +1258,57 @@ static int show(int argc, const char **argv)
 			printf_ln(_("  Push  URL: %s"), _("(no URL)"));
 		if (no_query)
 			printf_ln(_("  HEAD branch: %s"), _("(not queried)"));
-		else if (!states.heads.nr)
+		else if (!info.states.heads.nr)
 			printf_ln(_("  HEAD branch: %s"), _("(unknown)"));
-		else if (states.heads.nr == 1)
-			printf_ln(_("  HEAD branch: %s"), states.heads.items[0].string);
+		else if (info.states.heads.nr == 1)
+			printf_ln(_("  HEAD branch: %s"), info.states.heads.items[0].string);
 		else {
 			printf(_("  HEAD branch (remote HEAD is ambiguous,"
 				 " may be one of the following):\n"));
-			for (i = 0; i < states.heads.nr; i++)
-				printf("    %s\n", states.heads.items[i].string);
+			for (i = 0; i < info.states.heads.nr; i++)
+				printf("    %s\n", info.states.heads.items[i].string);
 		}
 
 		/* remote branch info */
 		info.width = 0;
-		for_each_string_list(&states.new_refs, add_remote_to_show_info, &info);
-		for_each_string_list(&states.tracked, add_remote_to_show_info, &info);
-		for_each_string_list(&states.stale, add_remote_to_show_info, &info);
-		if (info.list->nr)
+		for_each_string_list(&info.states.new_refs, add_remote_to_show_info, &info);
+		for_each_string_list(&info.states.tracked, add_remote_to_show_info, &info);
+		for_each_string_list(&info.states.stale, add_remote_to_show_info, &info);
+		if (info.list.nr)
 			printf_ln(Q_("  Remote branch:%s",
 				     "  Remote branches:%s",
-				     info.list->nr),
+				     info.list.nr),
 				  no_query ? _(" (status not queried)") : "");
-		for_each_string_list(info.list, show_remote_info_item, &info);
-		string_list_clear(info.list, 0);
+		for_each_string_list(&info.list, show_remote_info_item, &info);
+		string_list_clear(&info.list, 0);
 
 		/* git pull info */
 		info.width = 0;
 		info.any_rebase = 0;
 		for_each_string_list(&branch_list, add_local_to_show_info, &info);
-		if (info.list->nr)
+		if (info.list.nr)
 			printf_ln(Q_("  Local branch configured for 'git pull':",
 				     "  Local branches configured for 'git pull':",
-				     info.list->nr));
-		for_each_string_list(info.list, show_local_info_item, &info);
-		string_list_clear(info.list, 0);
+				     info.list.nr));
+		for_each_string_list(&info.list, show_local_info_item, &info);
+		string_list_clear(&info.list, 0);
 
 		/* git push info */
-		if (states.remote->mirror)
+		if (info.states.remote->mirror)
 			printf_ln(_("  Local refs will be mirrored by 'git push'"));
 
 		info.width = info.width2 = 0;
-		for_each_string_list(&states.push, add_push_to_show_info, &info);
-		QSORT(info.list->items, info.list->nr, cmp_string_with_push);
-		if (info.list->nr)
+		for_each_string_list(&info.states.push, add_push_to_show_info, &info);
+		QSORT(info.list.items, info.list.nr, cmp_string_with_push);
+		if (info.list.nr)
 			printf_ln(Q_("  Local ref configured for 'git push'%s:",
 				     "  Local refs configured for 'git push'%s:",
-				     info.list->nr),
+				     info.list.nr),
 				  no_query ? _(" (status not queried)") : "");
-		for_each_string_list(info.list, show_push_info_item, &info);
-		string_list_clear(info.list, 0);
+		for_each_string_list(&info.list, show_push_info_item, &info);
+		string_list_clear(&info.list, 0);
 
-		free_remote_ref_states(&states);
+		free_remote_ref_states(&info.states);
 	}
 
 	return result;
-- 
2.33.0.1316.gb2e9b3ba3ae

[PATCH v2 0/5] Non-trivial designated initializer conversion

[Ævar Arnfjörð Bjarmason]

See
http://lore.kernel.org/git/cover-0.6-00000000000-20210927T004920Z-avarab@gmail.com
for the v1 & goals. As noted in
http://lore.kernel.org/git/87lf3i1e7k.fsf@evledraar.gmail.com I
ejected the previous 3/6. It wasn't incorrect, but partially
converting to an *_INIT macro doesn't make any sense in the "struct
shortlog" case.

Ævar Arnfjörð Bjarmason (5):
  daemon.c: refactor hostinfo_init() to HOSTINFO_INIT macro
  builtin/blame.c: refactor commit_info_init() to COMMIT_INFO_INIT macro
  urlmatch.[ch]: add and use URLMATCH_CONFIG_INIT
  builtin/remote.c: add and use a REF_STATES_INIT
  builtin/remote.c: add and use SHOW_INFO_INIT

 builtin/blame.c  |  30 ++++++-------
 builtin/config.c |   3 +-
 builtin/remote.c | 111 +++++++++++++++++++++++------------------------
 credential.c     |   4 +-
 daemon.c         |  19 +++-----
 http.c           |   4 +-
 urlmatch.h       |   4 ++
 7 files changed, 82 insertions(+), 93 deletions(-)

Range-diff against v1:
1:  3130693b416 = 1:  3130693b416 daemon.c: refactor hostinfo_init() to HOSTINFO_INIT macro
2:  65c5295c1ac = 2:  65c5295c1ac builtin/blame.c: refactor commit_info_init() to COMMIT_INFO_INIT macro
3:  c9db107fcb1 < -:  ----------- shortlog: use designated initializer for "struct shortlog"
4:  cb4c81dcc83 = 3:  3783788b553 urlmatch.[ch]: add and use URLMATCH_CONFIG_INIT
5:  1c34c00feb5 = 4:  13ef9566903 builtin/remote.c: add and use a REF_STATES_INIT
6:  76fa070e89c = 5:  b78a9ec0846 builtin/remote.c: add and use SHOW_INFO_INIT
-- 
2.33.0.1316.gb2e9b3ba3ae

[PATCH v2 1/5] daemon.c: refactor hostinfo_init() to HOSTINFO_INIT macro

[Ævar Arnfjörð Bjarmason]

Remove the hostinfo_init() function added in 01cec54e135 (daemon:
deglobalize hostname information, 2015-03-07) and instead initialize
the "struct hostinfo" with a macro.

This is the more idiomatic pattern in the codebase, and doesn't leave
us wondering when we see the *_init() function if this struct needs
more complex initialization than a macro can provide.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 daemon.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/daemon.c b/daemon.c
index 5c4cbad62d0..d80d009d1a1 100644
--- a/daemon.c
+++ b/daemon.c
@@ -63,6 +63,12 @@ struct hostinfo {
 	unsigned int hostname_lookup_done:1;
 	unsigned int saw_extended_args:1;
 };
+#define HOSTINFO_INIT { \
+	.hostname = STRBUF_INIT, \
+	.canon_hostname = STRBUF_INIT, \
+	.ip_address = STRBUF_INIT, \
+	.tcp_port = STRBUF_INIT, \
+}
 
 static void lookup_hostname(struct hostinfo *hi);
 
@@ -727,15 +733,6 @@ static void lookup_hostname(struct hostinfo *hi)
 	}
 }
 
-static void hostinfo_init(struct hostinfo *hi)
-{
-	memset(hi, 0, sizeof(*hi));
-	strbuf_init(&hi->hostname, 0);
-	strbuf_init(&hi->canon_hostname, 0);
-	strbuf_init(&hi->ip_address, 0);
-	strbuf_init(&hi->tcp_port, 0);
-}
-
 static void hostinfo_clear(struct hostinfo *hi)
 {
 	strbuf_release(&hi->hostname);
@@ -760,11 +757,9 @@ static int execute(void)
 	char *line = packet_buffer;
 	int pktlen, len, i;
 	char *addr = getenv("REMOTE_ADDR"), *port = getenv("REMOTE_PORT");
-	struct hostinfo hi;
+	struct hostinfo hi = HOSTINFO_INIT;
 	struct strvec env = STRVEC_INIT;
 
-	hostinfo_init(&hi);
-
 	if (addr)
 		loginfo("Connection from %s:%s", addr, port);
 
-- 
2.33.0.1316.gb2e9b3ba3ae

[PATCH v2 2/5] builtin/blame.c: refactor commit_info_init() to COMMIT_INFO_INIT macro

[Ævar Arnfjörð Bjarmason]

Remove the commit_info_init() function addded in ea02ffa3857 (mailmap:
simplify map_user() interface, 2013-01-05) and instead initialize the
"struct commit_info" with a macro.

This is the more idiomatic pattern in the codebase, and doesn't leave
us wondering when we see the *_init() function if this struct needs
more complex initialization than a macro can provide.

The get_commit_info() function is only called by the three callers
being changed here immediately after initializing the struct with the
macros, so by moving the initialization to the callers we don't need
to do it in get_commit_info() anymore.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/blame.c | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 641523ff9af..1c31a996403 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -101,6 +101,16 @@ struct commit_info {
 	struct strbuf summary;
 };
 
+#define COMMIT_INFO_INIT { \
+	.author = STRBUF_INIT, \
+	.author_mail = STRBUF_INIT, \
+	.author_tz = STRBUF_INIT, \
+	.committer = STRBUF_INIT, \
+	.committer_mail = STRBUF_INIT, \
+	.committer_tz = STRBUF_INIT, \
+	.summary = STRBUF_INIT, \
+}
+
 /*
  * Parse author/committer line in the commit object buffer
  */
@@ -160,18 +170,6 @@ static void get_ac_line(const char *inbuf, const char *what,
 	strbuf_add(name, namebuf, namelen);
 }
 
-static void commit_info_init(struct commit_info *ci)
-{
-
-	strbuf_init(&ci->author, 0);
-	strbuf_init(&ci->author_mail, 0);
-	strbuf_init(&ci->author_tz, 0);
-	strbuf_init(&ci->committer, 0);
-	strbuf_init(&ci->committer_mail, 0);
-	strbuf_init(&ci->committer_tz, 0);
-	strbuf_init(&ci->summary, 0);
-}
-
 static void commit_info_destroy(struct commit_info *ci)
 {
 
@@ -192,8 +190,6 @@ static void get_commit_info(struct commit *commit,
 	const char *subject, *encoding;
 	const char *message;
 
-	commit_info_init(ret);
-
 	encoding = get_log_output_encoding();
 	message = logmsg_reencode(commit, NULL, encoding);
 	get_ac_line(message, "\nauthor ",
@@ -246,7 +242,7 @@ static void write_filename_info(struct blame_origin *suspect)
  */
 static int emit_one_suspect_detail(struct blame_origin *suspect, int repeat)
 {
-	struct commit_info ci;
+	struct commit_info ci = COMMIT_INFO_INIT;
 
 	if (!repeat && (suspect->commit->object.flags & METAINFO_SHOWN))
 		return 0;
@@ -440,7 +436,7 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int
 	int cnt;
 	const char *cp;
 	struct blame_origin *suspect = ent->suspect;
-	struct commit_info ci;
+	struct commit_info ci = COMMIT_INFO_INIT;
 	char hex[GIT_MAX_HEXSZ + 1];
 	int show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP);
 	const char *default_color = NULL, *color = NULL, *reset = NULL;
@@ -630,7 +626,7 @@ static void find_alignment(struct blame_scoreboard *sb, int *option)
 		if (longest_file < num)
 			longest_file = num;
 		if (!(suspect->commit->object.flags & METAINFO_SHOWN)) {
-			struct commit_info ci;
+			struct commit_info ci = COMMIT_INFO_INIT;
 			suspect->commit->object.flags |= METAINFO_SHOWN;
 			get_commit_info(suspect->commit, &ci, 1);
 			if (*option & OUTPUT_SHOW_EMAIL)
-- 
2.33.0.1316.gb2e9b3ba3ae

[PATCH v2 3/5] urlmatch.[ch]: add and use URLMATCH_CONFIG_INIT

[Ævar Arnfjörð Bjarmason]

Change the initialization pattern of "struct urlmatch_config" to use
an *_INIT macro and designated initializers.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/config.c | 3 +--
 credential.c     | 4 +---
 http.c           | 4 +---
 urlmatch.h       | 4 ++++
 4 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 865fddd6ce8..1ea4f68b7de 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -575,11 +575,10 @@ static int get_urlmatch(const char *var, const char *url)
 	int ret;
 	char *section_tail;
 	struct string_list_item *item;
-	struct urlmatch_config config = { STRING_LIST_INIT_DUP };
+	struct urlmatch_config config = URLMATCH_CONFIG_INIT;
 	struct string_list values = STRING_LIST_INIT_DUP;
 
 	config.collect_fn = urlmatch_collect_fn;
-	config.cascade_fn = NULL;
 	config.cb = &values;
 
 	if (!url_normalize(url, &config.url))
diff --git a/credential.c b/credential.c
index 000ac7a8d43..c85db8a75cd 100644
--- a/credential.c
+++ b/credential.c
@@ -105,7 +105,7 @@ static int match_partial_url(const char *url, void *cb)
 static void credential_apply_config(struct credential *c)
 {
 	char *normalized_url;
-	struct urlmatch_config config = { STRING_LIST_INIT_DUP };
+	struct urlmatch_config config = URLMATCH_CONFIG_INIT;
 	struct strbuf url = STRBUF_INIT;
 
 	if (!c->host)
@@ -117,9 +117,7 @@ static void credential_apply_config(struct credential *c)
 		return;
 
 	config.section = "credential";
-	config.key = NULL;
 	config.collect_fn = credential_config_callback;
-	config.cascade_fn = NULL;
 	config.select_fn = select_all;
 	config.fallback_match_fn = match_partial_url;
 	config.cb = c;
diff --git a/http.c b/http.c
index d7c20493d7f..3d6ad5c7be8 100644
--- a/http.c
+++ b/http.c
@@ -990,13 +990,11 @@ void http_init(struct remote *remote, const char *url, int proactive_auth)
 	char *low_speed_limit;
 	char *low_speed_time;
 	char *normalized_url;
-	struct urlmatch_config config = { STRING_LIST_INIT_DUP };
+	struct urlmatch_config config = URLMATCH_CONFIG_INIT;
 
 	config.section = "http";
-	config.key = NULL;
 	config.collect_fn = http_options;
 	config.cascade_fn = git_default_config;
-	config.cb = NULL;
 
 	http_is_verbose = 0;
 	normalized_url = url_normalize(url, &config.url);
diff --git a/urlmatch.h b/urlmatch.h
index 6ff42f81b0c..34a3ba6d197 100644
--- a/urlmatch.h
+++ b/urlmatch.h
@@ -66,6 +66,10 @@ struct urlmatch_config {
 	int (*fallback_match_fn)(const char *url, void *cb);
 };
 
+#define URLMATCH_CONFIG_INIT { \
+	.vars = STRING_LIST_INIT_DUP, \
+}
+
 int urlmatch_config_entry(const char *var, const char *value, void *cb);
 
 #endif /* URL_MATCH_H */
-- 
2.33.0.1316.gb2e9b3ba3ae

Re: [PATCH v2 3/5] urlmatch.[ch]: add and use URLMATCH_CONFIG_INIT

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> @@ -575,11 +575,10 @@ static int get_urlmatch(const char *var, const char *url)
>  	int ret;
>  	char *section_tail;
>  	struct string_list_item *item;
> -	struct urlmatch_config config = { STRING_LIST_INIT_DUP };
> +	struct urlmatch_config config = URLMATCH_CONFIG_INIT;

While I think this is much easier to follow than the original, I am
not quite sure about the removal of these explicit _assignments_ in
code, like this ...

>  	config.collect_fn = urlmatch_collect_fn;
> -	config.cascade_fn = NULL;
>  	config.cb = &values;

and this ...

>  	config.section = "credential";
> -	config.key = NULL;
>  	config.collect_fn = credential_config_callback;
> -	config.cascade_fn = NULL;
>  	config.select_fn = select_all;

and this ...

>  	config.section = "http";
> -	config.key = NULL;
>  	config.collect_fn = http_options;
>  	config.cascade_fn = git_default_config;
> -	config.cb = NULL;

as they have documentation value (e.g. "for this invocation I do
want to use no cascade_fn" is a statement worth making, by having
it next to the assignment to .collect_fn member).

[PATCH v2 4/5] builtin/remote.c: add and use a REF_STATES_INIT

[Ævar Arnfjörð Bjarmason]

Use a new REF_STATES_INIT designated initializer instead of assigning
to the "strdup_strings" member of the previously memzero()'d version
of this struct.

The pattern of assigning to "strdup_strings" dates back to
211c89682ee (Make git-remote a builtin, 2008-02-29) (when it was
"strdup_paths"), i.e. long before we used anything like our current
established *_INIT patterns consistently.

Then in e61e0cc6b70 (builtin-remote: teach show to display remote
HEAD, 2009-02-25) and e5dcbfd9ab7 (builtin-remote: new show output
style for push refspecs, 2009-02-25) we added some more of these.

As it turns out we only initialized this struct three times, all the
other uses were of pointers to those initialized structs. So let's
initialize it in those three places, skip the memset(), and pass those
structs down appropriately.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/remote.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/builtin/remote.c b/builtin/remote.c
index 7f88e6ce9de..160dd954f74 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -344,6 +344,14 @@ struct ref_states {
 	int queried;
 };
 
+#define REF_STATES_INIT { \
+	.new_refs = STRING_LIST_INIT_DUP, \
+	.stale = STRING_LIST_INIT_DUP, \
+	.tracked = STRING_LIST_INIT_DUP, \
+	.heads = STRING_LIST_INIT_DUP, \
+	.push = STRING_LIST_INIT_DUP, \
+}
+
 static int get_ref_states(const struct ref *remote_refs, struct ref_states *states)
 {
 	struct ref *fetch_map = NULL, **tail = &fetch_map;
@@ -355,9 +363,6 @@ static int get_ref_states(const struct ref *remote_refs, struct ref_states *stat
 			die(_("Could not get fetch map for refspec %s"),
 				states->remote->fetch.raw[i]);
 
-	states->new_refs.strdup_strings = 1;
-	states->tracked.strdup_strings = 1;
-	states->stale.strdup_strings = 1;
 	for (ref = fetch_map; ref; ref = ref->next) {
 		if (!ref->peer_ref || !ref_exists(ref->peer_ref->name))
 			string_list_append(&states->new_refs, abbrev_branch(ref->name));
@@ -406,7 +411,6 @@ static int get_push_ref_states(const struct ref *remote_refs,
 
 	match_push_refs(local_refs, &push_map, &remote->push, MATCH_REFS_NONE);
 
-	states->push.strdup_strings = 1;
 	for (ref = push_map; ref; ref = ref->next) {
 		struct string_list_item *item;
 		struct push_info *info;
@@ -449,7 +453,6 @@ static int get_push_ref_states_noquery(struct ref_states *states)
 	if (remote->mirror)
 		return 0;
 
-	states->push.strdup_strings = 1;
 	if (!remote->push.nr) {
 		item = string_list_append(&states->push, _("(matching)"));
 		info = item->util = xcalloc(1, sizeof(struct push_info));
@@ -483,7 +486,6 @@ static int get_head_names(const struct ref *remote_refs, struct ref_states *stat
 	refspec.force = 0;
 	refspec.pattern = 1;
 	refspec.src = refspec.dst = "refs/heads/*";
-	states->heads.strdup_strings = 1;
 	get_fetch_map(remote_refs, &refspec, &fetch_map_tail, 0);
 	matches = guess_remote_head(find_ref_by_name(remote_refs, "HEAD"),
 				    fetch_map, 1);
@@ -1212,7 +1214,7 @@ static int show(int argc, const char **argv)
 		OPT_BOOL('n', NULL, &no_query, N_("do not query remotes")),
 		OPT_END()
 	};
-	struct ref_states states;
+	struct ref_states states = REF_STATES_INIT;
 	struct string_list info_list = STRING_LIST_INIT_NODUP;
 	struct show_info info;
 
@@ -1225,7 +1227,6 @@ static int show(int argc, const char **argv)
 	if (!no_query)
 		query_flag = (GET_REF_STATES | GET_HEAD_NAMES | GET_PUSH_REF_STATES);
 
-	memset(&states, 0, sizeof(states));
 	memset(&info, 0, sizeof(info));
 	info.states = &states;
 	info.list = &info_list;
@@ -1334,8 +1335,7 @@ static int set_head(int argc, const char **argv)
 	if (!opt_a && !opt_d && argc == 2) {
 		head_name = xstrdup(argv[1]);
 	} else if (opt_a && !opt_d && argc == 1) {
-		struct ref_states states;
-		memset(&states, 0, sizeof(states));
+		struct ref_states states = REF_STATES_INIT;
 		get_remote_ref_states(argv[0], &states, GET_HEAD_NAMES);
 		if (!states.heads.nr)
 			result |= error(_("Cannot determine remote HEAD"));
@@ -1374,14 +1374,13 @@ static int set_head(int argc, const char **argv)
 static int prune_remote(const char *remote, int dry_run)
 {
 	int result = 0;
-	struct ref_states states;
+	struct ref_states states = REF_STATES_INIT;
 	struct string_list refs_to_prune = STRING_LIST_INIT_NODUP;
 	struct string_list_item *item;
 	const char *dangling_msg = dry_run
 		? _(" %s will become dangling!")
 		: _(" %s has become dangling!");
 
-	memset(&states, 0, sizeof(states));
 	get_remote_ref_states(remote, &states, GET_REF_STATES);
 
 	if (!states.stale.nr) {
-- 
2.33.0.1316.gb2e9b3ba3ae

Re: [PATCH v2 4/5] builtin/remote.c: add and use a REF_STATES_INIT

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Use a new REF_STATES_INIT designated initializer instead of assigning
> to the "strdup_strings" member of the previously memzero()'d version
> of this struct.
>
> The pattern of assigning to "strdup_strings" dates back to
> 211c89682ee (Make git-remote a builtin, 2008-02-29) (when it was
> "strdup_paths"), i.e. long before we used anything like our current
> established *_INIT patterns consistently.
>
> Then in e61e0cc6b70 (builtin-remote: teach show to display remote
> HEAD, 2009-02-25) and e5dcbfd9ab7 (builtin-remote: new show output
> style for push refspecs, 2009-02-25) we added some more of these.
>
> As it turns out we only initialized this struct three times, all the
> other uses were of pointers to those initialized structs. So let's
> initialize it in those three places, skip the memset(), and pass those
> structs down appropriately.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  builtin/remote.c | 23 +++++++++++------------
>  1 file changed, 11 insertions(+), 12 deletions(-)
>
> diff --git a/builtin/remote.c b/builtin/remote.c
> index 7f88e6ce9de..160dd954f74 100644
> --- a/builtin/remote.c
> +++ b/builtin/remote.c
> @@ -344,6 +344,14 @@ struct ref_states {
>  	int queried;
>  };
>  
> +#define REF_STATES_INIT { \
> +	.new_refs = STRING_LIST_INIT_DUP, \
> +	.stale = STRING_LIST_INIT_DUP, \
> +	.tracked = STRING_LIST_INIT_DUP, \
> +	.heads = STRING_LIST_INIT_DUP, \
> +	.push = STRING_LIST_INIT_DUP, \
> +}

So, now everybody owns the string, but ...

>  static int get_ref_states(const struct ref *remote_refs, struct ref_states *states)
>  {
>  	struct ref *fetch_map = NULL, **tail = &fetch_map;
> @@ -355,9 +363,6 @@ static int get_ref_states(const struct ref *remote_refs, struct ref_states *stat
>  			die(_("Could not get fetch map for refspec %s"),
>  				states->remote->fetch.raw[i]);
>  
> -	states->new_refs.strdup_strings = 1;
> -	states->tracked.strdup_strings = 1;
> -	states->stale.strdup_strings = 1;

... we used to set up selectively to own.

How would we make sure after this change we are not adding leaks?
Is there a way to do so mechanically?

> -	struct ref_states states;
> +	struct ref_states states = REF_STATES_INIT;
>  	struct string_list refs_to_prune = STRING_LIST_INIT_NODUP;
>  	struct string_list_item *item;
>  	const char *dangling_msg = dry_run
>  		? _(" %s will become dangling!")
>  		: _(" %s has become dangling!");
>  
> -	memset(&states, 0, sizeof(states));
>  	get_remote_ref_states(remote, &states, GET_REF_STATES);

Like this one, get_remote_ref_states() used to receive states that
are set to borrow strings, but now we get duplicated strings, right?
Are we leaking whatever strings we push to these string lists now?

Re: [PATCH v2 4/5] builtin/remote.c: add and use a REF_STATES_INIT

[Ævar Arnfjörð Bjarmason]

On Mon, Sep 27 2021, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> Use a new REF_STATES_INIT designated initializer instead of assigning
>> to the "strdup_strings" member of the previously memzero()'d version
>> of this struct.
>>
>> The pattern of assigning to "strdup_strings" dates back to
>> 211c89682ee (Make git-remote a builtin, 2008-02-29) (when it was
>> "strdup_paths"), i.e. long before we used anything like our current
>> established *_INIT patterns consistently.
>>
>> Then in e61e0cc6b70 (builtin-remote: teach show to display remote
>> HEAD, 2009-02-25) and e5dcbfd9ab7 (builtin-remote: new show output
>> style for push refspecs, 2009-02-25) we added some more of these.
>>
>> As it turns out we only initialized this struct three times, all the
>> other uses were of pointers to those initialized structs. So let's
>> initialize it in those three places, skip the memset(), and pass those
>> structs down appropriately.
>>
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> ---
>>  builtin/remote.c | 23 +++++++++++------------
>>  1 file changed, 11 insertions(+), 12 deletions(-)
>>
>> diff --git a/builtin/remote.c b/builtin/remote.c
>> index 7f88e6ce9de..160dd954f74 100644
>> --- a/builtin/remote.c
>> +++ b/builtin/remote.c
>> @@ -344,6 +344,14 @@ struct ref_states {
>>  	int queried;
>>  };
>>  
>> +#define REF_STATES_INIT { \
>> +	.new_refs = STRING_LIST_INIT_DUP, \
>> +	.stale = STRING_LIST_INIT_DUP, \
>> +	.tracked = STRING_LIST_INIT_DUP, \
>> +	.heads = STRING_LIST_INIT_DUP, \
>> +	.push = STRING_LIST_INIT_DUP, \
>> +}
>
> So, now everybody owns the string, but ...
>
>>  static int get_ref_states(const struct ref *remote_refs, struct ref_states *states)
>>  {
>>  	struct ref *fetch_map = NULL, **tail = &fetch_map;
>> @@ -355,9 +363,6 @@ static int get_ref_states(const struct ref *remote_refs, struct ref_states *stat
>>  			die(_("Could not get fetch map for refspec %s"),
>>  				states->remote->fetch.raw[i]);
>>  
>> -	states->new_refs.strdup_strings = 1;
>> -	states->tracked.strdup_strings = 1;
>> -	states->stale.strdup_strings = 1;
>
> ... we used to set up selectively to own.
>
> How would we make sure after this change we are not adding leaks?
> Is there a way to do so mechanically?

I manual run with SANITIZE=leak shows the same amount of memory leakage
before & after this change for me.

We should fix those leaks, but until my "tests: add a test mode for
SANITIZE=leak, run it in CI" lands on master I'm still waiting on fixing
those, and this series will help make it easier.

The ownership of the "states" struct or its lifetime isn't different
after this change. It's only that we're doing:

    struct foo = FOO_INIT;
    /* use &foo */

Instead of:

    struct foo;
    memset(&foo, 0, sizeof(foo));
    foo->some_list.strdup_strings = 1;

What is different is that that "strdup_strings" member in the contained
"some_list" will be set to "1" earlier. E.g. in the case of this in the
pre-image:

    states->new_refs.strdup_strings = 1;

We'd do that in get_ref_states(), which was called by
get_remote_ref_states(), which was e.g. called by show(). So before we
wouldn't flip that particular value until we got all the way down into
get_remote_ref_states(), now we initialize the struct like that on the
stack right away.

But it doesn't result in any new allocations, string_list like strbuf
etc. won't allocate until something is pushed onto the list, using the
common "nr/alloc" & malloc on demand pattern.

>> -	struct ref_states states;
>> +	struct ref_states states = REF_STATES_INIT;
>>  	struct string_list refs_to_prune = STRING_LIST_INIT_NODUP;
>>  	struct string_list_item *item;
>>  	const char *dangling_msg = dry_run
>>  		? _(" %s will become dangling!")
>>  		: _(" %s has become dangling!");
>>  
>> -	memset(&states, 0, sizeof(states));
>>  	get_remote_ref_states(remote, &states, GET_REF_STATES);
>
> Like this one, get_remote_ref_states() used to receive states that
> are set to borrow strings, but now we get duplicated strings, right?
> Are we leaking whatever strings we push to these string lists now?

Ah, yes it *could* happen as a side-effect of this sotr of change that
that memset() was implicitly flipping some string_list structs to the
equivalent of strdup_strings=0.

But that's not the case here, it was just memset() boilerplate, then in
get_remote_ref_states() we'd set all the string lists we'd use to "dup".

Re: [PATCH v2 4/5] builtin/remote.c: add and use a REF_STATES_INIT

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> The ownership of the "states" struct or its lifetime isn't different
> after this change. It's only that we're doing:
>
>     struct foo = FOO_INIT;
>     /* use &foo */
>
> Instead of:
>
>     struct foo;
>     memset(&foo, 0, sizeof(foo));
>     foo->some_list.strdup_strings = 1;

No, I am not talking about ownership of "foo" itself.  What changes
is ownership of some of the string_list embedded in "foo" that used
NOT to have strdup_strings member set now have the bit set, so the
string list owns the string.

>>> +	struct ref_states states = REF_STATES_INIT;
>>>  	struct string_list refs_to_prune = STRING_LIST_INIT_NODUP;
>>>  	struct string_list_item *item;
>>>  	const char *dangling_msg = dry_run
>>>  		? _(" %s will become dangling!")
>>>  		: _(" %s has become dangling!");
>>>  
>>> -	memset(&states, 0, sizeof(states));
>>>  	get_remote_ref_states(remote, &states, GET_REF_STATES);
>>
>> Like this one, get_remote_ref_states() used to receive states that
>> are set to borrow strings, but now we get duplicated strings, right?
>> Are we leaking whatever strings we push to these string lists now?
>
> Ah, yes it *could* happen as a side-effect of this sotr of change that
> that memset() was implicitly flipping some string_list structs to the
> equivalent of strdup_strings=0.
>
> But that's not the case here, it was just memset() boilerplate, then in
> get_remote_ref_states() we'd set all the string lists we'd use to "dup".

OK, get_remotes_ref_states() marked all string list to the dup
variant, then we won't have a problem.  I was trying to make sure
that was something you looked at when preparing these patches.

Thanks.

[PATCH v2 5/5] builtin/remote.c: add and use SHOW_INFO_INIT

[Ævar Arnfjörð Bjarmason]

In the preceding commit we introduced REF_STATES_INIT, but did not
change the "struct show_info" to have a corresponding
initializer. Let's do that, and make it use "REF_STATES_INIT" and
"STRING_LIST_INIT_DUP", doing that requires changing "list" and
"states" away from being pointers.

The resulting end-state is simpler since we omit the local "info_list"
and "states" variables in show() as well as the memset().

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/remote.c | 90 ++++++++++++++++++++++++------------------------
 1 file changed, 45 insertions(+), 45 deletions(-)

diff --git a/builtin/remote.c b/builtin/remote.c
index 160dd954f74..deb48772ac5 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -972,26 +972,31 @@ static int get_remote_ref_states(const char *name,
 }
 
 struct show_info {
-	struct string_list *list;
-	struct ref_states *states;
+	struct string_list list;
+	struct ref_states states;
 	int width, width2;
 	int any_rebase;
 };
 
+#define SHOW_INFO_INIT { \
+	.list = STRING_LIST_INIT_DUP, \
+	.states = REF_STATES_INIT, \
+}
+
 static int add_remote_to_show_info(struct string_list_item *item, void *cb_data)
 {
 	struct show_info *info = cb_data;
 	int n = strlen(item->string);
 	if (n > info->width)
 		info->width = n;
-	string_list_insert(info->list, item->string);
+	string_list_insert(&info->list, item->string);
 	return 0;
 }
 
 static int show_remote_info_item(struct string_list_item *item, void *cb_data)
 {
 	struct show_info *info = cb_data;
-	struct ref_states *states = info->states;
+	struct ref_states *states = &info->states;
 	const char *name = item->string;
 
 	if (states->queried) {
@@ -1018,7 +1023,7 @@ static int show_remote_info_item(struct string_list_item *item, void *cb_data)
 static int add_local_to_show_info(struct string_list_item *branch_item, void *cb_data)
 {
 	struct show_info *show_info = cb_data;
-	struct ref_states *states = show_info->states;
+	struct ref_states *states = &show_info->states;
 	struct branch_info *branch_info = branch_item->util;
 	struct string_list_item *item;
 	int n;
@@ -1031,7 +1036,7 @@ static int add_local_to_show_info(struct string_list_item *branch_item, void *cb
 	if (branch_info->rebase >= REBASE_TRUE)
 		show_info->any_rebase = 1;
 
-	item = string_list_insert(show_info->list, branch_item->string);
+	item = string_list_insert(&show_info->list, branch_item->string);
 	item->util = branch_info;
 
 	return 0;
@@ -1086,7 +1091,7 @@ static int add_push_to_show_info(struct string_list_item *push_item, void *cb_da
 		show_info->width = n;
 	if ((n = strlen(push_info->dest)) > show_info->width2)
 		show_info->width2 = n;
-	item = string_list_append(show_info->list, push_item->string);
+	item = string_list_append(&show_info->list, push_item->string);
 	item->util = push_item->util;
 	return 0;
 }
@@ -1214,9 +1219,7 @@ static int show(int argc, const char **argv)
 		OPT_BOOL('n', NULL, &no_query, N_("do not query remotes")),
 		OPT_END()
 	};
-	struct ref_states states = REF_STATES_INIT;
-	struct string_list info_list = STRING_LIST_INIT_NODUP;
-	struct show_info info;
+	struct show_info info = SHOW_INFO_INIT;
 
 	argc = parse_options(argc, argv, NULL, options, builtin_remote_show_usage,
 			     0);
@@ -1227,25 +1230,22 @@ static int show(int argc, const char **argv)
 	if (!no_query)
 		query_flag = (GET_REF_STATES | GET_HEAD_NAMES | GET_PUSH_REF_STATES);
 
-	memset(&info, 0, sizeof(info));
-	info.states = &states;
-	info.list = &info_list;
 	for (; argc; argc--, argv++) {
 		int i;
 		const char **url;
 		int url_nr;
 
-		get_remote_ref_states(*argv, &states, query_flag);
+		get_remote_ref_states(*argv, &info.states, query_flag);
 
 		printf_ln(_("* remote %s"), *argv);
-		printf_ln(_("  Fetch URL: %s"), states.remote->url_nr > 0 ?
-		       states.remote->url[0] : _("(no URL)"));
-		if (states.remote->pushurl_nr) {
-			url = states.remote->pushurl;
-			url_nr = states.remote->pushurl_nr;
+		printf_ln(_("  Fetch URL: %s"), info.states.remote->url_nr > 0 ?
+		       info.states.remote->url[0] : _("(no URL)"));
+		if (info.states.remote->pushurl_nr) {
+			url = info.states.remote->pushurl;
+			url_nr = info.states.remote->pushurl_nr;
 		} else {
-			url = states.remote->url;
-			url_nr = states.remote->url_nr;
+			url = info.states.remote->url;
+			url_nr = info.states.remote->url_nr;
 		}
 		for (i = 0; i < url_nr; i++)
 			/*
@@ -1258,57 +1258,57 @@ static int show(int argc, const char **argv)
 			printf_ln(_("  Push  URL: %s"), _("(no URL)"));
 		if (no_query)
 			printf_ln(_("  HEAD branch: %s"), _("(not queried)"));
-		else if (!states.heads.nr)
+		else if (!info.states.heads.nr)
 			printf_ln(_("  HEAD branch: %s"), _("(unknown)"));
-		else if (states.heads.nr == 1)
-			printf_ln(_("  HEAD branch: %s"), states.heads.items[0].string);
+		else if (info.states.heads.nr == 1)
+			printf_ln(_("  HEAD branch: %s"), info.states.heads.items[0].string);
 		else {
 			printf(_("  HEAD branch (remote HEAD is ambiguous,"
 				 " may be one of the following):\n"));
-			for (i = 0; i < states.heads.nr; i++)
-				printf("    %s\n", states.heads.items[i].string);
+			for (i = 0; i < info.states.heads.nr; i++)
+				printf("    %s\n", info.states.heads.items[i].string);
 		}
 
 		/* remote branch info */
 		info.width = 0;
-		for_each_string_list(&states.new_refs, add_remote_to_show_info, &info);
-		for_each_string_list(&states.tracked, add_remote_to_show_info, &info);
-		for_each_string_list(&states.stale, add_remote_to_show_info, &info);
-		if (info.list->nr)
+		for_each_string_list(&info.states.new_refs, add_remote_to_show_info, &info);
+		for_each_string_list(&info.states.tracked, add_remote_to_show_info, &info);
+		for_each_string_list(&info.states.stale, add_remote_to_show_info, &info);
+		if (info.list.nr)
 			printf_ln(Q_("  Remote branch:%s",
 				     "  Remote branches:%s",
-				     info.list->nr),
+				     info.list.nr),
 				  no_query ? _(" (status not queried)") : "");
-		for_each_string_list(info.list, show_remote_info_item, &info);
-		string_list_clear(info.list, 0);
+		for_each_string_list(&info.list, show_remote_info_item, &info);
+		string_list_clear(&info.list, 0);
 
 		/* git pull info */
 		info.width = 0;
 		info.any_rebase = 0;
 		for_each_string_list(&branch_list, add_local_to_show_info, &info);
-		if (info.list->nr)
+		if (info.list.nr)
 			printf_ln(Q_("  Local branch configured for 'git pull':",
 				     "  Local branches configured for 'git pull':",
-				     info.list->nr));
-		for_each_string_list(info.list, show_local_info_item, &info);
-		string_list_clear(info.list, 0);
+				     info.list.nr));
+		for_each_string_list(&info.list, show_local_info_item, &info);
+		string_list_clear(&info.list, 0);
 
 		/* git push info */
-		if (states.remote->mirror)
+		if (info.states.remote->mirror)
 			printf_ln(_("  Local refs will be mirrored by 'git push'"));
 
 		info.width = info.width2 = 0;
-		for_each_string_list(&states.push, add_push_to_show_info, &info);
-		QSORT(info.list->items, info.list->nr, cmp_string_with_push);
-		if (info.list->nr)
+		for_each_string_list(&info.states.push, add_push_to_show_info, &info);
+		QSORT(info.list.items, info.list.nr, cmp_string_with_push);
+		if (info.list.nr)
 			printf_ln(Q_("  Local ref configured for 'git push'%s:",
 				     "  Local refs configured for 'git push'%s:",
-				     info.list->nr),
+				     info.list.nr),
 				  no_query ? _(" (status not queried)") : "");
-		for_each_string_list(info.list, show_push_info_item, &info);
-		string_list_clear(info.list, 0);
+		for_each_string_list(&info.list, show_push_info_item, &info);
+		string_list_clear(&info.list, 0);
 
-		free_remote_ref_states(&states);
+		free_remote_ref_states(&info.states);
 	}
 
 	return result;
-- 
2.33.0.1316.gb2e9b3ba3ae

[PATCH v3 0/6] Non-trivial designated initializer conversion

[Ævar Arnfjörð Bjarmason]

See
http://lore.kernel.org/git/cover-0.6-00000000000-20210927T004920Z-avarab@gmail.com
for the v1 & goals, and
https://lore.kernel.org/git/cover-v2-0.5-00000000000-20210927T125715Z-avarab@gmail.com/
for the v2.

This v3:

 * Addresses Junio's comments about URLMATCH_CONFIG_INIT use, I'm no
   longer removing explicit assignments to NULL related to it.

 * Updates the commit message of 5/6 for the discussion ending at
   https://lore.kernel.org/git/xmqqv92lmv2b.fsf@gitster.g/

 * I've added a new 6/6 with an UNPACK_TREES_OPTIONS_INIT. It's
   related to the discussion about how unpack-trees.[ch] memory
   management happens at [1], but is really orthagonal to the points
   Elijah and I were discussing there.

   I think whatever approach we'd decide to go with (his in
   en/removing-untracked-fixes, or my WIP suggestion) the move to an
   UNPACK_TREES_OPTIONS_INIT makes sense, and would be the same either
   way.

   It does make the WIP patches I've got at[1] to solve memory leaks &
   simplify the setup in unpack-trees.[ch] much smaller, and is like
   the other *_INIT changes here, so I've included it.

1. https://lore.kernel.org/git/87fstlrumj.fsf@evledraar.gmail.com/
2. https://github.com/avar/git/compare/avar/post-sanitize-leak-test-mode-add-and-use-revisions-release...avar/post-sanitize-leak-test-mode-unpack-trees-and-dir

Ævar Arnfjörð Bjarmason (6):
  daemon.c: refactor hostinfo_init() to HOSTINFO_INIT macro
  builtin/blame.c: refactor commit_info_init() to COMMIT_INFO_INIT macro
  urlmatch.[ch]: add and use URLMATCH_CONFIG_INIT
  builtin/remote.c: add and use a REF_STATES_INIT
  builtin/remote.c: add and use SHOW_INFO_INIT
  unpack-trees.[ch]: define and use a UNPACK_TREES_OPTIONS_INIT

 archive.c                 |   3 +-
 builtin/am.c              |   6 +--
 builtin/blame.c           |  30 +++++------
 builtin/checkout.c        |   6 +--
 builtin/clone.c           |   3 +-
 builtin/commit.c          |   3 +-
 builtin/config.c          |   2 +-
 builtin/merge.c           |   3 +-
 builtin/read-tree.c       |   3 +-
 builtin/remote.c          | 111 +++++++++++++++++++-------------------
 builtin/reset.c           |   3 +-
 builtin/sparse-checkout.c |   3 +-
 builtin/stash.c           |   6 +--
 credential.c              |   2 +-
 daemon.c                  |  19 +++----
 diff-lib.c                |   3 +-
 http.c                    |   2 +-
 merge-ort.c               |   3 +-
 merge-recursive.c         |   4 +-
 reset.c                   |   2 +-
 sequencer.c               |   3 +-
 unpack-trees.h            |   1 +
 urlmatch.h                |   4 ++
 23 files changed, 103 insertions(+), 122 deletions(-)

Range-diff against v2:
1:  3130693b416 = 1:  8f3f3f97fcb daemon.c: refactor hostinfo_init() to HOSTINFO_INIT macro
2:  65c5295c1ac = 2:  ced1d581f15 builtin/blame.c: refactor commit_info_init() to COMMIT_INFO_INIT macro
3:  3783788b553 ! 3:  266948e604c urlmatch.[ch]: add and use URLMATCH_CONFIG_INIT
    @@ Commit message
         urlmatch.[ch]: add and use URLMATCH_CONFIG_INIT
     
         Change the initialization pattern of "struct urlmatch_config" to use
    -    an *_INIT macro and designated initializers.
    +    an *_INIT macro and designated initializers. Right now there's no
    +    other "struct" member of "struct urlmatch_config" which would require
    +    its own *_INIT, but it's good practice not to assume that. Let's also
    +    change this to a designated initializer while we're at it.
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
    @@ builtin/config.c: static int get_urlmatch(const char *var, const char *url)
      	struct string_list values = STRING_LIST_INIT_DUP;
      
      	config.collect_fn = urlmatch_collect_fn;
    --	config.cascade_fn = NULL;
    - 	config.cb = &values;
    - 
    - 	if (!url_normalize(url, &config.url))
     
      ## credential.c ##
     @@ credential.c: static int match_partial_url(const char *url, void *cb)
    @@ credential.c: static int match_partial_url(const char *url, void *cb)
      	struct strbuf url = STRBUF_INIT;
      
      	if (!c->host)
    -@@ credential.c: static void credential_apply_config(struct credential *c)
    - 		return;
    - 
    - 	config.section = "credential";
    --	config.key = NULL;
    - 	config.collect_fn = credential_config_callback;
    --	config.cascade_fn = NULL;
    - 	config.select_fn = select_all;
    - 	config.fallback_match_fn = match_partial_url;
    - 	config.cb = c;
     
      ## http.c ##
     @@ http.c: void http_init(struct remote *remote, const char *url, int proactive_auth)
    @@ http.c: void http_init(struct remote *remote, const char *url, int proactive_aut
     +	struct urlmatch_config config = URLMATCH_CONFIG_INIT;
      
      	config.section = "http";
    --	config.key = NULL;
    - 	config.collect_fn = http_options;
    - 	config.cascade_fn = git_default_config;
    --	config.cb = NULL;
    - 
    - 	http_is_verbose = 0;
    - 	normalized_url = url_normalize(url, &config.url);
    + 	config.key = NULL;
     
      ## urlmatch.h ##
     @@ urlmatch.h: struct urlmatch_config {
4:  13ef9566903 ! 4:  41fcb0a45e5 builtin/remote.c: add and use a REF_STATES_INIT
    @@ Commit message
         initialize it in those three places, skip the memset(), and pass those
         structs down appropriately.
     
    +    This would be a behavior change if we had codepaths that relied say on
    +    implicitly having had "new_refs" initialized to STRING_LIST_INIT_NODUP
    +    with the memset(), but only set the "strdup_strings" on some other
    +    struct, but then called string_list_append() on "new_refs". There
    +    isn't any such codepath, all of the late assignments to
    +    "strdup_strings" assigned to those structs that we'd use for those
    +    codepaths.
    +
    +    So just initializing them all up-front makes for easier to understand
    +    code, i.e. in the pre-image it looked as though we had that tricky
    +    edge case, but we didn't.
    +
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## builtin/remote.c ##
5:  b78a9ec0846 = 5:  25fec54877b builtin/remote.c: add and use SHOW_INFO_INIT
-:  ----------- > 6:  18358f5d57a unpack-trees.[ch]: define and use a UNPACK_TREES_OPTIONS_INIT
-- 
2.33.0.1375.gbbd823cc90f

[PATCH v3 1/6] daemon.c: refactor hostinfo_init() to HOSTINFO_INIT macro

[Ævar Arnfjörð Bjarmason]

Remove the hostinfo_init() function added in 01cec54e135 (daemon:
deglobalize hostname information, 2015-03-07) and instead initialize
the "struct hostinfo" with a macro.

This is the more idiomatic pattern in the codebase, and doesn't leave
us wondering when we see the *_init() function if this struct needs
more complex initialization than a macro can provide.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 daemon.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/daemon.c b/daemon.c
index 5c4cbad62d0..d80d009d1a1 100644
--- a/daemon.c
+++ b/daemon.c
@@ -63,6 +63,12 @@ struct hostinfo {
 	unsigned int hostname_lookup_done:1;
 	unsigned int saw_extended_args:1;
 };
+#define HOSTINFO_INIT { \
+	.hostname = STRBUF_INIT, \
+	.canon_hostname = STRBUF_INIT, \
+	.ip_address = STRBUF_INIT, \
+	.tcp_port = STRBUF_INIT, \
+}
 
 static void lookup_hostname(struct hostinfo *hi);
 
@@ -727,15 +733,6 @@ static void lookup_hostname(struct hostinfo *hi)
 	}
 }
 
-static void hostinfo_init(struct hostinfo *hi)
-{
-	memset(hi, 0, sizeof(*hi));
-	strbuf_init(&hi->hostname, 0);
-	strbuf_init(&hi->canon_hostname, 0);
-	strbuf_init(&hi->ip_address, 0);
-	strbuf_init(&hi->tcp_port, 0);
-}
-
 static void hostinfo_clear(struct hostinfo *hi)
 {
 	strbuf_release(&hi->hostname);
@@ -760,11 +757,9 @@ static int execute(void)
 	char *line = packet_buffer;
 	int pktlen, len, i;
 	char *addr = getenv("REMOTE_ADDR"), *port = getenv("REMOTE_PORT");
-	struct hostinfo hi;
+	struct hostinfo hi = HOSTINFO_INIT;
 	struct strvec env = STRVEC_INIT;
 
-	hostinfo_init(&hi);
-
 	if (addr)
 		loginfo("Connection from %s:%s", addr, port);
 
-- 
2.33.0.1375.gbbd823cc90f

[PATCH v3 2/6] builtin/blame.c: refactor commit_info_init() to COMMIT_INFO_INIT macro

[Ævar Arnfjörð Bjarmason]

Remove the commit_info_init() function addded in ea02ffa3857 (mailmap:
simplify map_user() interface, 2013-01-05) and instead initialize the
"struct commit_info" with a macro.

This is the more idiomatic pattern in the codebase, and doesn't leave
us wondering when we see the *_init() function if this struct needs
more complex initialization than a macro can provide.

The get_commit_info() function is only called by the three callers
being changed here immediately after initializing the struct with the
macros, so by moving the initialization to the callers we don't need
to do it in get_commit_info() anymore.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/blame.c | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 641523ff9af..1c31a996403 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -101,6 +101,16 @@ struct commit_info {
 	struct strbuf summary;
 };
 
+#define COMMIT_INFO_INIT { \
+	.author = STRBUF_INIT, \
+	.author_mail = STRBUF_INIT, \
+	.author_tz = STRBUF_INIT, \
+	.committer = STRBUF_INIT, \
+	.committer_mail = STRBUF_INIT, \
+	.committer_tz = STRBUF_INIT, \
+	.summary = STRBUF_INIT, \
+}
+
 /*
  * Parse author/committer line in the commit object buffer
  */
@@ -160,18 +170,6 @@ static void get_ac_line(const char *inbuf, const char *what,
 	strbuf_add(name, namebuf, namelen);
 }
 
-static void commit_info_init(struct commit_info *ci)
-{
-
-	strbuf_init(&ci->author, 0);
-	strbuf_init(&ci->author_mail, 0);
-	strbuf_init(&ci->author_tz, 0);
-	strbuf_init(&ci->committer, 0);
-	strbuf_init(&ci->committer_mail, 0);
-	strbuf_init(&ci->committer_tz, 0);
-	strbuf_init(&ci->summary, 0);
-}
-
 static void commit_info_destroy(struct commit_info *ci)
 {
 
@@ -192,8 +190,6 @@ static void get_commit_info(struct commit *commit,
 	const char *subject, *encoding;
 	const char *message;
 
-	commit_info_init(ret);
-
 	encoding = get_log_output_encoding();
 	message = logmsg_reencode(commit, NULL, encoding);
 	get_ac_line(message, "\nauthor ",
@@ -246,7 +242,7 @@ static void write_filename_info(struct blame_origin *suspect)
  */
 static int emit_one_suspect_detail(struct blame_origin *suspect, int repeat)
 {
-	struct commit_info ci;
+	struct commit_info ci = COMMIT_INFO_INIT;
 
 	if (!repeat && (suspect->commit->object.flags & METAINFO_SHOWN))
 		return 0;
@@ -440,7 +436,7 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int
 	int cnt;
 	const char *cp;
 	struct blame_origin *suspect = ent->suspect;
-	struct commit_info ci;
+	struct commit_info ci = COMMIT_INFO_INIT;
 	char hex[GIT_MAX_HEXSZ + 1];
 	int show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP);
 	const char *default_color = NULL, *color = NULL, *reset = NULL;
@@ -630,7 +626,7 @@ static void find_alignment(struct blame_scoreboard *sb, int *option)
 		if (longest_file < num)
 			longest_file = num;
 		if (!(suspect->commit->object.flags & METAINFO_SHOWN)) {
-			struct commit_info ci;
+			struct commit_info ci = COMMIT_INFO_INIT;
 			suspect->commit->object.flags |= METAINFO_SHOWN;
 			get_commit_info(suspect->commit, &ci, 1);
 			if (*option & OUTPUT_SHOW_EMAIL)
-- 
2.33.0.1375.gbbd823cc90f

[PATCH v3 3/6] urlmatch.[ch]: add and use URLMATCH_CONFIG_INIT

[Ævar Arnfjörð Bjarmason]

Change the initialization pattern of "struct urlmatch_config" to use
an *_INIT macro and designated initializers. Right now there's no
other "struct" member of "struct urlmatch_config" which would require
its own *_INIT, but it's good practice not to assume that. Let's also
change this to a designated initializer while we're at it.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/config.c | 2 +-
 credential.c     | 2 +-
 http.c           | 2 +-
 urlmatch.h       | 4 ++++
 4 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 865fddd6ce8..542d8d02b2b 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -575,7 +575,7 @@ static int get_urlmatch(const char *var, const char *url)
 	int ret;
 	char *section_tail;
 	struct string_list_item *item;
-	struct urlmatch_config config = { STRING_LIST_INIT_DUP };
+	struct urlmatch_config config = URLMATCH_CONFIG_INIT;
 	struct string_list values = STRING_LIST_INIT_DUP;
 
 	config.collect_fn = urlmatch_collect_fn;
diff --git a/credential.c b/credential.c
index 000ac7a8d43..e7240f3f636 100644
--- a/credential.c
+++ b/credential.c
@@ -105,7 +105,7 @@ static int match_partial_url(const char *url, void *cb)
 static void credential_apply_config(struct credential *c)
 {
 	char *normalized_url;
-	struct urlmatch_config config = { STRING_LIST_INIT_DUP };
+	struct urlmatch_config config = URLMATCH_CONFIG_INIT;
 	struct strbuf url = STRBUF_INIT;
 
 	if (!c->host)
diff --git a/http.c b/http.c
index d7c20493d7f..da12471c242 100644
--- a/http.c
+++ b/http.c
@@ -990,7 +990,7 @@ void http_init(struct remote *remote, const char *url, int proactive_auth)
 	char *low_speed_limit;
 	char *low_speed_time;
 	char *normalized_url;
-	struct urlmatch_config config = { STRING_LIST_INIT_DUP };
+	struct urlmatch_config config = URLMATCH_CONFIG_INIT;
 
 	config.section = "http";
 	config.key = NULL;
diff --git a/urlmatch.h b/urlmatch.h
index 6ff42f81b0c..34a3ba6d197 100644
--- a/urlmatch.h
+++ b/urlmatch.h
@@ -66,6 +66,10 @@ struct urlmatch_config {
 	int (*fallback_match_fn)(const char *url, void *cb);
 };
 
+#define URLMATCH_CONFIG_INIT { \
+	.vars = STRING_LIST_INIT_DUP, \
+}
+
 int urlmatch_config_entry(const char *var, const char *value, void *cb);
 
 #endif /* URL_MATCH_H */
-- 
2.33.0.1375.gbbd823cc90f

[PATCH v3 4/6] builtin/remote.c: add and use a REF_STATES_INIT

[Ævar Arnfjörð Bjarmason]

Use a new REF_STATES_INIT designated initializer instead of assigning
to the "strdup_strings" member of the previously memzero()'d version
of this struct.

The pattern of assigning to "strdup_strings" dates back to
211c89682ee (Make git-remote a builtin, 2008-02-29) (when it was
"strdup_paths"), i.e. long before we used anything like our current
established *_INIT patterns consistently.

Then in e61e0cc6b70 (builtin-remote: teach show to display remote
HEAD, 2009-02-25) and e5dcbfd9ab7 (builtin-remote: new show output
style for push refspecs, 2009-02-25) we added some more of these.

As it turns out we only initialized this struct three times, all the
other uses were of pointers to those initialized structs. So let's
initialize it in those three places, skip the memset(), and pass those
structs down appropriately.

This would be a behavior change if we had codepaths that relied say on
implicitly having had "new_refs" initialized to STRING_LIST_INIT_NODUP
with the memset(), but only set the "strdup_strings" on some other
struct, but then called string_list_append() on "new_refs". There
isn't any such codepath, all of the late assignments to
"strdup_strings" assigned to those structs that we'd use for those
codepaths.

So just initializing them all up-front makes for easier to understand
code, i.e. in the pre-image it looked as though we had that tricky
edge case, but we didn't.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/remote.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/builtin/remote.c b/builtin/remote.c
index 7f88e6ce9de..160dd954f74 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -344,6 +344,14 @@ struct ref_states {
 	int queried;
 };
 
+#define REF_STATES_INIT { \
+	.new_refs = STRING_LIST_INIT_DUP, \
+	.stale = STRING_LIST_INIT_DUP, \
+	.tracked = STRING_LIST_INIT_DUP, \
+	.heads = STRING_LIST_INIT_DUP, \
+	.push = STRING_LIST_INIT_DUP, \
+}
+
 static int get_ref_states(const struct ref *remote_refs, struct ref_states *states)
 {
 	struct ref *fetch_map = NULL, **tail = &fetch_map;
@@ -355,9 +363,6 @@ static int get_ref_states(const struct ref *remote_refs, struct ref_states *stat
 			die(_("Could not get fetch map for refspec %s"),
 				states->remote->fetch.raw[i]);
 
-	states->new_refs.strdup_strings = 1;
-	states->tracked.strdup_strings = 1;
-	states->stale.strdup_strings = 1;
 	for (ref = fetch_map; ref; ref = ref->next) {
 		if (!ref->peer_ref || !ref_exists(ref->peer_ref->name))
 			string_list_append(&states->new_refs, abbrev_branch(ref->name));
@@ -406,7 +411,6 @@ static int get_push_ref_states(const struct ref *remote_refs,
 
 	match_push_refs(local_refs, &push_map, &remote->push, MATCH_REFS_NONE);
 
-	states->push.strdup_strings = 1;
 	for (ref = push_map; ref; ref = ref->next) {
 		struct string_list_item *item;
 		struct push_info *info;
@@ -449,7 +453,6 @@ static int get_push_ref_states_noquery(struct ref_states *states)
 	if (remote->mirror)
 		return 0;
 
-	states->push.strdup_strings = 1;
 	if (!remote->push.nr) {
 		item = string_list_append(&states->push, _("(matching)"));
 		info = item->util = xcalloc(1, sizeof(struct push_info));
@@ -483,7 +486,6 @@ static int get_head_names(const struct ref *remote_refs, struct ref_states *stat
 	refspec.force = 0;
 	refspec.pattern = 1;
 	refspec.src = refspec.dst = "refs/heads/*";
-	states->heads.strdup_strings = 1;
 	get_fetch_map(remote_refs, &refspec, &fetch_map_tail, 0);
 	matches = guess_remote_head(find_ref_by_name(remote_refs, "HEAD"),
 				    fetch_map, 1);
@@ -1212,7 +1214,7 @@ static int show(int argc, const char **argv)
 		OPT_BOOL('n', NULL, &no_query, N_("do not query remotes")),
 		OPT_END()
 	};
-	struct ref_states states;
+	struct ref_states states = REF_STATES_INIT;
 	struct string_list info_list = STRING_LIST_INIT_NODUP;
 	struct show_info info;
 
@@ -1225,7 +1227,6 @@ static int show(int argc, const char **argv)
 	if (!no_query)
 		query_flag = (GET_REF_STATES | GET_HEAD_NAMES | GET_PUSH_REF_STATES);
 
-	memset(&states, 0, sizeof(states));
 	memset(&info, 0, sizeof(info));
 	info.states = &states;
 	info.list = &info_list;
@@ -1334,8 +1335,7 @@ static int set_head(int argc, const char **argv)
 	if (!opt_a && !opt_d && argc == 2) {
 		head_name = xstrdup(argv[1]);
 	} else if (opt_a && !opt_d && argc == 1) {
-		struct ref_states states;
-		memset(&states, 0, sizeof(states));
+		struct ref_states states = REF_STATES_INIT;
 		get_remote_ref_states(argv[0], &states, GET_HEAD_NAMES);
 		if (!states.heads.nr)
 			result |= error(_("Cannot determine remote HEAD"));
@@ -1374,14 +1374,13 @@ static int set_head(int argc, const char **argv)
 static int prune_remote(const char *remote, int dry_run)
 {
 	int result = 0;
-	struct ref_states states;
+	struct ref_states states = REF_STATES_INIT;
 	struct string_list refs_to_prune = STRING_LIST_INIT_NODUP;
 	struct string_list_item *item;
 	const char *dangling_msg = dry_run
 		? _(" %s will become dangling!")
 		: _(" %s has become dangling!");
 
-	memset(&states, 0, sizeof(states));
 	get_remote_ref_states(remote, &states, GET_REF_STATES);
 
 	if (!states.stale.nr) {
-- 
2.33.0.1375.gbbd823cc90f

[PATCH v3 5/6] builtin/remote.c: add and use SHOW_INFO_INIT

[Ævar Arnfjörð Bjarmason]

In the preceding commit we introduced REF_STATES_INIT, but did not
change the "struct show_info" to have a corresponding
initializer. Let's do that, and make it use "REF_STATES_INIT" and
"STRING_LIST_INIT_DUP", doing that requires changing "list" and
"states" away from being pointers.

The resulting end-state is simpler since we omit the local "info_list"
and "states" variables in show() as well as the memset().

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/remote.c | 90 ++++++++++++++++++++++++------------------------
 1 file changed, 45 insertions(+), 45 deletions(-)

diff --git a/builtin/remote.c b/builtin/remote.c
index 160dd954f74..deb48772ac5 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -972,26 +972,31 @@ static int get_remote_ref_states(const char *name,
 }
 
 struct show_info {
-	struct string_list *list;
-	struct ref_states *states;
+	struct string_list list;
+	struct ref_states states;
 	int width, width2;
 	int any_rebase;
 };
 
+#define SHOW_INFO_INIT { \
+	.list = STRING_LIST_INIT_DUP, \
+	.states = REF_STATES_INIT, \
+}
+
 static int add_remote_to_show_info(struct string_list_item *item, void *cb_data)
 {
 	struct show_info *info = cb_data;
 	int n = strlen(item->string);
 	if (n > info->width)
 		info->width = n;
-	string_list_insert(info->list, item->string);
+	string_list_insert(&info->list, item->string);
 	return 0;
 }
 
 static int show_remote_info_item(struct string_list_item *item, void *cb_data)
 {
 	struct show_info *info = cb_data;
-	struct ref_states *states = info->states;
+	struct ref_states *states = &info->states;
 	const char *name = item->string;
 
 	if (states->queried) {
@@ -1018,7 +1023,7 @@ static int show_remote_info_item(struct string_list_item *item, void *cb_data)
 static int add_local_to_show_info(struct string_list_item *branch_item, void *cb_data)
 {
 	struct show_info *show_info = cb_data;
-	struct ref_states *states = show_info->states;
+	struct ref_states *states = &show_info->states;
 	struct branch_info *branch_info = branch_item->util;
 	struct string_list_item *item;
 	int n;
@@ -1031,7 +1036,7 @@ static int add_local_to_show_info(struct string_list_item *branch_item, void *cb
 	if (branch_info->rebase >= REBASE_TRUE)
 		show_info->any_rebase = 1;
 
-	item = string_list_insert(show_info->list, branch_item->string);
+	item = string_list_insert(&show_info->list, branch_item->string);
 	item->util = branch_info;
 
 	return 0;
@@ -1086,7 +1091,7 @@ static int add_push_to_show_info(struct string_list_item *push_item, void *cb_da
 		show_info->width = n;
 	if ((n = strlen(push_info->dest)) > show_info->width2)
 		show_info->width2 = n;
-	item = string_list_append(show_info->list, push_item->string);
+	item = string_list_append(&show_info->list, push_item->string);
 	item->util = push_item->util;
 	return 0;
 }
@@ -1214,9 +1219,7 @@ static int show(int argc, const char **argv)
 		OPT_BOOL('n', NULL, &no_query, N_("do not query remotes")),
 		OPT_END()
 	};
-	struct ref_states states = REF_STATES_INIT;
-	struct string_list info_list = STRING_LIST_INIT_NODUP;
-	struct show_info info;
+	struct show_info info = SHOW_INFO_INIT;
 
 	argc = parse_options(argc, argv, NULL, options, builtin_remote_show_usage,
 			     0);
@@ -1227,25 +1230,22 @@ static int show(int argc, const char **argv)
 	if (!no_query)
 		query_flag = (GET_REF_STATES | GET_HEAD_NAMES | GET_PUSH_REF_STATES);
 
-	memset(&info, 0, sizeof(info));
-	info.states = &states;
-	info.list = &info_list;
 	for (; argc; argc--, argv++) {
 		int i;
 		const char **url;
 		int url_nr;
 
-		get_remote_ref_states(*argv, &states, query_flag);
+		get_remote_ref_states(*argv, &info.states, query_flag);
 
 		printf_ln(_("* remote %s"), *argv);
-		printf_ln(_("  Fetch URL: %s"), states.remote->url_nr > 0 ?
-		       states.remote->url[0] : _("(no URL)"));
-		if (states.remote->pushurl_nr) {
-			url = states.remote->pushurl;
-			url_nr = states.remote->pushurl_nr;
+		printf_ln(_("  Fetch URL: %s"), info.states.remote->url_nr > 0 ?
+		       info.states.remote->url[0] : _("(no URL)"));
+		if (info.states.remote->pushurl_nr) {
+			url = info.states.remote->pushurl;
+			url_nr = info.states.remote->pushurl_nr;
 		} else {
-			url = states.remote->url;
-			url_nr = states.remote->url_nr;
+			url = info.states.remote->url;
+			url_nr = info.states.remote->url_nr;
 		}
 		for (i = 0; i < url_nr; i++)
 			/*
@@ -1258,57 +1258,57 @@ static int show(int argc, const char **argv)
 			printf_ln(_("  Push  URL: %s"), _("(no URL)"));
 		if (no_query)
 			printf_ln(_("  HEAD branch: %s"), _("(not queried)"));
-		else if (!states.heads.nr)
+		else if (!info.states.heads.nr)
 			printf_ln(_("  HEAD branch: %s"), _("(unknown)"));
-		else if (states.heads.nr == 1)
-			printf_ln(_("  HEAD branch: %s"), states.heads.items[0].string);
+		else if (info.states.heads.nr == 1)
+			printf_ln(_("  HEAD branch: %s"), info.states.heads.items[0].string);
 		else {
 			printf(_("  HEAD branch (remote HEAD is ambiguous,"
 				 " may be one of the following):\n"));
-			for (i = 0; i < states.heads.nr; i++)
-				printf("    %s\n", states.heads.items[i].string);
+			for (i = 0; i < info.states.heads.nr; i++)
+				printf("    %s\n", info.states.heads.items[i].string);
 		}
 
 		/* remote branch info */
 		info.width = 0;
-		for_each_string_list(&states.new_refs, add_remote_to_show_info, &info);
-		for_each_string_list(&states.tracked, add_remote_to_show_info, &info);
-		for_each_string_list(&states.stale, add_remote_to_show_info, &info);
-		if (info.list->nr)
+		for_each_string_list(&info.states.new_refs, add_remote_to_show_info, &info);
+		for_each_string_list(&info.states.tracked, add_remote_to_show_info, &info);
+		for_each_string_list(&info.states.stale, add_remote_to_show_info, &info);
+		if (info.list.nr)
 			printf_ln(Q_("  Remote branch:%s",
 				     "  Remote branches:%s",
-				     info.list->nr),
+				     info.list.nr),
 				  no_query ? _(" (status not queried)") : "");
-		for_each_string_list(info.list, show_remote_info_item, &info);
-		string_list_clear(info.list, 0);
+		for_each_string_list(&info.list, show_remote_info_item, &info);
+		string_list_clear(&info.list, 0);
 
 		/* git pull info */
 		info.width = 0;
 		info.any_rebase = 0;
 		for_each_string_list(&branch_list, add_local_to_show_info, &info);
-		if (info.list->nr)
+		if (info.list.nr)
 			printf_ln(Q_("  Local branch configured for 'git pull':",
 				     "  Local branches configured for 'git pull':",
-				     info.list->nr));
-		for_each_string_list(info.list, show_local_info_item, &info);
-		string_list_clear(info.list, 0);
+				     info.list.nr));
+		for_each_string_list(&info.list, show_local_info_item, &info);
+		string_list_clear(&info.list, 0);
 
 		/* git push info */
-		if (states.remote->mirror)
+		if (info.states.remote->mirror)
 			printf_ln(_("  Local refs will be mirrored by 'git push'"));
 
 		info.width = info.width2 = 0;
-		for_each_string_list(&states.push, add_push_to_show_info, &info);
-		QSORT(info.list->items, info.list->nr, cmp_string_with_push);
-		if (info.list->nr)
+		for_each_string_list(&info.states.push, add_push_to_show_info, &info);
+		QSORT(info.list.items, info.list.nr, cmp_string_with_push);
+		if (info.list.nr)
 			printf_ln(Q_("  Local ref configured for 'git push'%s:",
 				     "  Local refs configured for 'git push'%s:",
-				     info.list->nr),
+				     info.list.nr),
 				  no_query ? _(" (status not queried)") : "");
-		for_each_string_list(info.list, show_push_info_item, &info);
-		string_list_clear(info.list, 0);
+		for_each_string_list(&info.list, show_push_info_item, &info);
+		string_list_clear(&info.list, 0);
 
-		free_remote_ref_states(&states);
+		free_remote_ref_states(&info.states);
 	}
 
 	return result;
-- 
2.33.0.1375.gbbd823cc90f

[PATCH v3 6/6] unpack-trees.[ch]: define and use a UNPACK_TREES_OPTIONS_INIT

[Ævar Arnfjörð Bjarmason]

Almost all users of "struct unpack_trees_options" allocate it on the
stack already, let's provide an *_INIT macro for them to use. This
will make later changes that would like to initialize other members of
the struct easier[1][2], but for now we're keeping it compatible with
a memset() to "0" with an "{ 0 }" initialization.

This leaves a caller in "checkout_fast_forward()" in "merge.c" still
doing the memset(). I've left it to avoid a conflict with the
in-flight en/removing-untracked-fixes. As noted above conflict is just
textual, not semantic, so we can clean it up some other time. I've
manually validated[3] with some WIP changes on top of this that the
remaining "checkout_fast_forward()" caller is the only one that.s
left.

1. https://lore.kernel.org/git/87k0ixrv23.fsf@evledraar.gmail.com/
2. https://lore.kernel.org/git/87fstlrumj.fsf@evledraar.gmail.com/
3. https://lore.kernel.org/git/877dexrqvg.fsf@evledraar.gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 archive.c                 | 3 +--
 builtin/am.c              | 6 ++----
 builtin/checkout.c        | 6 ++----
 builtin/clone.c           | 3 +--
 builtin/commit.c          | 3 +--
 builtin/merge.c           | 3 +--
 builtin/read-tree.c       | 3 +--
 builtin/reset.c           | 3 +--
 builtin/sparse-checkout.c | 3 +--
 builtin/stash.c           | 6 ++----
 diff-lib.c                | 3 +--
 merge-ort.c               | 3 +--
 merge-recursive.c         | 4 +++-
 reset.c                   | 2 +-
 sequencer.c               | 3 +--
 unpack-trees.h            | 1 +
 16 files changed, 21 insertions(+), 34 deletions(-)

diff --git a/archive.c b/archive.c
index a3bbb091256..210d7235c5a 100644
--- a/archive.c
+++ b/archive.c
@@ -269,7 +269,7 @@ int write_archive_entries(struct archiver_args *args,
 		write_archive_entry_fn_t write_entry)
 {
 	struct archiver_context context;
-	struct unpack_trees_options opts;
+	struct unpack_trees_options opts = UNPACK_TREES_OPTIONS_INIT;
 	struct tree_desc t;
 	int err;
 	struct strbuf path_in_archive = STRBUF_INIT;
@@ -300,7 +300,6 @@ int write_archive_entries(struct archiver_args *args,
 	 * Setup index and instruct attr to read index only
 	 */
 	if (!args->worktree_attributes) {
-		memset(&opts, 0, sizeof(opts));
 		opts.index_only = 1;
 		opts.head_idx = -1;
 		opts.src_index = args->repo->index;
diff --git a/builtin/am.c b/builtin/am.c
index e4a0ff9cd7c..82641ce68ec 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1901,7 +1901,7 @@ static void am_resolve(struct am_state *state)
 static int fast_forward_to(struct tree *head, struct tree *remote, int reset)
 {
 	struct lock_file lock_file = LOCK_INIT;
-	struct unpack_trees_options opts;
+	struct unpack_trees_options opts = UNPACK_TREES_OPTIONS_INIT;
 	struct tree_desc t[2];
 
 	if (parse_tree(head) || parse_tree(remote))
@@ -1911,7 +1911,6 @@ static int fast_forward_to(struct tree *head, struct tree *remote, int reset)
 
 	refresh_cache(REFRESH_QUIET);
 
-	memset(&opts, 0, sizeof(opts));
 	opts.head_idx = 1;
 	opts.src_index = &the_index;
 	opts.dst_index = &the_index;
@@ -1940,7 +1939,7 @@ static int fast_forward_to(struct tree *head, struct tree *remote, int reset)
 static int merge_tree(struct tree *tree)
 {
 	struct lock_file lock_file = LOCK_INIT;
-	struct unpack_trees_options opts;
+	struct unpack_trees_options opts = UNPACK_TREES_OPTIONS_INIT;
 	struct tree_desc t[1];
 
 	if (parse_tree(tree))
@@ -1948,7 +1947,6 @@ static int merge_tree(struct tree *tree)
 
 	hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);
 
-	memset(&opts, 0, sizeof(opts));
 	opts.head_idx = 1;
 	opts.src_index = &the_index;
 	opts.dst_index = &the_index;
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 8c69dcdf72a..fea4533dbec 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -639,10 +639,9 @@ static int reset_tree(struct tree *tree, const struct checkout_opts *o,
 		      int worktree, int *writeout_error,
 		      struct branch_info *info)
 {
-	struct unpack_trees_options opts;
+	struct unpack_trees_options opts = UNPACK_TREES_OPTIONS_INIT;
 	struct tree_desc tree_desc;
 
-	memset(&opts, 0, sizeof(opts));
 	opts.head_idx = -1;
 	opts.update = worktree;
 	opts.skip_unmerged = !worktree;
@@ -719,9 +718,8 @@ static int merge_working_tree(const struct checkout_opts *opts,
 	} else {
 		struct tree_desc trees[2];
 		struct tree *tree;
-		struct unpack_trees_options topts;
+		struct unpack_trees_options topts = UNPACK_TREES_OPTIONS_INIT;
 
-		memset(&topts, 0, sizeof(topts));
 		topts.head_idx = -1;
 		topts.src_index = &the_index;
 		topts.dst_index = &the_index;
diff --git a/builtin/clone.c b/builtin/clone.c
index ff1d3d447a3..0df820c5970 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -655,7 +655,7 @@ static int checkout(int submodule_progress)
 	struct object_id oid;
 	char *head;
 	struct lock_file lock_file = LOCK_INIT;
-	struct unpack_trees_options opts;
+	struct unpack_trees_options opts = UNPACK_TREES_OPTIONS_INIT;
 	struct tree *tree;
 	struct tree_desc t;
 	int err = 0;
@@ -683,7 +683,6 @@ static int checkout(int submodule_progress)
 
 	hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);
 
-	memset(&opts, 0, sizeof opts);
 	opts.update = 1;
 	opts.merge = 1;
 	opts.clone = 1;
diff --git a/builtin/commit.c b/builtin/commit.c
index e7320f66f95..6cc7313bad8 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -303,7 +303,7 @@ static void add_remove_files(struct string_list *list)
 static void create_base_index(const struct commit *current_head)
 {
 	struct tree *tree;
-	struct unpack_trees_options opts;
+	struct unpack_trees_options opts = UNPACK_TREES_OPTIONS_INIT;
 	struct tree_desc t;
 
 	if (!current_head) {
@@ -311,7 +311,6 @@ static void create_base_index(const struct commit *current_head)
 		return;
 	}
 
-	memset(&opts, 0, sizeof(opts));
 	opts.head_idx = 1;
 	opts.index_only = 1;
 	opts.merge = 1;
diff --git a/builtin/merge.c b/builtin/merge.c
index 3fbdacc7db4..73290a07fcc 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -670,9 +670,8 @@ static int read_tree_trivial(struct object_id *common, struct object_id *head,
 	int i, nr_trees = 0;
 	struct tree *trees[MAX_UNPACK_TREES];
 	struct tree_desc t[MAX_UNPACK_TREES];
-	struct unpack_trees_options opts;
+	struct unpack_trees_options opts = UNPACK_TREES_OPTIONS_INIT;
 
-	memset(&opts, 0, sizeof(opts));
 	opts.head_idx = 2;
 	opts.src_index = &the_index;
 	opts.dst_index = &the_index;
diff --git a/builtin/read-tree.c b/builtin/read-tree.c
index 485e7b04794..847182fdad6 100644
--- a/builtin/read-tree.c
+++ b/builtin/read-tree.c
@@ -116,7 +116,7 @@ int cmd_read_tree(int argc, const char **argv, const char *cmd_prefix)
 	int i, stage = 0;
 	struct object_id oid;
 	struct tree_desc t[MAX_UNPACK_TREES];
-	struct unpack_trees_options opts;
+	struct unpack_trees_options opts = UNPACK_TREES_OPTIONS_INIT;
 	int prefix_set = 0;
 	struct lock_file lock_file = LOCK_INIT;
 	const struct option read_tree_options[] = {
@@ -158,7 +158,6 @@ int cmd_read_tree(int argc, const char **argv, const char *cmd_prefix)
 		OPT_END()
 	};
 
-	memset(&opts, 0, sizeof(opts));
 	opts.head_idx = -1;
 	opts.src_index = &the_index;
 	opts.dst_index = &the_index;
diff --git a/builtin/reset.c b/builtin/reset.c
index 51c9e2f43ff..86c604b21e9 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -51,10 +51,9 @@ static int reset_index(const char *ref, const struct object_id *oid, int reset_t
 	int i, nr = 0;
 	struct tree_desc desc[2];
 	struct tree *tree;
-	struct unpack_trees_options opts;
+	struct unpack_trees_options opts = UNPACK_TREES_OPTIONS_INIT;
 	int ret = -1;
 
-	memset(&opts, 0, sizeof(opts));
 	opts.head_idx = 1;
 	opts.src_index = &the_index;
 	opts.dst_index = &the_index;
diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index d0f5c4702be..4c3c29fb580 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -195,7 +195,7 @@ static void clean_tracked_sparse_directories(struct repository *r)
 static int update_working_directory(struct pattern_list *pl)
 {
 	enum update_sparsity_result result;
-	struct unpack_trees_options o;
+	struct unpack_trees_options o = UNPACK_TREES_OPTIONS_INIT;
 	struct lock_file lock_file = LOCK_INIT;
 	struct repository *r = the_repository;
 
@@ -205,7 +205,6 @@ static int update_working_directory(struct pattern_list *pl)
 
 	r->index->sparse_checkout_patterns = pl;
 
-	memset(&o, 0, sizeof(o));
 	o.verbose_update = isatty(2);
 	o.update = 1;
 	o.head_idx = -1;
diff --git a/builtin/stash.c b/builtin/stash.c
index 8f42360ca91..1137e5fcbe8 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -233,7 +233,7 @@ static int clear_stash(int argc, const char **argv, const char *prefix)
 static int reset_tree(struct object_id *i_tree, int update, int reset)
 {
 	int nr_trees = 1;
-	struct unpack_trees_options opts;
+	struct unpack_trees_options opts = UNPACK_TREES_OPTIONS_INIT;
 	struct tree_desc t[MAX_UNPACK_TREES];
 	struct tree *tree;
 	struct lock_file lock_file = LOCK_INIT;
@@ -244,8 +244,6 @@ static int reset_tree(struct object_id *i_tree, int update, int reset)
 
 	hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);
 
-	memset(&opts, 0, sizeof(opts));
-
 	tree = parse_tree_indirect(i_tree);
 	if (parse_tree(tree))
 		return -1;
@@ -799,7 +797,7 @@ static void diff_include_untracked(const struct stash_info *info, struct diff_op
 	const struct object_id *oid[] = { &info->w_commit, &info->u_tree };
 	struct tree *tree[ARRAY_SIZE(oid)];
 	struct tree_desc tree_desc[ARRAY_SIZE(oid)];
-	struct unpack_trees_options unpack_tree_opt = { 0 };
+	struct unpack_trees_options unpack_tree_opt = UNPACK_TREES_OPTIONS_INIT;
 	int i;
 
 	for (i = 0; i < ARRAY_SIZE(oid); i++) {
diff --git a/diff-lib.c b/diff-lib.c
index ca085a03efc..8a08d9af4eb 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -526,13 +526,12 @@ static int diff_cache(struct rev_info *revs,
 {
 	struct tree *tree;
 	struct tree_desc t;
-	struct unpack_trees_options opts;
+	struct unpack_trees_options opts = UNPACK_TREES_OPTIONS_INIT;
 
 	tree = parse_tree_indirect(tree_oid);
 	if (!tree)
 		return error("bad tree object %s",
 			     tree_name ? tree_name : oid_to_hex(tree_oid));
-	memset(&opts, 0, sizeof(opts));
 	opts.head_idx = 1;
 	opts.index_only = cached;
 	opts.diff_index_cached = (cached &&
diff --git a/merge-ort.c b/merge-ort.c
index 35aa979c3a4..75d2b8e4b99 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -4021,9 +4021,8 @@ static int checkout(struct merge_options *opt,
 	/* Switch the index/working copy from old to new */
 	int ret;
 	struct tree_desc trees[2];
-	struct unpack_trees_options unpack_opts;
+	struct unpack_trees_options unpack_opts = UNPACK_TREES_OPTIONS_INIT;
 
-	memset(&unpack_opts, 0, sizeof(unpack_opts));
 	unpack_opts.head_idx = -1;
 	unpack_opts.src_index = opt->repo->index;
 	unpack_opts.dst_index = opt->repo->index;
diff --git a/merge-recursive.c b/merge-recursive.c
index e594d4c3fa1..40254ec0b3d 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -404,8 +404,10 @@ static int unpack_trees_start(struct merge_options *opt,
 	int rc;
 	struct tree_desc t[3];
 	struct index_state tmp_index = { NULL };
+	struct unpack_trees_options blank = UNPACK_TREES_OPTIONS_INIT;
 
-	memset(&opt->priv->unpack_opts, 0, sizeof(opt->priv->unpack_opts));
+	assert(sizeof(blank) == sizeof(opt->priv->unpack_opts));
+	memcpy(&opt->priv->unpack_opts, &blank, sizeof(opt->priv->unpack_opts));
 	if (opt->priv->call_depth)
 		opt->priv->unpack_opts.index_only = 1;
 	else
diff --git a/reset.c b/reset.c
index 79310ae071b..d13984ab781 100644
--- a/reset.c
+++ b/reset.c
@@ -21,7 +21,7 @@ int reset_head(struct repository *r, struct object_id *oid, const char *action,
 	struct object_id head_oid;
 	struct tree_desc desc[2] = { { NULL }, { NULL } };
 	struct lock_file lock = LOCK_INIT;
-	struct unpack_trees_options unpack_tree_opts = { 0 };
+	struct unpack_trees_options unpack_tree_opts = UNPACK_TREES_OPTIONS_INIT;
 	struct tree *tree;
 	const char *reflog_action;
 	struct strbuf msg = STRBUF_INIT;
diff --git a/sequencer.c b/sequencer.c
index 614d56f5e21..abd85b6c562 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3652,7 +3652,7 @@ static int do_reset(struct repository *r,
 	struct lock_file lock = LOCK_INIT;
 	struct tree_desc desc;
 	struct tree *tree;
-	struct unpack_trees_options unpack_tree_opts;
+	struct unpack_trees_options unpack_tree_opts = UNPACK_TREES_OPTIONS_INIT;
 	int ret = 0;
 
 	if (repo_hold_locked_index(r, &lock, LOCK_REPORT_ON_ERROR) < 0)
@@ -3691,7 +3691,6 @@ static int do_reset(struct repository *r,
 		}
 	}
 
-	memset(&unpack_tree_opts, 0, sizeof(unpack_tree_opts));
 	setup_unpack_trees_porcelain(&unpack_tree_opts, "reset");
 	unpack_tree_opts.head_idx = 1;
 	unpack_tree_opts.src_index = r->index;
diff --git a/unpack-trees.h b/unpack-trees.h
index 2d88b19dca7..caa7ada59ab 100644
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -90,6 +90,7 @@ struct unpack_trees_options {
 	struct pattern_list *pl; /* for internal use */
 	struct checkout_metadata meta;
 };
+#define UNPACK_TREES_OPTIONS_INIT { 0 }
 
 int unpack_trees(unsigned n, struct tree_desc *t,
 		 struct unpack_trees_options *options);
-- 
2.33.0.1375.gbbd823cc90f

Re: [PATCH v3 6/6] unpack-trees.[ch]: define and use a UNPACK_TREES_OPTIONS_INIT

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> diff --git a/archive.c b/archive.c
> index a3bbb091256..210d7235c5a 100644
> --- a/archive.c
> +++ b/archive.c
> @@ -269,7 +269,7 @@ int write_archive_entries(struct archiver_args *args,
>  		write_archive_entry_fn_t write_entry)
>  {
>  	struct archiver_context context;
> -	struct unpack_trees_options opts;
> +	struct unpack_trees_options opts = UNPACK_TREES_OPTIONS_INIT;
>  	struct tree_desc t;
>  	int err;
>  	struct strbuf path_in_archive = STRBUF_INIT;
> @@ -300,7 +300,6 @@ int write_archive_entries(struct archiver_args *args,
>  	 * Setup index and instruct attr to read index only
>  	 */
>  	if (!args->worktree_attributes) {
> -		memset(&opts, 0, sizeof(opts));
>  		opts.index_only = 1;
>  		opts.head_idx = -1;
>  		opts.src_index = args->repo->index;

These two hunks almost makes me say "Meh" (I am adding this
parenthetical comment after reading the patch thru and I am not yet
convinced otherwise).

The reason why we had memset(0), getting rid of which I have no
problem about, is because we didn't prepare the members of the
structure using initializer in the first place, no?  The "opts" is
used only in this block, so instead of these lines, we could have

	if (!args->worktree_attributes) {
		struct unpack_trees_options opts = {
			.index_only = 1,
                        .head_idx = -1,
		        ...
			.fn = oneway_merge;
		};
		init_tree_desc(...);
		if (unpack_trees(1, &t, &opts))
			...

which would be much easier to understand.  We can get rid of
memset(0), having UNPACK_TREES_OPTIONS_INIT did not help us much.

Some hunks show that we have code between the location where "opts"
is declared and where we know for certain that "opts" needs to be
actually used and with what values in its members, like ...

> diff --git a/builtin/am.c b/builtin/am.c
> index e4a0ff9cd7c..82641ce68ec 100644
> --- a/builtin/am.c
> +++ b/builtin/am.c
> @@ -1901,7 +1901,7 @@ static void am_resolve(struct am_state *state)
>  static int fast_forward_to(struct tree *head, struct tree *remote, int reset)
>  {
>  	struct lock_file lock_file = LOCK_INIT;
> -	struct unpack_trees_options opts;
> +	struct unpack_trees_options opts = UNPACK_TREES_OPTIONS_INIT;
>  	struct tree_desc t[2];
>  
>  	if (parse_tree(head) || parse_tree(remote))
> ...
>  
>  	refresh_cache(REFRESH_QUIET);
>  
> -	memset(&opts, 0, sizeof(opts));
>  	opts.head_idx = 1;
>  	opts.src_index = &the_index;
>  	opts.dst_index = &the_index;

... this one.  And it is not necessarily a good idea to initialize
"opts" with the actual values we'd use, and _INIT does help us
getting rid of memset(0);

But many in this patch are much simpler like this one:

> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index 8c69dcdf72a..fea4533dbec 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -639,10 +639,9 @@ static int reset_tree(struct tree *tree, const struct checkout_opts *o,
>  		      int worktree, int *writeout_error,
>  		      struct branch_info *info)
>  {
> -	struct unpack_trees_options opts;
> +	struct unpack_trees_options opts = UNPACK_TREES_OPTIONS_INIT;
>  	struct tree_desc tree_desc;
>  
> -	memset(&opts, 0, sizeof(opts));
>  	opts.head_idx = -1;
>  	opts.update = worktree;
>  	opts.skip_unmerged = !worktree;

which can be initialized with designated initializers in place, and
having an _INIT macro would not help us very much.

> @@ -719,9 +718,8 @@ static int merge_working_tree(const struct checkout_opts *opts,
>  	} else {
>  		struct tree_desc trees[2];
>  		struct tree *tree;
> -		struct unpack_trees_options topts;
> +		struct unpack_trees_options topts = UNPACK_TREES_OPTIONS_INIT;
>  
> -		memset(&topts, 0, sizeof(topts));
>  		topts.head_idx = -1;
>  		topts.src_index = &the_index;
>  		topts.dst_index = &the_index;

Likewise here.

So...

[PATCH v4 0/5] Non-trivial designated initializer conversion

[Ævar Arnfjörð Bjarmason]

See
http://lore.kernel.org/git/cover-0.6-00000000000-20210927T004920Z-avarab@gmail.com
for the v1 & goals, and
https://lore.kernel.org/git/cover-v2-0.5-00000000000-20210927T125715Z-avarab@gmail.com/
for the v2 and
https://lore.kernel.org/git/cover-v3-0.6-00000000000-20211001T102056Z-avarab@gmail.com
for the v3.

I ejected teh "UNPACK_TREES_OPTIONS_INIT" patch. I'll just propose
that as some alternate/fixup for en/removing-untracked-fixes
instead. It makes more sense as a lead-in to wider unpack-trees.c
fixes.

Ævar Arnfjörð Bjarmason (5):
  daemon.c: refactor hostinfo_init() to HOSTINFO_INIT macro
  builtin/blame.c: refactor commit_info_init() to COMMIT_INFO_INIT macro
  urlmatch.[ch]: add and use URLMATCH_CONFIG_INIT
  builtin/remote.c: add and use a REF_STATES_INIT
  builtin/remote.c: add and use SHOW_INFO_INIT

 builtin/blame.c  |  30 ++++++-------
 builtin/config.c |   2 +-
 builtin/remote.c | 111 +++++++++++++++++++++++------------------------
 credential.c     |   2 +-
 daemon.c         |  19 +++-----
 http.c           |   2 +-
 urlmatch.h       |   4 ++
 7 files changed, 82 insertions(+), 88 deletions(-)

Range-diff against v3:
1:  8f3f3f97fcb = 1:  8f3f3f97fcb daemon.c: refactor hostinfo_init() to HOSTINFO_INIT macro
2:  ced1d581f15 = 2:  ced1d581f15 builtin/blame.c: refactor commit_info_init() to COMMIT_INFO_INIT macro
3:  266948e604c = 3:  266948e604c urlmatch.[ch]: add and use URLMATCH_CONFIG_INIT
4:  41fcb0a45e5 = 4:  41fcb0a45e5 builtin/remote.c: add and use a REF_STATES_INIT
5:  25fec54877b = 5:  25fec54877b builtin/remote.c: add and use SHOW_INFO_INIT
6:  18358f5d57a < -:  ----------- unpack-trees.[ch]: define and use a UNPACK_TREES_OPTIONS_INIT
-- 
2.33.0.1380.g193143c62ce

[PATCH v4 1/5] daemon.c: refactor hostinfo_init() to HOSTINFO_INIT macro

[Ævar Arnfjörð Bjarmason]

Remove the hostinfo_init() function added in 01cec54e135 (daemon:
deglobalize hostname information, 2015-03-07) and instead initialize
the "struct hostinfo" with a macro.

This is the more idiomatic pattern in the codebase, and doesn't leave
us wondering when we see the *_init() function if this struct needs
more complex initialization than a macro can provide.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 daemon.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/daemon.c b/daemon.c
index 5c4cbad62d0..d80d009d1a1 100644
--- a/daemon.c
+++ b/daemon.c
@@ -63,6 +63,12 @@ struct hostinfo {
 	unsigned int hostname_lookup_done:1;
 	unsigned int saw_extended_args:1;
 };
+#define HOSTINFO_INIT { \
+	.hostname = STRBUF_INIT, \
+	.canon_hostname = STRBUF_INIT, \
+	.ip_address = STRBUF_INIT, \
+	.tcp_port = STRBUF_INIT, \
+}
 
 static void lookup_hostname(struct hostinfo *hi);
 
@@ -727,15 +733,6 @@ static void lookup_hostname(struct hostinfo *hi)
 	}
 }
 
-static void hostinfo_init(struct hostinfo *hi)
-{
-	memset(hi, 0, sizeof(*hi));
-	strbuf_init(&hi->hostname, 0);
-	strbuf_init(&hi->canon_hostname, 0);
-	strbuf_init(&hi->ip_address, 0);
-	strbuf_init(&hi->tcp_port, 0);
-}
-
 static void hostinfo_clear(struct hostinfo *hi)
 {
 	strbuf_release(&hi->hostname);
@@ -760,11 +757,9 @@ static int execute(void)
 	char *line = packet_buffer;
 	int pktlen, len, i;
 	char *addr = getenv("REMOTE_ADDR"), *port = getenv("REMOTE_PORT");
-	struct hostinfo hi;
+	struct hostinfo hi = HOSTINFO_INIT;
 	struct strvec env = STRVEC_INIT;
 
-	hostinfo_init(&hi);
-
 	if (addr)
 		loginfo("Connection from %s:%s", addr, port);
 
-- 
2.33.0.1380.g193143c62ce

[PATCH v4 2/5] builtin/blame.c: refactor commit_info_init() to COMMIT_INFO_INIT macro

[Ævar Arnfjörð Bjarmason]

Remove the commit_info_init() function addded in ea02ffa3857 (mailmap:
simplify map_user() interface, 2013-01-05) and instead initialize the
"struct commit_info" with a macro.

This is the more idiomatic pattern in the codebase, and doesn't leave
us wondering when we see the *_init() function if this struct needs
more complex initialization than a macro can provide.

The get_commit_info() function is only called by the three callers
being changed here immediately after initializing the struct with the
macros, so by moving the initialization to the callers we don't need
to do it in get_commit_info() anymore.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/blame.c | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 641523ff9af..1c31a996403 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -101,6 +101,16 @@ struct commit_info {
 	struct strbuf summary;
 };
 
+#define COMMIT_INFO_INIT { \
+	.author = STRBUF_INIT, \
+	.author_mail = STRBUF_INIT, \
+	.author_tz = STRBUF_INIT, \
+	.committer = STRBUF_INIT, \
+	.committer_mail = STRBUF_INIT, \
+	.committer_tz = STRBUF_INIT, \
+	.summary = STRBUF_INIT, \
+}
+
 /*
  * Parse author/committer line in the commit object buffer
  */
@@ -160,18 +170,6 @@ static void get_ac_line(const char *inbuf, const char *what,
 	strbuf_add(name, namebuf, namelen);
 }
 
-static void commit_info_init(struct commit_info *ci)
-{
-
-	strbuf_init(&ci->author, 0);
-	strbuf_init(&ci->author_mail, 0);
-	strbuf_init(&ci->author_tz, 0);
-	strbuf_init(&ci->committer, 0);
-	strbuf_init(&ci->committer_mail, 0);
-	strbuf_init(&ci->committer_tz, 0);
-	strbuf_init(&ci->summary, 0);
-}
-
 static void commit_info_destroy(struct commit_info *ci)
 {
 
@@ -192,8 +190,6 @@ static void get_commit_info(struct commit *commit,
 	const char *subject, *encoding;
 	const char *message;
 
-	commit_info_init(ret);
-
 	encoding = get_log_output_encoding();
 	message = logmsg_reencode(commit, NULL, encoding);
 	get_ac_line(message, "\nauthor ",
@@ -246,7 +242,7 @@ static void write_filename_info(struct blame_origin *suspect)
  */
 static int emit_one_suspect_detail(struct blame_origin *suspect, int repeat)
 {
-	struct commit_info ci;
+	struct commit_info ci = COMMIT_INFO_INIT;
 
 	if (!repeat && (suspect->commit->object.flags & METAINFO_SHOWN))
 		return 0;
@@ -440,7 +436,7 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int
 	int cnt;
 	const char *cp;
 	struct blame_origin *suspect = ent->suspect;
-	struct commit_info ci;
+	struct commit_info ci = COMMIT_INFO_INIT;
 	char hex[GIT_MAX_HEXSZ + 1];
 	int show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP);
 	const char *default_color = NULL, *color = NULL, *reset = NULL;
@@ -630,7 +626,7 @@ static void find_alignment(struct blame_scoreboard *sb, int *option)
 		if (longest_file < num)
 			longest_file = num;
 		if (!(suspect->commit->object.flags & METAINFO_SHOWN)) {
-			struct commit_info ci;
+			struct commit_info ci = COMMIT_INFO_INIT;
 			suspect->commit->object.flags |= METAINFO_SHOWN;
 			get_commit_info(suspect->commit, &ci, 1);
 			if (*option & OUTPUT_SHOW_EMAIL)
-- 
2.33.0.1380.g193143c62ce

[PATCH v4 3/5] urlmatch.[ch]: add and use URLMATCH_CONFIG_INIT

[Ævar Arnfjörð Bjarmason]

Change the initialization pattern of "struct urlmatch_config" to use
an *_INIT macro and designated initializers. Right now there's no
other "struct" member of "struct urlmatch_config" which would require
its own *_INIT, but it's good practice not to assume that. Let's also
change this to a designated initializer while we're at it.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/config.c | 2 +-
 credential.c     | 2 +-
 http.c           | 2 +-
 urlmatch.h       | 4 ++++
 4 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 865fddd6ce8..542d8d02b2b 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -575,7 +575,7 @@ static int get_urlmatch(const char *var, const char *url)
 	int ret;
 	char *section_tail;
 	struct string_list_item *item;
-	struct urlmatch_config config = { STRING_LIST_INIT_DUP };
+	struct urlmatch_config config = URLMATCH_CONFIG_INIT;
 	struct string_list values = STRING_LIST_INIT_DUP;
 
 	config.collect_fn = urlmatch_collect_fn;
diff --git a/credential.c b/credential.c
index 000ac7a8d43..e7240f3f636 100644
--- a/credential.c
+++ b/credential.c
@@ -105,7 +105,7 @@ static int match_partial_url(const char *url, void *cb)
 static void credential_apply_config(struct credential *c)
 {
 	char *normalized_url;
-	struct urlmatch_config config = { STRING_LIST_INIT_DUP };
+	struct urlmatch_config config = URLMATCH_CONFIG_INIT;
 	struct strbuf url = STRBUF_INIT;
 
 	if (!c->host)
diff --git a/http.c b/http.c
index d7c20493d7f..da12471c242 100644
--- a/http.c
+++ b/http.c
@@ -990,7 +990,7 @@ void http_init(struct remote *remote, const char *url, int proactive_auth)
 	char *low_speed_limit;
 	char *low_speed_time;
 	char *normalized_url;
-	struct urlmatch_config config = { STRING_LIST_INIT_DUP };
+	struct urlmatch_config config = URLMATCH_CONFIG_INIT;
 
 	config.section = "http";
 	config.key = NULL;
diff --git a/urlmatch.h b/urlmatch.h
index 6ff42f81b0c..34a3ba6d197 100644
--- a/urlmatch.h
+++ b/urlmatch.h
@@ -66,6 +66,10 @@ struct urlmatch_config {
 	int (*fallback_match_fn)(const char *url, void *cb);
 };
 
+#define URLMATCH_CONFIG_INIT { \
+	.vars = STRING_LIST_INIT_DUP, \
+}
+
 int urlmatch_config_entry(const char *var, const char *value, void *cb);
 
 #endif /* URL_MATCH_H */
-- 
2.33.0.1380.g193143c62ce

[PATCH v4 4/5] builtin/remote.c: add and use a REF_STATES_INIT

[Ævar Arnfjörð Bjarmason]

Use a new REF_STATES_INIT designated initializer instead of assigning
to the "strdup_strings" member of the previously memzero()'d version
of this struct.

The pattern of assigning to "strdup_strings" dates back to
211c89682ee (Make git-remote a builtin, 2008-02-29) (when it was
"strdup_paths"), i.e. long before we used anything like our current
established *_INIT patterns consistently.

Then in e61e0cc6b70 (builtin-remote: teach show to display remote
HEAD, 2009-02-25) and e5dcbfd9ab7 (builtin-remote: new show output
style for push refspecs, 2009-02-25) we added some more of these.

As it turns out we only initialized this struct three times, all the
other uses were of pointers to those initialized structs. So let's
initialize it in those three places, skip the memset(), and pass those
structs down appropriately.

This would be a behavior change if we had codepaths that relied say on
implicitly having had "new_refs" initialized to STRING_LIST_INIT_NODUP
with the memset(), but only set the "strdup_strings" on some other
struct, but then called string_list_append() on "new_refs". There
isn't any such codepath, all of the late assignments to
"strdup_strings" assigned to those structs that we'd use for those
codepaths.

So just initializing them all up-front makes for easier to understand
code, i.e. in the pre-image it looked as though we had that tricky
edge case, but we didn't.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/remote.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/builtin/remote.c b/builtin/remote.c
index 7f88e6ce9de..160dd954f74 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -344,6 +344,14 @@ struct ref_states {
 	int queried;
 };
 
+#define REF_STATES_INIT { \
+	.new_refs = STRING_LIST_INIT_DUP, \
+	.stale = STRING_LIST_INIT_DUP, \
+	.tracked = STRING_LIST_INIT_DUP, \
+	.heads = STRING_LIST_INIT_DUP, \
+	.push = STRING_LIST_INIT_DUP, \
+}
+
 static int get_ref_states(const struct ref *remote_refs, struct ref_states *states)
 {
 	struct ref *fetch_map = NULL, **tail = &fetch_map;
@@ -355,9 +363,6 @@ static int get_ref_states(const struct ref *remote_refs, struct ref_states *stat
 			die(_("Could not get fetch map for refspec %s"),
 				states->remote->fetch.raw[i]);
 
-	states->new_refs.strdup_strings = 1;
-	states->tracked.strdup_strings = 1;
-	states->stale.strdup_strings = 1;
 	for (ref = fetch_map; ref; ref = ref->next) {
 		if (!ref->peer_ref || !ref_exists(ref->peer_ref->name))
 			string_list_append(&states->new_refs, abbrev_branch(ref->name));
@@ -406,7 +411,6 @@ static int get_push_ref_states(const struct ref *remote_refs,
 
 	match_push_refs(local_refs, &push_map, &remote->push, MATCH_REFS_NONE);
 
-	states->push.strdup_strings = 1;
 	for (ref = push_map; ref; ref = ref->next) {
 		struct string_list_item *item;
 		struct push_info *info;
@@ -449,7 +453,6 @@ static int get_push_ref_states_noquery(struct ref_states *states)
 	if (remote->mirror)
 		return 0;
 
-	states->push.strdup_strings = 1;
 	if (!remote->push.nr) {
 		item = string_list_append(&states->push, _("(matching)"));
 		info = item->util = xcalloc(1, sizeof(struct push_info));
@@ -483,7 +486,6 @@ static int get_head_names(const struct ref *remote_refs, struct ref_states *stat
 	refspec.force = 0;
 	refspec.pattern = 1;
 	refspec.src = refspec.dst = "refs/heads/*";
-	states->heads.strdup_strings = 1;
 	get_fetch_map(remote_refs, &refspec, &fetch_map_tail, 0);
 	matches = guess_remote_head(find_ref_by_name(remote_refs, "HEAD"),
 				    fetch_map, 1);
@@ -1212,7 +1214,7 @@ static int show(int argc, const char **argv)
 		OPT_BOOL('n', NULL, &no_query, N_("do not query remotes")),
 		OPT_END()
 	};
-	struct ref_states states;
+	struct ref_states states = REF_STATES_INIT;
 	struct string_list info_list = STRING_LIST_INIT_NODUP;
 	struct show_info info;
 
@@ -1225,7 +1227,6 @@ static int show(int argc, const char **argv)
 	if (!no_query)
 		query_flag = (GET_REF_STATES | GET_HEAD_NAMES | GET_PUSH_REF_STATES);
 
-	memset(&states, 0, sizeof(states));
 	memset(&info, 0, sizeof(info));
 	info.states = &states;
 	info.list = &info_list;
@@ -1334,8 +1335,7 @@ static int set_head(int argc, const char **argv)
 	if (!opt_a && !opt_d && argc == 2) {
 		head_name = xstrdup(argv[1]);
 	} else if (opt_a && !opt_d && argc == 1) {
-		struct ref_states states;
-		memset(&states, 0, sizeof(states));
+		struct ref_states states = REF_STATES_INIT;
 		get_remote_ref_states(argv[0], &states, GET_HEAD_NAMES);
 		if (!states.heads.nr)
 			result |= error(_("Cannot determine remote HEAD"));
@@ -1374,14 +1374,13 @@ static int set_head(int argc, const char **argv)
 static int prune_remote(const char *remote, int dry_run)
 {
 	int result = 0;
-	struct ref_states states;
+	struct ref_states states = REF_STATES_INIT;
 	struct string_list refs_to_prune = STRING_LIST_INIT_NODUP;
 	struct string_list_item *item;
 	const char *dangling_msg = dry_run
 		? _(" %s will become dangling!")
 		: _(" %s has become dangling!");
 
-	memset(&states, 0, sizeof(states));
 	get_remote_ref_states(remote, &states, GET_REF_STATES);
 
 	if (!states.stale.nr) {
-- 
2.33.0.1380.g193143c62ce

[PATCH v4 5/5] builtin/remote.c: add and use SHOW_INFO_INIT

[Ævar Arnfjörð Bjarmason]

In the preceding commit we introduced REF_STATES_INIT, but did not
change the "struct show_info" to have a corresponding
initializer. Let's do that, and make it use "REF_STATES_INIT" and
"STRING_LIST_INIT_DUP", doing that requires changing "list" and
"states" away from being pointers.

The resulting end-state is simpler since we omit the local "info_list"
and "states" variables in show() as well as the memset().

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/remote.c | 90 ++++++++++++++++++++++++------------------------
 1 file changed, 45 insertions(+), 45 deletions(-)

diff --git a/builtin/remote.c b/builtin/remote.c
index 160dd954f74..deb48772ac5 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -972,26 +972,31 @@ static int get_remote_ref_states(const char *name,
 }
 
 struct show_info {
-	struct string_list *list;
-	struct ref_states *states;
+	struct string_list list;
+	struct ref_states states;
 	int width, width2;
 	int any_rebase;
 };
 
+#define SHOW_INFO_INIT { \
+	.list = STRING_LIST_INIT_DUP, \
+	.states = REF_STATES_INIT, \
+}
+
 static int add_remote_to_show_info(struct string_list_item *item, void *cb_data)
 {
 	struct show_info *info = cb_data;
 	int n = strlen(item->string);
 	if (n > info->width)
 		info->width = n;
-	string_list_insert(info->list, item->string);
+	string_list_insert(&info->list, item->string);
 	return 0;
 }
 
 static int show_remote_info_item(struct string_list_item *item, void *cb_data)
 {
 	struct show_info *info = cb_data;
-	struct ref_states *states = info->states;
+	struct ref_states *states = &info->states;
 	const char *name = item->string;
 
 	if (states->queried) {
@@ -1018,7 +1023,7 @@ static int show_remote_info_item(struct string_list_item *item, void *cb_data)
 static int add_local_to_show_info(struct string_list_item *branch_item, void *cb_data)
 {
 	struct show_info *show_info = cb_data;
-	struct ref_states *states = show_info->states;
+	struct ref_states *states = &show_info->states;
 	struct branch_info *branch_info = branch_item->util;
 	struct string_list_item *item;
 	int n;
@@ -1031,7 +1036,7 @@ static int add_local_to_show_info(struct string_list_item *branch_item, void *cb
 	if (branch_info->rebase >= REBASE_TRUE)
 		show_info->any_rebase = 1;
 
-	item = string_list_insert(show_info->list, branch_item->string);
+	item = string_list_insert(&show_info->list, branch_item->string);
 	item->util = branch_info;
 
 	return 0;
@@ -1086,7 +1091,7 @@ static int add_push_to_show_info(struct string_list_item *push_item, void *cb_da
 		show_info->width = n;
 	if ((n = strlen(push_info->dest)) > show_info->width2)
 		show_info->width2 = n;
-	item = string_list_append(show_info->list, push_item->string);
+	item = string_list_append(&show_info->list, push_item->string);
 	item->util = push_item->util;
 	return 0;
 }
@@ -1214,9 +1219,7 @@ static int show(int argc, const char **argv)
 		OPT_BOOL('n', NULL, &no_query, N_("do not query remotes")),
 		OPT_END()
 	};
-	struct ref_states states = REF_STATES_INIT;
-	struct string_list info_list = STRING_LIST_INIT_NODUP;
-	struct show_info info;
+	struct show_info info = SHOW_INFO_INIT;
 
 	argc = parse_options(argc, argv, NULL, options, builtin_remote_show_usage,
 			     0);
@@ -1227,25 +1230,22 @@ static int show(int argc, const char **argv)
 	if (!no_query)
 		query_flag = (GET_REF_STATES | GET_HEAD_NAMES | GET_PUSH_REF_STATES);
 
-	memset(&info, 0, sizeof(info));
-	info.states = &states;
-	info.list = &info_list;
 	for (; argc; argc--, argv++) {
 		int i;
 		const char **url;
 		int url_nr;
 
-		get_remote_ref_states(*argv, &states, query_flag);
+		get_remote_ref_states(*argv, &info.states, query_flag);
 
 		printf_ln(_("* remote %s"), *argv);
-		printf_ln(_("  Fetch URL: %s"), states.remote->url_nr > 0 ?
-		       states.remote->url[0] : _("(no URL)"));
-		if (states.remote->pushurl_nr) {
-			url = states.remote->pushurl;
-			url_nr = states.remote->pushurl_nr;
+		printf_ln(_("  Fetch URL: %s"), info.states.remote->url_nr > 0 ?
+		       info.states.remote->url[0] : _("(no URL)"));
+		if (info.states.remote->pushurl_nr) {
+			url = info.states.remote->pushurl;
+			url_nr = info.states.remote->pushurl_nr;
 		} else {
-			url = states.remote->url;
-			url_nr = states.remote->url_nr;
+			url = info.states.remote->url;
+			url_nr = info.states.remote->url_nr;
 		}
 		for (i = 0; i < url_nr; i++)
 			/*
@@ -1258,57 +1258,57 @@ static int show(int argc, const char **argv)
 			printf_ln(_("  Push  URL: %s"), _("(no URL)"));
 		if (no_query)
 			printf_ln(_("  HEAD branch: %s"), _("(not queried)"));
-		else if (!states.heads.nr)
+		else if (!info.states.heads.nr)
 			printf_ln(_("  HEAD branch: %s"), _("(unknown)"));
-		else if (states.heads.nr == 1)
-			printf_ln(_("  HEAD branch: %s"), states.heads.items[0].string);
+		else if (info.states.heads.nr == 1)
+			printf_ln(_("  HEAD branch: %s"), info.states.heads.items[0].string);
 		else {
 			printf(_("  HEAD branch (remote HEAD is ambiguous,"
 				 " may be one of the following):\n"));
-			for (i = 0; i < states.heads.nr; i++)
-				printf("    %s\n", states.heads.items[i].string);
+			for (i = 0; i < info.states.heads.nr; i++)
+				printf("    %s\n", info.states.heads.items[i].string);
 		}
 
 		/* remote branch info */
 		info.width = 0;
-		for_each_string_list(&states.new_refs, add_remote_to_show_info, &info);
-		for_each_string_list(&states.tracked, add_remote_to_show_info, &info);
-		for_each_string_list(&states.stale, add_remote_to_show_info, &info);
-		if (info.list->nr)
+		for_each_string_list(&info.states.new_refs, add_remote_to_show_info, &info);
+		for_each_string_list(&info.states.tracked, add_remote_to_show_info, &info);
+		for_each_string_list(&info.states.stale, add_remote_to_show_info, &info);
+		if (info.list.nr)
 			printf_ln(Q_("  Remote branch:%s",
 				     "  Remote branches:%s",
-				     info.list->nr),
+				     info.list.nr),
 				  no_query ? _(" (status not queried)") : "");
-		for_each_string_list(info.list, show_remote_info_item, &info);
-		string_list_clear(info.list, 0);
+		for_each_string_list(&info.list, show_remote_info_item, &info);
+		string_list_clear(&info.list, 0);
 
 		/* git pull info */
 		info.width = 0;
 		info.any_rebase = 0;
 		for_each_string_list(&branch_list, add_local_to_show_info, &info);
-		if (info.list->nr)
+		if (info.list.nr)
 			printf_ln(Q_("  Local branch configured for 'git pull':",
 				     "  Local branches configured for 'git pull':",
-				     info.list->nr));
-		for_each_string_list(info.list, show_local_info_item, &info);
-		string_list_clear(info.list, 0);
+				     info.list.nr));
+		for_each_string_list(&info.list, show_local_info_item, &info);
+		string_list_clear(&info.list, 0);
 
 		/* git push info */
-		if (states.remote->mirror)
+		if (info.states.remote->mirror)
 			printf_ln(_("  Local refs will be mirrored by 'git push'"));
 
 		info.width = info.width2 = 0;
-		for_each_string_list(&states.push, add_push_to_show_info, &info);
-		QSORT(info.list->items, info.list->nr, cmp_string_with_push);
-		if (info.list->nr)
+		for_each_string_list(&info.states.push, add_push_to_show_info, &info);
+		QSORT(info.list.items, info.list.nr, cmp_string_with_push);
+		if (info.list.nr)
 			printf_ln(Q_("  Local ref configured for 'git push'%s:",
 				     "  Local refs configured for 'git push'%s:",
-				     info.list->nr),
+				     info.list.nr),
 				  no_query ? _(" (status not queried)") : "");
-		for_each_string_list(info.list, show_push_info_item, &info);
-		string_list_clear(info.list, 0);
+		for_each_string_list(&info.list, show_push_info_item, &info);
+		string_list_clear(&info.list, 0);
 
-		free_remote_ref_states(&states);
+		free_remote_ref_states(&info.states);
 	}
 
 	return result;
-- 
2.33.0.1380.g193143c62ce