[PATCH 0/5] Designated initializer cleanup & conversion

[PATCH 0/5] Designated initializer cleanup & conversion

[Ævar Arnfjörð Bjarmason]

For some of the memory leak fixes I've got queued up the first step is
often to migrate the relevant code to using modern initializer
patterns. I.e. using an *_INIT macro, initializing on the stack not
the heap when possible etc.

I found that for some of those I'd start with a migration to
designated initializers, but if we're doing that we might as well do
(almost) all of them.

This series does /most/ of that, the remaining parts are things that
would conflict with other in-flight changes, i.e. this merges cleanly
with "seen". We can clean up any stray stragglers some other time.

See
https://lore.kernel.org/git/cover-0.5-00000000000-20210701T104855Z-avarab@gmail.com/
for a predecessor series which already landed as bd4232fac33 (Merge
branch 'ab/struct-init', 2021-07-16).

Ævar Arnfjörð Bjarmason (5):
  submodule-config.h: remove unused SUBMODULE_INIT macro
  *.[ch] *_INIT macros: use { 0 } for a "zero out" idiom
  *.h _INIT macros: don't specify fields equal to 0
  *.h: move some *_INIT to designated initializers
  cbtree.h: define cb_init() in terms of CBTREE_INIT

 add-interactive.c                             |  8 +++++--
 builtin/submodule--helper.c                   | 21 ++++++++++---------
 cache.h                                       |  4 +++-
 cbtree.h                                      |  5 +++--
 checkout.c                                    |  2 +-
 .../git-credential-gnome-keyring.c            |  2 +-
 .../libsecret/git-credential-libsecret.c      |  2 +-
 diff.c                                        |  4 ++--
 entry.h                                       |  2 +-
 lockfile.h                                    |  2 +-
 object-store.h                                |  2 +-
 object.h                                      |  2 +-
 oid-array.h                                   |  2 +-
 path.h                                        |  5 +----
 ref-filter.c                                  |  2 +-
 remote.c                                      |  2 +-
 revision.c                                    |  2 +-
 sequencer.h                                   |  4 +++-
 shallow.h                                     |  4 +++-
 simple-ipc.h                                  |  6 +-----
 strbuf.h                                      |  2 +-
 strvec.h                                      |  4 +++-
 submodule-config.h                            |  4 ----
 submodule.c                                   |  8 ++++---
 submodule.h                                   |  4 +++-
 t/helper/test-run-command.c                   |  6 ++++--
 transport.h                                   |  4 +++-
 27 files changed, 63 insertions(+), 52 deletions(-)

-- 
2.33.0.1294.g2bdf2798764

[PATCH 1/5] submodule-config.h: remove unused SUBMODULE_INIT macro

[Ævar Arnfjörð Bjarmason]

This macro was added and used in c68f8375760 (implement fetching of
moved submodules, 2017-10-16) but its last user went away in
be76c212823 (fetch: ensure submodule objects fetched, 2018-12-06).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 submodule-config.h | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/submodule-config.h b/submodule-config.h
index c11e22cf509..65875b94ea5 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -45,10 +45,6 @@ struct submodule {
 	struct object_id gitmodules_oid;
 	int recommend_shallow;
 };
-
-#define SUBMODULE_INIT { NULL, NULL, NULL, RECURSE_SUBMODULES_NONE, \
-	NULL, NULL, SUBMODULE_UPDATE_STRATEGY_INIT, { { 0 } }, -1 };
-
 struct submodule_cache;
 struct repository;
 
-- 
2.33.0.1294.g2bdf2798764

[PATCH 2/5] *.[ch] *_INIT macros: use { 0 } for a "zero out" idiom

[Ævar Arnfjörð Bjarmason]

In C it isn't required to specify that all members of a struct are
zero'd out to 0, NULL or '\0', just providing a "{ 0 }" will
accomplish that.

Let's also change change code that provided N zero'd fields to just
provide one, and change e.g. "{ NULL }" to "{ 0 }" for
consistency. I.e. even if the first member is a pointer let's use "0"
instead of "NULL". The point of using "0" consistently is to pick one,
and to not have the reader wonder why we're not using the same pattern
everywhere.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/submodule--helper.c                      | 16 ++++++++--------
 checkout.c                                       |  2 +-
 .../gnome-keyring/git-credential-gnome-keyring.c |  2 +-
 .../libsecret/git-credential-libsecret.c         |  2 +-
 diff.c                                           |  4 ++--
 entry.h                                          |  2 +-
 lockfile.h                                       |  2 +-
 object-store.h                                   |  2 +-
 object.h                                         |  2 +-
 oid-array.h                                      |  2 +-
 path.h                                           |  5 +----
 ref-filter.c                                     |  2 +-
 remote.c                                         |  2 +-
 revision.c                                       |  2 +-
 14 files changed, 22 insertions(+), 25 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 5336daf186d..deca75c83ee 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -307,7 +307,7 @@ struct module_list {
 	const struct cache_entry **entries;
 	int alloc, nr;
 };
-#define MODULE_LIST_INIT { NULL, 0, 0 }
+#define MODULE_LIST_INIT { 0 }
 
 static int module_list_compute(int argc, const char **argv,
 			       const char *prefix,
@@ -588,7 +588,7 @@ struct init_cb {
 	const char *prefix;
 	unsigned int flags;
 };
-#define INIT_CB_INIT { NULL, 0 }
+#define INIT_CB_INIT { 0 }
 
 static void init_submodule(const char *path, const char *prefix,
 			   unsigned int flags)
@@ -717,7 +717,7 @@ struct status_cb {
 	const char *prefix;
 	unsigned int flags;
 };
-#define STATUS_CB_INIT { NULL, 0 }
+#define STATUS_CB_INIT { 0 }
 
 static void print_status(unsigned int flags, char state, const char *path,
 			 const struct object_id *oid, const char *displaypath)
@@ -911,13 +911,13 @@ struct module_cb {
 	char status;
 	const char *sm_path;
 };
-#define MODULE_CB_INIT { 0, 0, NULL, NULL, '\0', NULL }
+#define MODULE_CB_INIT { 0 }
 
 struct module_cb_list {
 	struct module_cb **entries;
 	int alloc, nr;
 };
-#define MODULE_CB_LIST_INIT { NULL, 0, 0 }
+#define MODULE_CB_LIST_INIT { 0 }
 
 struct summary_cb {
 	int argc;
@@ -928,7 +928,7 @@ struct summary_cb {
 	unsigned int files: 1;
 	int summary_limit;
 };
-#define SUMMARY_CB_INIT { 0, NULL, NULL, 0, 0, 0, 0 }
+#define SUMMARY_CB_INIT { 0 }
 
 enum diff_cmd {
 	DIFF_INDEX,
@@ -1334,7 +1334,7 @@ struct sync_cb {
 	const char *prefix;
 	unsigned int flags;
 };
-#define SYNC_CB_INIT { NULL, 0 }
+#define SYNC_CB_INIT { 0 }
 
 static void sync_submodule(const char *path, const char *prefix,
 			   unsigned int flags)
@@ -1480,7 +1480,7 @@ struct deinit_cb {
 	const char *prefix;
 	unsigned int flags;
 };
-#define DEINIT_CB_INIT { NULL, 0 }
+#define DEINIT_CB_INIT { 0 }
 
 static void deinit_submodule(const char *path, const char *prefix,
 			     unsigned int flags)
diff --git a/checkout.c b/checkout.c
index 6586e30ca5a..2e39dae684f 100644
--- a/checkout.c
+++ b/checkout.c
@@ -14,7 +14,7 @@ struct tracking_name_data {
 	struct object_id *default_dst_oid;
 };
 
-#define TRACKING_NAME_DATA_INIT { NULL, NULL, NULL, 0, NULL, NULL, NULL }
+#define TRACKING_NAME_DATA_INIT { 0 }
 
 static int check_tracking_name(struct remote *remote, void *cb_data)
 {
diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index d389bfadcee..5927e27ae6e 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -138,7 +138,7 @@ struct credential {
 	char *password;
 };
 
-#define CREDENTIAL_INIT { NULL, NULL, 0, NULL, NULL, NULL }
+#define CREDENTIAL_INIT { 0 }
 
 typedef int (*credential_op_cb)(struct credential *);
 
diff --git a/contrib/credential/libsecret/git-credential-libsecret.c b/contrib/credential/libsecret/git-credential-libsecret.c
index e6598b63833..2c5d76d789f 100644
--- a/contrib/credential/libsecret/git-credential-libsecret.c
+++ b/contrib/credential/libsecret/git-credential-libsecret.c
@@ -41,7 +41,7 @@ struct credential {
 	char *password;
 };
 
-#define CREDENTIAL_INIT { NULL, NULL, 0, NULL, NULL, NULL }
+#define CREDENTIAL_INIT { 0 }
 
 typedef int (*credential_op_cb)(struct credential *);
 
diff --git a/diff.c b/diff.c
index c8f530ffdbe..861282db1c3 100644
--- a/diff.c
+++ b/diff.c
@@ -775,13 +775,13 @@ struct emitted_diff_symbol {
 	int indent_width; /* The visual width of the indentation */
 	enum diff_symbol s;
 };
-#define EMITTED_DIFF_SYMBOL_INIT {NULL}
+#define EMITTED_DIFF_SYMBOL_INIT { 0 }
 
 struct emitted_diff_symbols {
 	struct emitted_diff_symbol *buf;
 	int nr, alloc;
 };
-#define EMITTED_DIFF_SYMBOLS_INIT {NULL, 0, 0}
+#define EMITTED_DIFF_SYMBOLS_INIT { 0 }
 
 static void append_emitted_diff_symbol(struct diff_options *o,
 				       struct emitted_diff_symbol *e)
diff --git a/entry.h b/entry.h
index 7c889e58fd7..04bc8bb59f0 100644
--- a/entry.h
+++ b/entry.h
@@ -16,7 +16,7 @@ struct checkout {
 		 clone:1,
 		 refresh_cache:1;
 };
-#define CHECKOUT_INIT { NULL, "" }
+#define CHECKOUT_INIT { 0 }
 
 #define TEMPORARY_FILENAME_LENGTH 25
 /*
diff --git a/lockfile.h b/lockfile.h
index db93e6ba73e..90af4e66b28 100644
--- a/lockfile.h
+++ b/lockfile.h
@@ -121,7 +121,7 @@ struct lock_file {
 	struct tempfile *tempfile;
 };
 
-#define LOCK_INIT { NULL }
+#define LOCK_INIT { 0 }
 
 /* String appended to a filename to derive the lockfile name: */
 #define LOCK_SUFFIX ".lock"
diff --git a/object-store.h b/object-store.h
index c5130d8baea..1e647a5be30 100644
--- a/object-store.h
+++ b/object-store.h
@@ -371,7 +371,7 @@ struct object_info {
  * Initializer for a "struct object_info" that wants no items. You may
  * also memset() the memory to all-zeroes.
  */
-#define OBJECT_INFO_INIT {NULL}
+#define OBJECT_INFO_INIT { 0 }
 
 /* Invoke lookup_replace_object() on the given hash */
 #define OBJECT_INFO_LOOKUP_REPLACE 1
diff --git a/object.h b/object.h
index 549f2d256bc..cb556ab7753 100644
--- a/object.h
+++ b/object.h
@@ -55,7 +55,7 @@ struct object_array {
 	} *objects;
 };
 
-#define OBJECT_ARRAY_INIT { 0, 0, NULL }
+#define OBJECT_ARRAY_INIT { 0 }
 
 /*
  * object flag allocation:
diff --git a/oid-array.h b/oid-array.h
index 72bca78b7dc..f60f9af6741 100644
--- a/oid-array.h
+++ b/oid-array.h
@@ -56,7 +56,7 @@ struct oid_array {
 	int sorted;
 };
 
-#define OID_ARRAY_INIT { NULL, 0, 0, 0 }
+#define OID_ARRAY_INIT { 0 }
 
 /**
  * Add an item to the set. The object ID will be placed at the end of the array
diff --git a/path.h b/path.h
index 251c78d9800..b68691a86b8 100644
--- a/path.h
+++ b/path.h
@@ -181,10 +181,7 @@ struct path_cache {
 	const char *shallow;
 };
 
-#define PATH_CACHE_INIT                                        \
-	{                                                      \
-		NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL \
-	}
+#define PATH_CACHE_INIT { 0 }
 
 const char *git_path_squash_msg(struct repository *r);
 const char *git_path_merge_msg(struct repository *r);
diff --git a/ref-filter.c b/ref-filter.c
index 93ce2a6ef2e..eee4f9b17c5 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -633,7 +633,7 @@ static struct {
 	 */
 };
 
-#define REF_FORMATTING_STATE_INIT  { 0, NULL }
+#define REF_FORMATTING_STATE_INIT  { 0 }
 
 struct ref_formatting_stack {
 	struct ref_formatting_stack *prev;
diff --git a/remote.c b/remote.c
index 31e141b01fa..f958543d707 100644
--- a/remote.c
+++ b/remote.c
@@ -2403,7 +2403,7 @@ struct reflog_commit_array {
 	size_t nr, alloc;
 };
 
-#define REFLOG_COMMIT_ARRAY_INIT { NULL, 0, 0 }
+#define REFLOG_COMMIT_ARRAY_INIT { 0 }
 
 /* Append a commit to the array. */
 static void append_commit(struct reflog_commit_array *arr,
diff --git a/revision.c b/revision.c
index 0dabb5a0bcf..73e5004d60b 100644
--- a/revision.c
+++ b/revision.c
@@ -249,7 +249,7 @@ struct commit_stack {
 	struct commit **items;
 	size_t nr, alloc;
 };
-#define COMMIT_STACK_INIT { NULL, 0, 0 }
+#define COMMIT_STACK_INIT { 0 }
 
 static void commit_stack_push(struct commit_stack *stack, struct commit *commit)
 {
-- 
2.33.0.1294.g2bdf2798764

Re: [PATCH 2/5] *.[ch] *_INIT macros: use { 0 } for a "zero out" idiom

[Eric Sunshine]

On Sun, Sep 26, 2021 at 8:40 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> In C it isn't required to specify that all members of a struct are
> zero'd out to 0, NULL or '\0', just providing a "{ 0 }" will
> accomplish that.
>
> Let's also change change code that provided N zero'd fields to just

s/change change/change/

> provide one, and change e.g. "{ NULL }" to "{ 0 }" for
> consistency. I.e. even if the first member is a pointer let's use "0"
> instead of "NULL". The point of using "0" consistently is to pick one,
> and to not have the reader wonder why we're not using the same pattern
> everywhere.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>

Re: [PATCH 2/5] *.[ch] *_INIT macros: use { 0 } for a "zero out" idiom

[Johannes Sixt]

Am 27.09.21 um 02:39 schrieb Ævar Arnfjörð Bjarmason:
> diff --git a/entry.h b/entry.h
> index 7c889e58fd7..04bc8bb59f0 100644
> --- a/entry.h
> +++ b/entry.h
> @@ -16,7 +16,7 @@ struct checkout {
>  		 clone:1,
>  		 refresh_cache:1;
>  };
> -#define CHECKOUT_INIT { NULL, "" }
> +#define CHECKOUT_INIT { 0 }

This is not an equivalence transformation and does not belong in this patch.

-- Hannes

Re: [PATCH 2/5] *.[ch] *_INIT macros: use { 0 } for a "zero out" idiom

[Junio C Hamano]

Johannes Sixt <j6t@kdbg.org> writes:

> Am 27.09.21 um 02:39 schrieb Ævar Arnfjörð Bjarmason:
>> diff --git a/entry.h b/entry.h
>> index 7c889e58fd7..04bc8bb59f0 100644
>> --- a/entry.h
>> +++ b/entry.h
>> @@ -16,7 +16,7 @@ struct checkout {
>>  		 clone:1,
>>  		 refresh_cache:1;
>>  };
>> -#define CHECKOUT_INIT { NULL, "" }
>> +#define CHECKOUT_INIT { 0 }
>
> This is not an equivalence transformation and does not belong in this patch.

Thanks for carefully reading.  This is the kind of mistake I hate
about these mind-numbing "let's do the clean-up" series.

[PATCH 3/5] *.h _INIT macros: don't specify fields equal to 0

[Ævar Arnfjörð Bjarmason]

Change the initialization of "struct strbuf" changed in
cbc0f81d96f (strbuf: use designated initializers in STRBUF_INIT,
2017-07-10) to omit specifying "alloc" and "len", as we do with other
"alloc" and "len" (or "nr") in similar structs.

Let's likewise omit the explicit initialization of all fields in the
"struct ipc_client_connect_option" struct added in
59c7b88198a (simple-ipc: add win32 implementation, 2021-03-15).

Finally, start incrementally changing the same pattern in
"t/helper/test-run-command.c". This change was part of an earlier
on-list version[1] of c90be786da9 (test-tool run-command: fix
flip-flop init pattern, 2021-09-11).

1. https://lore.kernel.org/git/patch-1.1-0aa4523ab6e-20210909T130849Z-avarab@gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 simple-ipc.h                | 6 +-----
 strbuf.h                    | 2 +-
 t/helper/test-run-command.c | 2 +-
 3 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/simple-ipc.h b/simple-ipc.h
index 2c48a5ee004..08b2908d5f8 100644
--- a/simple-ipc.h
+++ b/simple-ipc.h
@@ -65,11 +65,7 @@ struct ipc_client_connect_options {
 	unsigned int uds_disallow_chdir:1;
 };
 
-#define IPC_CLIENT_CONNECT_OPTIONS_INIT { \
-	.wait_if_busy = 0, \
-	.wait_if_not_found = 0, \
-	.uds_disallow_chdir = 0, \
-}
+#define IPC_CLIENT_CONNECT_OPTIONS_INIT { 0 }
 
 /*
  * Determine if a server is listening on this named pipe or socket using
diff --git a/strbuf.h b/strbuf.h
index 5b1113abf8f..3b36bbc49f0 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -70,7 +70,7 @@ struct strbuf {
 };
 
 extern char strbuf_slopbuf[];
-#define STRBUF_INIT  { .alloc = 0, .len = 0, .buf = strbuf_slopbuf }
+#define STRBUF_INIT  { .buf = strbuf_slopbuf }
 
 /*
  * Predeclare this here, since cache.h includes this file before it defines the
diff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c
index 14c57365e76..50bb98b7e04 100644
--- a/t/helper/test-run-command.c
+++ b/t/helper/test-run-command.c
@@ -61,7 +61,7 @@ struct testsuite {
 	int quiet, immediate, verbose, verbose_log, trace, write_junit_xml;
 };
 #define TESTSUITE_INIT \
-	{ STRING_LIST_INIT_DUP, STRING_LIST_INIT_DUP, 0, 0, 0, 0, 0, 0, 0 }
+	{ STRING_LIST_INIT_DUP, STRING_LIST_INIT_DUP }
 
 static int next_test(struct child_process *cp, struct strbuf *err, void *cb,
 		     void **task_cb)
-- 
2.33.0.1294.g2bdf2798764

[PATCH 4/5] *.h: move some *_INIT to designated initializers

[Ævar Arnfjörð Bjarmason]

Move various *_INIT macros to use designated initializers. This helps
readability. I've only picked those leftover macros that were not
touched by another in-flight series of mine which changed others, but
also how initialization was done.

In the case of SUBMODULE_ALTERNATE_SETUP_INIT I've left an explicit
initialization of "error_mode", even though
SUBMODULE_ALTERNATE_ERROR_IGNORE itself is defined as "0". Let's not
peek under the hood and assume that enum fields we know the value of
will stay at "0".

The change to "TESTSUITE_INIT" in "t/helper/test-run-command.c" was
part of an earlier on-list version[1] of c90be786da9 (test-tool
run-command: fix flip-flop init pattern, 2021-09-11).

1. https://lore.kernel.org/git/patch-1.1-0aa4523ab6e-20210909T130849Z-avarab@gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 add-interactive.c           | 8 ++++++--
 builtin/submodule--helper.c | 5 +++--
 cache.h                     | 4 +++-
 sequencer.h                 | 4 +++-
 shallow.h                   | 4 +++-
 strvec.h                    | 4 +++-
 submodule.c                 | 8 +++++---
 submodule.h                 | 4 +++-
 t/helper/test-run-command.c | 6 ++++--
 transport.h                 | 4 +++-
 10 files changed, 36 insertions(+), 15 deletions(-)

diff --git a/add-interactive.c b/add-interactive.c
index 36ebdbdf7e2..6498ae196f1 100644
--- a/add-interactive.c
+++ b/add-interactive.c
@@ -102,8 +102,12 @@ struct prefix_item_list {
 	int *selected; /* for multi-selections */
 	size_t min_length, max_length;
 };
-#define PREFIX_ITEM_LIST_INIT \
-	{ STRING_LIST_INIT_DUP, STRING_LIST_INIT_NODUP, NULL, 1, 4 }
+#define PREFIX_ITEM_LIST_INIT { \
+	.items = STRING_LIST_INIT_DUP, \
+	.sorted = STRING_LIST_INIT_NODUP, \
+	.min_length = 1, \
+	.max_length = 4, \
+}
 
 static void prefix_item_list_clear(struct prefix_item_list *list)
 {
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index deca75c83ee..57f09925157 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1647,8 +1647,9 @@ struct submodule_alternate_setup {
 	} error_mode;
 	struct string_list *reference;
 };
-#define SUBMODULE_ALTERNATE_SETUP_INIT { NULL, \
-	SUBMODULE_ALTERNATE_ERROR_IGNORE, NULL }
+#define SUBMODULE_ALTERNATE_SETUP_INIT { \
+	.error_mode = SUBMODULE_ALTERNATE_ERROR_IGNORE, \
+}
 
 static const char alternate_error_advice[] = N_(
 "An alternate computed from a superproject's alternate is invalid.\n"
diff --git a/cache.h b/cache.h
index f6295f3b048..794237515b9 100644
--- a/cache.h
+++ b/cache.h
@@ -1668,7 +1668,9 @@ struct cache_def {
 	int track_flags;
 	int prefix_len_stat_func;
 };
-#define CACHE_DEF_INIT { STRBUF_INIT, 0, 0, 0 }
+#define CACHE_DEF_INIT { \
+	.path = STRBUF_INIT, \
+}
 static inline void cache_def_clear(struct cache_def *cache)
 {
 	strbuf_release(&cache->path);
diff --git a/sequencer.h b/sequencer.h
index 2088344cb37..259d4eb4a8a 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -119,7 +119,9 @@ struct todo_list {
 	struct stat_data stat;
 };
 
-#define TODO_LIST_INIT { STRBUF_INIT }
+#define TODO_LIST_INIT { \
+	.buf = STRBUF_INIT, \
+}
 
 int todo_list_parse_insn_buffer(struct repository *r, char *buf,
 				struct todo_list *todo_list);
diff --git a/shallow.h b/shallow.h
index 5b4a96dcd69..aba6ff58294 100644
--- a/shallow.h
+++ b/shallow.h
@@ -23,7 +23,9 @@ int is_repository_shallow(struct repository *r);
 struct shallow_lock {
 	struct lock_file lock;
 };
-#define SHALLOW_LOCK_INIT { LOCK_INIT }
+#define SHALLOW_LOCK_INIT { \
+	.lock = LOCK_INIT, \
+}
 
 /* commit $GIT_DIR/shallow and reset stat-validity checks */
 int commit_shallow_file(struct repository *r, struct shallow_lock *lk);
diff --git a/strvec.h b/strvec.h
index 6b3cbd67589..9f55c8766ba 100644
--- a/strvec.h
+++ b/strvec.h
@@ -33,7 +33,9 @@ struct strvec {
 	size_t alloc;
 };
 
-#define STRVEC_INIT { empty_strvec, 0, 0 }
+#define STRVEC_INIT { \
+	.v = empty_strvec, \
+}
 
 /**
  * Initialize an array. This is no different than assigning from
diff --git a/submodule.c b/submodule.c
index 78aed03d928..03cf36707ae 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1321,9 +1321,11 @@ struct submodule_parallel_fetch {
 
 	struct strbuf submodules_with_errors;
 };
-#define SPF_INIT {0, STRVEC_INIT, NULL, NULL, 0, 0, 0, 0, \
-		  STRING_LIST_INIT_DUP, \
-		  NULL, 0, 0, STRBUF_INIT}
+#define SPF_INIT { \
+	.args = STRVEC_INIT, \
+	.changed_submodule_names = STRING_LIST_INIT_DUP, \
+	.submodules_with_errors = STRBUF_INIT, \
+}
 
 static int get_fetch_recurse_config(const struct submodule *submodule,
 				    struct submodule_parallel_fetch *spf)
diff --git a/submodule.h b/submodule.h
index 4578e501b86..6bd2c99fd99 100644
--- a/submodule.h
+++ b/submodule.h
@@ -37,7 +37,9 @@ struct submodule_update_strategy {
 	enum submodule_update_type type;
 	const char *command;
 };
-#define SUBMODULE_UPDATE_STRATEGY_INIT {SM_UPDATE_UNSPECIFIED, NULL}
+#define SUBMODULE_UPDATE_STRATEGY_INIT { \
+	.type = SM_UPDATE_UNSPECIFIED, \
+}
 
 int is_gitmodules_unmerged(struct index_state *istate);
 int is_writing_gitmodules_ok(void);
diff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c
index 50bb98b7e04..3c4fb862234 100644
--- a/t/helper/test-run-command.c
+++ b/t/helper/test-run-command.c
@@ -60,8 +60,10 @@ struct testsuite {
 	int next;
 	int quiet, immediate, verbose, verbose_log, trace, write_junit_xml;
 };
-#define TESTSUITE_INIT \
-	{ STRING_LIST_INIT_DUP, STRING_LIST_INIT_DUP }
+#define TESTSUITE_INIT { \
+	.tests = STRING_LIST_INIT_DUP, \
+	.failed = STRING_LIST_INIT_DUP, \
+}
 
 static int next_test(struct child_process *cp, struct strbuf *err, void *cb,
 		     void **task_cb)
diff --git a/transport.h b/transport.h
index 1cbab113730..8bb4c8bbc8c 100644
--- a/transport.h
+++ b/transport.h
@@ -262,7 +262,9 @@ struct transport_ls_refs_options {
 	 */
 	char *unborn_head_target;
 };
-#define TRANSPORT_LS_REFS_OPTIONS_INIT { STRVEC_INIT }
+#define TRANSPORT_LS_REFS_OPTIONS_INIT { \
+	.ref_prefixes = STRVEC_INIT, \
+}
 
 /*
  * Retrieve refs from a remote.
-- 
2.33.0.1294.g2bdf2798764

[PATCH 5/5] cbtree.h: define cb_init() in terms of CBTREE_INIT

[Ævar Arnfjörð Bjarmason]

Use the same pattern for cb_init() as the one established in the
recent refactoring of other such patterns in
5726a6b4012 (*.c *_init(): define in terms of corresponding *_INIT
macro, 2021-07-01).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 cbtree.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/cbtree.h b/cbtree.h
index a04a312c3f5..dedbb8e2a45 100644
--- a/cbtree.h
+++ b/cbtree.h
@@ -37,11 +37,12 @@ enum cb_next {
 	CB_BREAK = 1
 };
 
-#define CBTREE_INIT { .root = NULL }
+#define CBTREE_INIT { 0 }
 
 static inline void cb_init(struct cb_tree *t)
 {
-	t->root = NULL;
+	struct cb_tree blank = CBTREE_INIT;
+	memcpy(t, &blank, sizeof(*t));
 }
 
 struct cb_node *cb_lookup(struct cb_tree *, const uint8_t *k, size_t klen);
-- 
2.33.0.1294.g2bdf2798764

Re: [PATCH 5/5] cbtree.h: define cb_init() in terms of CBTREE_INIT

[Johannes Sixt]

Am 27.09.21 um 02:39 schrieb Ævar Arnfjörð Bjarmason:
> --- a/cbtree.h
> +++ b/cbtree.h
> @@ -37,11 +37,12 @@ enum cb_next {
>  	CB_BREAK = 1
>  };
>  
> -#define CBTREE_INIT { .root = NULL }
> +#define CBTREE_INIT { 0 }
>  
>  static inline void cb_init(struct cb_tree *t)
>  {
> -	t->root = NULL;
> +	struct cb_tree blank = CBTREE_INIT;

This could be

	static const struct cb_tree blank = CBTREE_INIT;

> +	memcpy(t, &blank, sizeof(*t));

Is
	*t = blank;

not a thing in C?

-- Hannes

Re: [PATCH 5/5] cbtree.h: define cb_init() in terms of CBTREE_INIT

[Ævar Arnfjörð Bjarmason]

On Mon, Sep 27 2021, Johannes Sixt wrote:

> Am 27.09.21 um 02:39 schrieb Ævar Arnfjörð Bjarmason:
>> --- a/cbtree.h
>> +++ b/cbtree.h
>> @@ -37,11 +37,12 @@ enum cb_next {
>>  	CB_BREAK = 1
>>  };
>>  
>> -#define CBTREE_INIT { .root = NULL }
>> +#define CBTREE_INIT { 0 }
>>  
>>  static inline void cb_init(struct cb_tree *t)
>>  {
>> -	t->root = NULL;
>> +	struct cb_tree blank = CBTREE_INIT;
>
> This could be
>
> 	static const struct cb_tree blank = CBTREE_INIT;

*nod*...

>> +	memcpy(t, &blank, sizeof(*t));
>
> Is
> 	*t = blank;
>
> not a thing in C?
>
> -- Hannes

...but to both this & the above my reply in the side-thread at
https://lore.kernel.org/git/87h7e61duk.fsf@evledraar.gmail.com/
applies. I.e. this is just following a pattern I got from Jeff King &
used in bd4232fac33 (Merge branch 'ab/struct-init', 2021-07-16).

FWIW with "const" in general I don't use it as much as I'd personally
prefer, see e.g. [1] for one recent discussion, but maybe there wouldn't
be any push-back in this case...

1. https://lore.kernel.org/git/patch-1.1-c317e6e125e-20210921T124416Z-avarab@gmail.com/

Re: [PATCH 5/5] cbtree.h: define cb_init() in terms of CBTREE_INIT

[Jeff King]

On Mon, Sep 27, 2021 at 01:02:35PM +0200, Ævar Arnfjörð Bjarmason wrote:

> >>  static inline void cb_init(struct cb_tree *t)
> >>  {
> >> -	t->root = NULL;
> >> +	struct cb_tree blank = CBTREE_INIT;
> >
> > This could be
> >
> > 	static const struct cb_tree blank = CBTREE_INIT;
> 
> *nod*...
> [...]
> ...but to both this & the above my reply in the side-thread at
> https://lore.kernel.org/git/87h7e61duk.fsf@evledraar.gmail.com/
> applies. I.e. this is just following a pattern I got from Jeff King &
> used in bd4232fac33 (Merge branch 'ab/struct-init', 2021-07-16).

I'm not sure how a compiler would react to the "static const" thing. I
tested the compiler output for the "auto" struct case you've written
here, and at least gcc and clang are smart enough to just initialize the
pointed-to struct directly, with no extra copy.

For a "static const" I'm not sure if they'd end up with the same code,
or if they'd allocate a struct in the data segment and just memcpy()
into place. A non-const static would perhaps push it in the direction of the
data/memcpy thing, though the compiler should be well aware that the
struct is never changed nor aliased, and thus we're always writing the
INIT values.

I suspect the performance is not that different either way (the big
thing to avoid is initializing an auto struct on the fly and then
copying from it, but this is a pretty easy optimization for compilers to
get right).

> >> +	memcpy(t, &blank, sizeof(*t));
> >
> > Is
> > 	*t = blank;
> >
> > not a thing in C?

It would be fine to use struct assignment here, and should be equivalent
in most compilers. They know about memcpy() and will inline it as
appropriate.

I think some C programmers tend to prefer memcpy() just because that's
how they think. It also wasn't legal in old K&R compilers, but as far as
I know was in C89.

You have to take care with assignment of flex-structs, of course, but
you also have to do so with memcpy(), too. :)

> FWIW with "const" in general I don't use it as much as I'd personally
> prefer, see e.g. [1] for one recent discussion, but maybe there wouldn't
> be any push-back in this case...

This isn't a parameter, so I don't think that discussion applies. _If_
you are going to make it a static, I think a const makes sense here (but
probably does nothing beyond signaling your intention, because the
compiler can see that it is never modified), but I wouldn't bother with
either.

-Peff

Re: [PATCH 5/5] cbtree.h: define cb_init() in terms of CBTREE_INIT

[Johannes Sixt]

Am 28.09.21 um 01:54 schrieb Jeff King:
> On Mon, Sep 27, 2021 at 01:02:35PM +0200, Ævar Arnfjörð Bjarmason wrote:
> 
>>>>  static inline void cb_init(struct cb_tree *t)
>>>>  {
>>>> -	t->root = NULL;
>>>> +	struct cb_tree blank = CBTREE_INIT;
>>>
>>> This could be
>>>
>>> 	static const struct cb_tree blank = CBTREE_INIT;
>>
>> *nod*...
>> [...]
>> ...but to both this & the above my reply in the side-thread at
>> https://lore.kernel.org/git/87h7e61duk.fsf@evledraar.gmail.com/
>> applies. I.e. this is just following a pattern I got from Jeff King &
>> used in bd4232fac33 (Merge branch 'ab/struct-init', 2021-07-16).
> 
> I'm not sure how a compiler would react to the "static const" thing. I
> tested the compiler output for the "auto" struct case you've written
> here, and at least gcc and clang are smart enough to just initialize the
> pointed-to struct directly, with no extra copy.

Good! Then a deviation from established patterns is not warranted.

-- Hannes

Re: [PATCH 5/5] cbtree.h: define cb_init() in terms of CBTREE_INIT

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

>> >> +	memcpy(t, &blank, sizeof(*t));
>> >
>> > Is
>> > 	*t = blank;
>> >
>> > not a thing in C?
>
> It would be fine to use struct assignment here, and should be equivalent
> in most compilers. They know about memcpy() and will inline it as
> appropriate.

FWIW, I'd be fine with structure assignment, but we already have too
many such memcpy(<ptr>, &<struct>, sizeof(struct)), adding one more
is not giving us too much incremental burden for later clean-up.

> I think some C programmers tend to prefer memcpy() just because that's
> how they think. It also wasn't legal in old K&R compilers, but as far as
> I know was in C89.

I think so, too.

Re: [PATCH 5/5] cbtree.h: define cb_init() in terms of CBTREE_INIT

[Ævar Arnfjörð Bjarmason]

On Tue, Sep 28 2021, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
>
>>> >> +	memcpy(t, &blank, sizeof(*t));
>>> >
>>> > Is
>>> > 	*t = blank;
>>> >
>>> > not a thing in C?
>>
>> It would be fine to use struct assignment here, and should be equivalent
>> in most compilers. They know about memcpy() and will inline it as
>> appropriate.
>
> FWIW, I'd be fine with structure assignment, but we already have too
> many such memcpy(<ptr>, &<struct>, sizeof(struct)), adding one more
> is not giving us too much incremental burden for later clean-up.
>
>> I think some C programmers tend to prefer memcpy() just because that's
>> how they think. It also wasn't legal in old K&R compilers, but as far as
>> I know was in C89.
>
> I think so, too.

Getting back to the topic of this v2 in general, my reading of the
discussion since then is that nothing in it necessitated a v3 re-roll to
address outstanding issues. If I've got that wrong please shout...

Re: [PATCH 5/5] cbtree.h: define cb_init() in terms of CBTREE_INIT

[Junio C Hamano]

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

> On Tue, Sep 28 2021, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>>
>>>> >> +	memcpy(t, &blank, sizeof(*t));
>>>> >
>>>> > Is
>>>> > 	*t = blank;
>>>> >
>>>> > not a thing in C?
>>>
>>> It would be fine to use struct assignment here, and should be equivalent
>>> in most compilers. They know about memcpy() and will inline it as
>>> appropriate.
>>
>> FWIW, I'd be fine with structure assignment, but we already have too
>> many such memcpy(<ptr>, &<struct>, sizeof(struct)), adding one more
>> is not giving us too much incremental burden for later clean-up.
>>
>>> I think some C programmers tend to prefer memcpy() just because that's
>>> how they think. It also wasn't legal in old K&R compilers, but as far as
>>> I know was in C89.
>>
>> I think so, too.
>
> Getting back to the topic of this v2 in general, my reading of the
> discussion since then is that nothing in it necessitated a v3 re-roll to
> address outstanding issues. If I've got that wrong please shout...

I was hoping that these can hit 'next' soonish.

Re: [PATCH 5/5] cbtree.h: define cb_init() in terms of CBTREE_INIT

[Phillip Wood]

Hi Ævar

On 27/09/2021 01:39, Ævar Arnfjörð Bjarmason wrote:
> Use the same pattern for cb_init() as the one established in the
> recent refactoring of other such patterns in
> 5726a6b4012 (*.c *_init(): define in terms of corresponding *_INIT
> macro, 2021-07-01).
> 
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>   cbtree.h | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/cbtree.h b/cbtree.h
> index a04a312c3f5..dedbb8e2a45 100644
> --- a/cbtree.h
> +++ b/cbtree.h
> @@ -37,11 +37,12 @@ enum cb_next {
>   	CB_BREAK = 1
>   };
>   
> -#define CBTREE_INIT { .root = NULL }
> +#define CBTREE_INIT { 0 }
>   
>   static inline void cb_init(struct cb_tree *t)
>   {
> -	t->root = NULL;
> +	struct cb_tree blank = CBTREE_INIT;
> +	memcpy(t, &blank, sizeof(*t));
>   }

Slightly off topic but would this be a good site for a compound literal 
test balloon?

	*t = (struct cb_tree){ 0 };

Compound literals are in C99 and seem to have been supported by MSVC 
since 2013 [1].

Best Wishes

Phillip

[1] 
https://docs.microsoft.com/en-us/cpp/porting/visual-cpp-what-s-new-2003-through-2015?view=msvc-160#whats-new-for-c-in-visual-studio-2013

>   struct cb_node *cb_lookup(struct cb_tree *, const uint8_t *k, size_t klen);
> 

Re: [PATCH 5/5] cbtree.h: define cb_init() in terms of CBTREE_INIT

[Ævar Arnfjörð Bjarmason]

On Mon, Sep 27 2021, Phillip Wood wrote:

> Hi Ævar
>
> On 27/09/2021 01:39, Ævar Arnfjörð Bjarmason wrote:
>> Use the same pattern for cb_init() as the one established in the
>> recent refactoring of other such patterns in
>> 5726a6b4012 (*.c *_init(): define in terms of corresponding *_INIT
>> macro, 2021-07-01).
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> ---
>>   cbtree.h | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>> diff --git a/cbtree.h b/cbtree.h
>> index a04a312c3f5..dedbb8e2a45 100644
>> --- a/cbtree.h
>> +++ b/cbtree.h
>> @@ -37,11 +37,12 @@ enum cb_next {
>>   	CB_BREAK = 1
>>   };
>>   -#define CBTREE_INIT { .root = NULL }
>> +#define CBTREE_INIT { 0 }
>>     static inline void cb_init(struct cb_tree *t)
>>   {
>> -	t->root = NULL;
>> +	struct cb_tree blank = CBTREE_INIT;
>> +	memcpy(t, &blank, sizeof(*t));
>>   }
>
> Slightly off topic but would this be a good site for a compound
> literal test balloon?
>
> 	*t = (struct cb_tree){ 0 };
>
> Compound literals are in C99 and seem to have been supported by MSVC
> since 2013 [1].

I think that's a good thing to test out, FWIW I've also tested it on the
IBM xlc, Oracle SunCC and HP/UX's aCC, they all seem to accept it.

But I'd prefer just doing that in some general follow-up to bd4232fac33
(Merge branch 'ab/struct-init', 2021-07-16), i.e. let's just use the
init pattern it established here.

Re: [PATCH 5/5] cbtree.h: define cb_init() in terms of CBTREE_INIT

[Phillip Wood]

On 27/09/2021 12:00, Ævar Arnfjörð Bjarmason wrote:
> 
> On Mon, Sep 27 2021, Phillip Wood wrote:
> 
>> Hi Ævar
>>
>> On 27/09/2021 01:39, Ævar Arnfjörð Bjarmason wrote:
>>> Use the same pattern for cb_init() as the one established in the
>>> recent refactoring of other such patterns in
>>> 5726a6b4012 (*.c *_init(): define in terms of corresponding *_INIT
>>> macro, 2021-07-01).
>>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>>> ---
>>>    cbtree.h | 5 +++--
>>>    1 file changed, 3 insertions(+), 2 deletions(-)
>>> diff --git a/cbtree.h b/cbtree.h
>>> index a04a312c3f5..dedbb8e2a45 100644
>>> --- a/cbtree.h
>>> +++ b/cbtree.h
>>> @@ -37,11 +37,12 @@ enum cb_next {
>>>    	CB_BREAK = 1
>>>    };
>>>    -#define CBTREE_INIT { .root = NULL }
>>> +#define CBTREE_INIT { 0 }
>>>      static inline void cb_init(struct cb_tree *t)
>>>    {
>>> -	t->root = NULL;
>>> +	struct cb_tree blank = CBTREE_INIT;
>>> +	memcpy(t, &blank, sizeof(*t));
>>>    }
>>
>> Slightly off topic but would this be a good site for a compound
>> literal test balloon?
>>
>> 	*t = (struct cb_tree){ 0 };
>>
>> Compound literals are in C99 and seem to have been supported by MSVC
>> since 2013 [1].
> 
> I think that's a good thing to test out, FWIW I've also tested it on the
> IBM xlc, Oracle SunCC and HP/UX's aCC, they all seem to accept it.

Thanks for taking the time to test those other systems, it's good to 
know they support compound literals

> But I'd prefer just doing that in some general follow-up to bd4232fac33
> (Merge branch 'ab/struct-init', 2021-07-16), i.e. let's just use the
> init pattern it established here.

I agree it makes sense to introduce it as a separate series. I'm not 
sure if there is a pressing need for them but it is the sort of thing 
that is occasionally useful.

Best Wishes

Phillip

[PATCH v2 0/5] Designated initializer cleanup & conversion

[Ævar Arnfjörð Bjarmason]

Migrates more code to designated initializers, see
http://lore.kernel.org/git/cover-0.5-00000000000-20210927T003330Z-avarab@gmail.com
for the v1.

This addresses all the feedback on v1, thanks all! Changes:

 * I'd misread the text around section 6.7.8 in C99 and misconverted
   the CHECKOUT_INIT macro. Thanks to Johannes Sixt for pointing it
   out.

 * Typo change in commit message & elaborate on why I'm not using the
   "*t = (struct cb_tree){ 0 };" pattern in 5/5.

 * Convert the LIST_HEAD_INIT and TRACE_KEY_INIT macros, which I
   missed in v1.

Nothing else changed as far as the end-state is concerned, but as the
range-diff shows I did some of the changes in the previous 4/5
partially in 3/5, e.g. for CACHE_DEF_INIT I first remove the duplicate
and redundant "0" fields, and then convert it to a designated
initializer.

Ævar Arnfjörð Bjarmason (5):
  submodule-config.h: remove unused SUBMODULE_INIT macro
  *.[ch] *_INIT macros: use { 0 } for a "zero out" idiom
  *.h _INIT macros: don't specify fields equal to 0
  *.h: move some *_INIT to designated initializers
  cbtree.h: define cb_init() in terms of CBTREE_INIT

 add-interactive.c                             |  8 +++++--
 builtin/submodule--helper.c                   | 21 ++++++++++---------
 cache.h                                       |  4 +++-
 cbtree.h                                      |  5 +++--
 checkout.c                                    |  2 +-
 .../git-credential-gnome-keyring.c            |  2 +-
 .../libsecret/git-credential-libsecret.c      |  2 +-
 diff.c                                        |  4 ++--
 entry.h                                       |  2 +-
 list.h                                        |  5 ++++-
 lockfile.h                                    |  2 +-
 object-store.h                                |  2 +-
 object.h                                      |  2 +-
 oid-array.h                                   |  2 +-
 path.h                                        |  5 +----
 ref-filter.c                                  |  2 +-
 remote.c                                      |  2 +-
 revision.c                                    |  2 +-
 sequencer.h                                   |  4 +++-
 shallow.h                                     |  4 +++-
 simple-ipc.h                                  |  6 +-----
 strbuf.h                                      |  2 +-
 strvec.h                                      |  4 +++-
 submodule-config.h                            |  4 ----
 submodule.c                                   |  8 ++++---
 submodule.h                                   |  4 +++-
 t/helper/test-run-command.c                   |  6 ++++--
 trace.h                                       |  2 +-
 transport.h                                   |  4 +++-
 29 files changed, 68 insertions(+), 54 deletions(-)

Range-diff against v1:
1:  7a7a0141515 = 1:  7a7a0141515 submodule-config.h: remove unused SUBMODULE_INIT macro
2:  d612e7df7a5 ! 2:  afcd2729c95 *.[ch] *_INIT macros: use { 0 } for a "zero out" idiom
    @@ Commit message
         zero'd out to 0, NULL or '\0', just providing a "{ 0 }" will
         accomplish that.
     
    -    Let's also change change code that provided N zero'd fields to just
    +    Let's also change code that provided N zero'd fields to just
         provide one, and change e.g. "{ NULL }" to "{ 0 }" for
         consistency. I.e. even if the first member is a pointer let's use "0"
         instead of "NULL". The point of using "0" consistently is to pick one,
    @@ diff.c: struct emitted_diff_symbol {
      static void append_emitted_diff_symbol(struct diff_options *o,
      				       struct emitted_diff_symbol *e)
     
    - ## entry.h ##
    -@@ entry.h: struct checkout {
    - 		 clone:1,
    - 		 refresh_cache:1;
    - };
    --#define CHECKOUT_INIT { NULL, "" }
    -+#define CHECKOUT_INIT { 0 }
    - 
    - #define TEMPORARY_FILENAME_LENGTH 25
    - /*
    -
      ## lockfile.h ##
     @@ lockfile.h: struct lock_file {
      	struct tempfile *tempfile;
3:  9e45d2e7bb3 ! 3:  590220bbdcc *.h _INIT macros: don't specify fields equal to 0
    @@ Commit message
         "struct ipc_client_connect_option" struct added in
         59c7b88198a (simple-ipc: add win32 implementation, 2021-03-15).
     
    +    Do the same for a few other initializers, e.g. STRVEC_INIT and
    +    CACHE_DEF_INIT.
    +
         Finally, start incrementally changing the same pattern in
         "t/helper/test-run-command.c". This change was part of an earlier
         on-list version[1] of c90be786da9 (test-tool run-command: fix
    @@ Commit message
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
    + ## cache.h ##
    +@@ cache.h: struct cache_def {
    + 	int track_flags;
    + 	int prefix_len_stat_func;
    + };
    +-#define CACHE_DEF_INIT { STRBUF_INIT, 0, 0, 0 }
    ++#define CACHE_DEF_INIT { STRBUF_INIT }
    + static inline void cache_def_clear(struct cache_def *cache)
    + {
    + 	strbuf_release(&cache->path);
    +
      ## simple-ipc.h ##
     @@ simple-ipc.h: struct ipc_client_connect_options {
      	unsigned int uds_disallow_chdir:1;
    @@ strbuf.h: struct strbuf {
      /*
       * Predeclare this here, since cache.h includes this file before it defines the
     
    + ## strvec.h ##
    +@@ strvec.h: struct strvec {
    + 	size_t alloc;
    + };
    + 
    +-#define STRVEC_INIT { empty_strvec, 0, 0 }
    ++#define STRVEC_INIT { empty_strvec }
    + 
    + /**
    +  * Initialize an array. This is no different than assigning from
    +
    + ## submodule.h ##
    +@@ submodule.h: struct submodule_update_strategy {
    + 	enum submodule_update_type type;
    + 	const char *command;
    + };
    +-#define SUBMODULE_UPDATE_STRATEGY_INIT {SM_UPDATE_UNSPECIFIED, NULL}
    ++#define SUBMODULE_UPDATE_STRATEGY_INIT {SM_UPDATE_UNSPECIFIED}
    + 
    + int is_gitmodules_unmerged(struct index_state *istate);
    + int is_writing_gitmodules_ok(void);
    +
      ## t/helper/test-run-command.c ##
     @@ t/helper/test-run-command.c: struct testsuite {
      	int quiet, immediate, verbose, verbose_log, trace, write_junit_xml;
    @@ t/helper/test-run-command.c: struct testsuite {
      
      static int next_test(struct child_process *cp, struct strbuf *err, void *cb,
      		     void **task_cb)
    +
    + ## trace.h ##
    +@@ trace.h: struct trace_key {
    + 
    + extern struct trace_key trace_default_key;
    + 
    +-#define TRACE_KEY_INIT(name) { "GIT_TRACE_" #name, 0, 0, 0 }
    ++#define TRACE_KEY_INIT(name) { "GIT_TRACE_" #name }
    + extern struct trace_key trace_perf_key;
    + extern struct trace_key trace_setup_key;
    + 
4:  1f364565111 ! 4:  dd4ec1a0219 *.h: move some *_INIT to designated initializers
    @@ cache.h: struct cache_def {
      	int track_flags;
      	int prefix_len_stat_func;
      };
    --#define CACHE_DEF_INIT { STRBUF_INIT, 0, 0, 0 }
    +-#define CACHE_DEF_INIT { STRBUF_INIT }
     +#define CACHE_DEF_INIT { \
     +	.path = STRBUF_INIT, \
     +}
    @@ cache.h: struct cache_def {
      {
      	strbuf_release(&cache->path);
     
    + ## entry.h ##
    +@@ entry.h: struct checkout {
    + 		 clone:1,
    + 		 refresh_cache:1;
    + };
    +-#define CHECKOUT_INIT { NULL, "" }
    ++#define CHECKOUT_INIT { .base_dir = "" }
    + 
    + #define TEMPORARY_FILENAME_LENGTH 25
    + /*
    +
    + ## list.h ##
    +@@ list.h: struct list_head {
    + #define INIT_LIST_HEAD(ptr) \
    + 	(ptr)->next = (ptr)->prev = (ptr)
    + 
    +-#define LIST_HEAD_INIT(name) { &(name), &(name) }
    ++#define LIST_HEAD_INIT(name) { \
    ++	.next = &(name), \
    ++	.prev = &(name), \
    ++}
    + 
    + /* Add new element at the head of the list. */
    + static inline void list_add(struct list_head *newp, struct list_head *head)
    +
      ## sequencer.h ##
     @@ sequencer.h: struct todo_list {
      	struct stat_data stat;
    @@ strvec.h: struct strvec {
      	size_t alloc;
      };
      
    --#define STRVEC_INIT { empty_strvec, 0, 0 }
    +-#define STRVEC_INIT { empty_strvec }
     +#define STRVEC_INIT { \
     +	.v = empty_strvec, \
     +}
    @@ submodule.h: struct submodule_update_strategy {
      	enum submodule_update_type type;
      	const char *command;
      };
    --#define SUBMODULE_UPDATE_STRATEGY_INIT {SM_UPDATE_UNSPECIFIED, NULL}
    +-#define SUBMODULE_UPDATE_STRATEGY_INIT {SM_UPDATE_UNSPECIFIED}
     +#define SUBMODULE_UPDATE_STRATEGY_INIT { \
     +	.type = SM_UPDATE_UNSPECIFIED, \
     +}
    @@ t/helper/test-run-command.c: struct testsuite {
      static int next_test(struct child_process *cp, struct strbuf *err, void *cb,
      		     void **task_cb)
     
    + ## trace.h ##
    +@@ trace.h: struct trace_key {
    + 
    + extern struct trace_key trace_default_key;
    + 
    +-#define TRACE_KEY_INIT(name) { "GIT_TRACE_" #name }
    ++#define TRACE_KEY_INIT(name) { .key = "GIT_TRACE_" #name }
    + extern struct trace_key trace_perf_key;
    + extern struct trace_key trace_setup_key;
    + 
    +
      ## transport.h ##
     @@ transport.h: struct transport_ls_refs_options {
      	 */
5:  7e571667674 ! 5:  76b47e7c80a cbtree.h: define cb_init() in terms of CBTREE_INIT
    @@ Commit message
         5726a6b4012 (*.c *_init(): define in terms of corresponding *_INIT
         macro, 2021-07-01).
     
    +    It has been pointed out[1] that we could perhaps use this C99
    +    replacement of using a compound literal for all of these:
    +
    +        *t = (struct cb_tree){ 0 };
    +
    +    But let's just stick to the existing pattern established in
    +    5726a6b4012 for now, we can leave another weather balloon for some
    +    other time.
    +
    +    1. http://lore.kernel.org/git/ef724a3a-a4b8-65d3-c928-13a7d78f189a@gmail.com
    +
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## cbtree.h ##
-- 
2.33.0.1316.gb2e9b3ba3ae

[PATCH v2 1/5] submodule-config.h: remove unused SUBMODULE_INIT macro

[Ævar Arnfjörð Bjarmason]

This macro was added and used in c68f8375760 (implement fetching of
moved submodules, 2017-10-16) but its last user went away in
be76c212823 (fetch: ensure submodule objects fetched, 2018-12-06).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 submodule-config.h | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/submodule-config.h b/submodule-config.h
index c11e22cf509..65875b94ea5 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -45,10 +45,6 @@ struct submodule {
 	struct object_id gitmodules_oid;
 	int recommend_shallow;
 };
-
-#define SUBMODULE_INIT { NULL, NULL, NULL, RECURSE_SUBMODULES_NONE, \
-	NULL, NULL, SUBMODULE_UPDATE_STRATEGY_INIT, { { 0 } }, -1 };
-
 struct submodule_cache;
 struct repository;
 
-- 
2.33.0.1316.gb2e9b3ba3ae

Re: [PATCH v2 1/5] submodule-config.h: remove unused SUBMODULE_INIT macro

[Jeff King]

On Mon, Sep 27, 2021 at 02:54:24PM +0200, Ævar Arnfjörð Bjarmason wrote:

> This macro was added and used in c68f8375760 (implement fetching of
> moved submodules, 2017-10-16) but its last user went away in
> be76c212823 (fetch: ensure submodule objects fetched, 2018-12-06).
> 
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  submodule-config.h | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/submodule-config.h b/submodule-config.h
> index c11e22cf509..65875b94ea5 100644
> --- a/submodule-config.h
> +++ b/submodule-config.h
> @@ -45,10 +45,6 @@ struct submodule {
>  	struct object_id gitmodules_oid;
>  	int recommend_shallow;
>  };
> -
> -#define SUBMODULE_INIT { NULL, NULL, NULL, RECURSE_SUBMODULES_NONE, \
> -	NULL, NULL, SUBMODULE_UPDATE_STRATEGY_INIT, { { 0 } }, -1 };

I was a bit surprised by this one, just because we generally prefer the
builtin initializers to init functions. And even if we are only using
the latter, I like the move to implementing it in terms of the former.

But this struct is extra funny, as now it will have neither. It comes
only from submodule_from_name() or submodule_from_path(), which in turn
are building up from config and the initialization in
lookup_or_create_by_name(). And keeping those two in sync is potentially
error-prone.

So this whole "make a submodule struct on the stack" thing is pretty odd
in the first place, and I'm happy to see this initializer going away.

-Peff

[PATCH v2 2/5] *.[ch] *_INIT macros: use { 0 } for a "zero out" idiom

[Ævar Arnfjörð Bjarmason]

In C it isn't required to specify that all members of a struct are
zero'd out to 0, NULL or '\0', just providing a "{ 0 }" will
accomplish that.

Let's also change code that provided N zero'd fields to just
provide one, and change e.g. "{ NULL }" to "{ 0 }" for
consistency. I.e. even if the first member is a pointer let's use "0"
instead of "NULL". The point of using "0" consistently is to pick one,
and to not have the reader wonder why we're not using the same pattern
everywhere.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/submodule--helper.c                      | 16 ++++++++--------
 checkout.c                                       |  2 +-
 .../gnome-keyring/git-credential-gnome-keyring.c |  2 +-
 .../libsecret/git-credential-libsecret.c         |  2 +-
 diff.c                                           |  4 ++--
 lockfile.h                                       |  2 +-
 object-store.h                                   |  2 +-
 object.h                                         |  2 +-
 oid-array.h                                      |  2 +-
 path.h                                           |  5 +----
 ref-filter.c                                     |  2 +-
 remote.c                                         |  2 +-
 revision.c                                       |  2 +-
 13 files changed, 21 insertions(+), 24 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 5336daf186d..deca75c83ee 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -307,7 +307,7 @@ struct module_list {
 	const struct cache_entry **entries;
 	int alloc, nr;
 };
-#define MODULE_LIST_INIT { NULL, 0, 0 }
+#define MODULE_LIST_INIT { 0 }
 
 static int module_list_compute(int argc, const char **argv,
 			       const char *prefix,
@@ -588,7 +588,7 @@ struct init_cb {
 	const char *prefix;
 	unsigned int flags;
 };
-#define INIT_CB_INIT { NULL, 0 }
+#define INIT_CB_INIT { 0 }
 
 static void init_submodule(const char *path, const char *prefix,
 			   unsigned int flags)
@@ -717,7 +717,7 @@ struct status_cb {
 	const char *prefix;
 	unsigned int flags;
 };
-#define STATUS_CB_INIT { NULL, 0 }
+#define STATUS_CB_INIT { 0 }
 
 static void print_status(unsigned int flags, char state, const char *path,
 			 const struct object_id *oid, const char *displaypath)
@@ -911,13 +911,13 @@ struct module_cb {
 	char status;
 	const char *sm_path;
 };
-#define MODULE_CB_INIT { 0, 0, NULL, NULL, '\0', NULL }
+#define MODULE_CB_INIT { 0 }
 
 struct module_cb_list {
 	struct module_cb **entries;
 	int alloc, nr;
 };
-#define MODULE_CB_LIST_INIT { NULL, 0, 0 }
+#define MODULE_CB_LIST_INIT { 0 }
 
 struct summary_cb {
 	int argc;
@@ -928,7 +928,7 @@ struct summary_cb {
 	unsigned int files: 1;
 	int summary_limit;
 };
-#define SUMMARY_CB_INIT { 0, NULL, NULL, 0, 0, 0, 0 }
+#define SUMMARY_CB_INIT { 0 }
 
 enum diff_cmd {
 	DIFF_INDEX,
@@ -1334,7 +1334,7 @@ struct sync_cb {
 	const char *prefix;
 	unsigned int flags;
 };
-#define SYNC_CB_INIT { NULL, 0 }
+#define SYNC_CB_INIT { 0 }
 
 static void sync_submodule(const char *path, const char *prefix,
 			   unsigned int flags)
@@ -1480,7 +1480,7 @@ struct deinit_cb {
 	const char *prefix;
 	unsigned int flags;
 };
-#define DEINIT_CB_INIT { NULL, 0 }
+#define DEINIT_CB_INIT { 0 }
 
 static void deinit_submodule(const char *path, const char *prefix,
 			     unsigned int flags)
diff --git a/checkout.c b/checkout.c
index 6586e30ca5a..2e39dae684f 100644
--- a/checkout.c
+++ b/checkout.c
@@ -14,7 +14,7 @@ struct tracking_name_data {
 	struct object_id *default_dst_oid;
 };
 
-#define TRACKING_NAME_DATA_INIT { NULL, NULL, NULL, 0, NULL, NULL, NULL }
+#define TRACKING_NAME_DATA_INIT { 0 }
 
 static int check_tracking_name(struct remote *remote, void *cb_data)
 {
diff --git a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
index d389bfadcee..5927e27ae6e 100644
--- a/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
+++ b/contrib/credential/gnome-keyring/git-credential-gnome-keyring.c
@@ -138,7 +138,7 @@ struct credential {
 	char *password;
 };
 
-#define CREDENTIAL_INIT { NULL, NULL, 0, NULL, NULL, NULL }
+#define CREDENTIAL_INIT { 0 }
 
 typedef int (*credential_op_cb)(struct credential *);
 
diff --git a/contrib/credential/libsecret/git-credential-libsecret.c b/contrib/credential/libsecret/git-credential-libsecret.c
index e6598b63833..2c5d76d789f 100644
--- a/contrib/credential/libsecret/git-credential-libsecret.c
+++ b/contrib/credential/libsecret/git-credential-libsecret.c
@@ -41,7 +41,7 @@ struct credential {
 	char *password;
 };
 
-#define CREDENTIAL_INIT { NULL, NULL, 0, NULL, NULL, NULL }
+#define CREDENTIAL_INIT { 0 }
 
 typedef int (*credential_op_cb)(struct credential *);
 
diff --git a/diff.c b/diff.c
index c8f530ffdbe..861282db1c3 100644
--- a/diff.c
+++ b/diff.c
@@ -775,13 +775,13 @@ struct emitted_diff_symbol {
 	int indent_width; /* The visual width of the indentation */
 	enum diff_symbol s;
 };
-#define EMITTED_DIFF_SYMBOL_INIT {NULL}
+#define EMITTED_DIFF_SYMBOL_INIT { 0 }
 
 struct emitted_diff_symbols {
 	struct emitted_diff_symbol *buf;
 	int nr, alloc;
 };
-#define EMITTED_DIFF_SYMBOLS_INIT {NULL, 0, 0}
+#define EMITTED_DIFF_SYMBOLS_INIT { 0 }
 
 static void append_emitted_diff_symbol(struct diff_options *o,
 				       struct emitted_diff_symbol *e)
diff --git a/lockfile.h b/lockfile.h
index db93e6ba73e..90af4e66b28 100644
--- a/lockfile.h
+++ b/lockfile.h
@@ -121,7 +121,7 @@ struct lock_file {
 	struct tempfile *tempfile;
 };
 
-#define LOCK_INIT { NULL }
+#define LOCK_INIT { 0 }
 
 /* String appended to a filename to derive the lockfile name: */
 #define LOCK_SUFFIX ".lock"
diff --git a/object-store.h b/object-store.h
index c5130d8baea..1e647a5be30 100644
--- a/object-store.h
+++ b/object-store.h
@@ -371,7 +371,7 @@ struct object_info {
  * Initializer for a "struct object_info" that wants no items. You may
  * also memset() the memory to all-zeroes.
  */
-#define OBJECT_INFO_INIT {NULL}
+#define OBJECT_INFO_INIT { 0 }
 
 /* Invoke lookup_replace_object() on the given hash */
 #define OBJECT_INFO_LOOKUP_REPLACE 1
diff --git a/object.h b/object.h
index 549f2d256bc..cb556ab7753 100644
--- a/object.h
+++ b/object.h
@@ -55,7 +55,7 @@ struct object_array {
 	} *objects;
 };
 
-#define OBJECT_ARRAY_INIT { 0, 0, NULL }
+#define OBJECT_ARRAY_INIT { 0 }
 
 /*
  * object flag allocation:
diff --git a/oid-array.h b/oid-array.h
index 72bca78b7dc..f60f9af6741 100644
--- a/oid-array.h
+++ b/oid-array.h
@@ -56,7 +56,7 @@ struct oid_array {
 	int sorted;
 };
 
-#define OID_ARRAY_INIT { NULL, 0, 0, 0 }
+#define OID_ARRAY_INIT { 0 }
 
 /**
  * Add an item to the set. The object ID will be placed at the end of the array
diff --git a/path.h b/path.h
index 251c78d9800..b68691a86b8 100644
--- a/path.h
+++ b/path.h
@@ -181,10 +181,7 @@ struct path_cache {
 	const char *shallow;
 };
 
-#define PATH_CACHE_INIT                                        \
-	{                                                      \
-		NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL \
-	}
+#define PATH_CACHE_INIT { 0 }
 
 const char *git_path_squash_msg(struct repository *r);
 const char *git_path_merge_msg(struct repository *r);
diff --git a/ref-filter.c b/ref-filter.c
index 93ce2a6ef2e..eee4f9b17c5 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -633,7 +633,7 @@ static struct {
 	 */
 };
 
-#define REF_FORMATTING_STATE_INIT  { 0, NULL }
+#define REF_FORMATTING_STATE_INIT  { 0 }
 
 struct ref_formatting_stack {
 	struct ref_formatting_stack *prev;
diff --git a/remote.c b/remote.c
index 31e141b01fa..f958543d707 100644
--- a/remote.c
+++ b/remote.c
@@ -2403,7 +2403,7 @@ struct reflog_commit_array {
 	size_t nr, alloc;
 };
 
-#define REFLOG_COMMIT_ARRAY_INIT { NULL, 0, 0 }
+#define REFLOG_COMMIT_ARRAY_INIT { 0 }
 
 /* Append a commit to the array. */
 static void append_commit(struct reflog_commit_array *arr,
diff --git a/revision.c b/revision.c
index 0dabb5a0bcf..73e5004d60b 100644
--- a/revision.c
+++ b/revision.c
@@ -249,7 +249,7 @@ struct commit_stack {
 	struct commit **items;
 	size_t nr, alloc;
 };
-#define COMMIT_STACK_INIT { NULL, 0, 0 }
+#define COMMIT_STACK_INIT { 0 }
 
 static void commit_stack_push(struct commit_stack *stack, struct commit *commit)
 {
-- 
2.33.0.1316.gb2e9b3ba3ae

Re: [PATCH v2 2/5] *.[ch] *_INIT macros: use { 0 } for a "zero out" idiom

[Jeff King]

On Mon, Sep 27, 2021 at 02:54:25PM +0200, Ævar Arnfjörð Bjarmason wrote:

> In C it isn't required to specify that all members of a struct are
> zero'd out to 0, NULL or '\0', just providing a "{ 0 }" will
> accomplish that.
> 
> Let's also change code that provided N zero'd fields to just
> provide one, and change e.g. "{ NULL }" to "{ 0 }" for
> consistency. I.e. even if the first member is a pointer let's use "0"
> instead of "NULL". The point of using "0" consistently is to pick one,
> and to not have the reader wonder why we're not using the same pattern
> everywhere.

I seem to recall we've had some linter complaints about using "0" to
initialize a pointer, but I think these days it's OK, per:

 - 1c96642326 (sparse: allow '{ 0 }' to be used without warnings,
   2020-05-22)

and

 - https://lore.kernel.org/git/18bd6127-be72-b7b7-8e2a-17bbe7214a2e@ramsayjones.plus.com/

I think this is a good step, as the long lists are unwieldy and difficult to
keep up to date without actually providing any readability or functional
value.

-Peff

Re: [PATCH v2 2/5] *.[ch] *_INIT macros: use { 0 } for a "zero out" idiom

[Ævar Arnfjörð Bjarmason]

On Mon, Sep 27 2021, Jeff King wrote:

> On Mon, Sep 27, 2021 at 02:54:25PM +0200, Ævar Arnfjörð Bjarmason wrote:
>
>> In C it isn't required to specify that all members of a struct are
>> zero'd out to 0, NULL or '\0', just providing a "{ 0 }" will
>> accomplish that.
>> 
>> Let's also change code that provided N zero'd fields to just
>> provide one, and change e.g. "{ NULL }" to "{ 0 }" for
>> consistency. I.e. even if the first member is a pointer let's use "0"
>> instead of "NULL". The point of using "0" consistently is to pick one,
>> and to not have the reader wonder why we're not using the same pattern
>> everywhere.
>
> I seem to recall we've had some linter complaints about using "0" to
> initialize a pointer, but I think these days it's OK, per:
>
>  - 1c96642326 (sparse: allow '{ 0 }' to be used without warnings,
>    2020-05-22)
>
> and
>
>  - https://lore.kernel.org/git/18bd6127-be72-b7b7-8e2a-17bbe7214a2e@ramsayjones.plus.com/
>
> I think this is a good step, as the long lists are unwieldy and difficult to
> keep up to date without actually providing any readability or functional
> value.

[+CC Luc Van Oostenryck <luc.vanoostenryck@gmail.com>]

It seems like we should just revert 1c96642326, looking at the history
of sparse.git there's:

 - 537e3e2d (univ-init: conditionally accept { 0 } without warnings, 2020-05-18)

Followed by git.git's 1c96642326 a few days later, but then in sparse.git:

 - 41f651b4 (univ-init: set default to -Wno-universal-initializer, 2020-05-29)

I.e. a few days after the workaround in git.git the upstream repo
changed the default. The 537e3e2d isn't in any release of sparse that
41f651b4 isn't in, they both first appeared in v0.6.2.

So us having -Wno-universal-initializer only seems useful if you're
using some old commit in sparse.git.

Having written the above I found
https://lore.kernel.org/git/20200530162432.a7fitzjc53hsn2ej@ltop.local/;
i.e. sparse's maintainer pretty much saying the same thing.

Re: [PATCH v2 2/5] *.[ch] *_INIT macros: use { 0 } for a "zero out" idiom

[Jeff King]

On Tue, Sep 28, 2021 at 02:25:16AM +0200, Ævar Arnfjörð Bjarmason wrote:

> It seems like we should just revert 1c96642326, looking at the history
> of sparse.git there's:
> 
>  - 537e3e2d (univ-init: conditionally accept { 0 } without warnings, 2020-05-18)
> 
> Followed by git.git's 1c96642326 a few days later, but then in sparse.git:
> 
>  - 41f651b4 (univ-init: set default to -Wno-universal-initializer, 2020-05-29)
> 
> I.e. a few days after the workaround in git.git the upstream repo
> changed the default. The 537e3e2d isn't in any release of sparse that
> 41f651b4 isn't in, they both first appeared in v0.6.2.
> 
> So us having -Wno-universal-initializer only seems useful if you're
> using some old commit in sparse.git.
> 
> Having written the above I found
> https://lore.kernel.org/git/20200530162432.a7fitzjc53hsn2ej@ltop.local/;
> i.e. sparse's maintainer pretty much saying the same thing.

Yeah, that seems reasonable. If somebody has an old version of sparse
they'll presumably see actual "don't use 0 to initialize a pointer"
warnings, as opposed to "hey, I don't understand -Wno-universal-initializer".
But either way, they should upgrade.

-Peff

Re: [PATCH v2 2/5] *.[ch] *_INIT macros: use { 0 } for a "zero out" idiom

[Ramsay Jones]

On 28/09/2021 01:25, Ævar Arnfjörð Bjarmason wrote:
> 
> On Mon, Sep 27 2021, Jeff King wrote:
> 
>> On Mon, Sep 27, 2021 at 02:54:25PM +0200, Ævar Arnfjörð Bjarmason wrote:
>>
>>> In C it isn't required to specify that all members of a struct are
>>> zero'd out to 0, NULL or '\0', just providing a "{ 0 }" will
>>> accomplish that.
>>>
>>> Let's also change code that provided N zero'd fields to just
>>> provide one, and change e.g. "{ NULL }" to "{ 0 }" for
>>> consistency. I.e. even if the first member is a pointer let's use "0"
>>> instead of "NULL". The point of using "0" consistently is to pick one,
>>> and to not have the reader wonder why we're not using the same pattern
>>> everywhere.
>>
>> I seem to recall we've had some linter complaints about using "0" to
>> initialize a pointer, but I think these days it's OK, per:
>>
>>  - 1c96642326 (sparse: allow '{ 0 }' to be used without warnings,
>>    2020-05-22)
>>
>> and
>>
>>  - https://lore.kernel.org/git/18bd6127-be72-b7b7-8e2a-17bbe7214a2e@ramsayjones.plus.com/
>>
>> I think this is a good step, as the long lists are unwieldy and difficult to
>> keep up to date without actually providing any readability or functional
>> value.
> 
> [+CC Luc Van Oostenryck <luc.vanoostenryck@gmail.com>]
> 
> It seems like we should just revert 1c96642326, looking at the history
> of sparse.git there's:
> 
>  - 537e3e2d (univ-init: conditionally accept { 0 } without warnings, 2020-05-18)
> 
> Followed by git.git's 1c96642326 a few days later, but then in sparse.git:
> 
>  - 41f651b4 (univ-init: set default to -Wno-universal-initializer, 2020-05-29)
> 
> I.e. a few days after the workaround in git.git the upstream repo
> changed the default. The 537e3e2d isn't in any release of sparse that
> 41f651b4 isn't in, they both first appeared in v0.6.2.
> 
> So us having -Wno-universal-initializer only seems useful if you're
> using some old commit in sparse.git.
> 
> Having written the above I found
> https://lore.kernel.org/git/20200530162432.a7fitzjc53hsn2ej@ltop.local/;
> i.e. sparse's maintainer pretty much saying the same thing.

Yes, this has been on my TODO list pretty much since commit 41f651b4, but
it wasn't a priority. ;-)

ATB,
Ramsay Jones

[PATCH v2 3/5] *.h _INIT macros: don't specify fields equal to 0

[Ævar Arnfjörð Bjarmason]

Change the initialization of "struct strbuf" changed in
cbc0f81d96f (strbuf: use designated initializers in STRBUF_INIT,
2017-07-10) to omit specifying "alloc" and "len", as we do with other
"alloc" and "len" (or "nr") in similar structs.

Let's likewise omit the explicit initialization of all fields in the
"struct ipc_client_connect_option" struct added in
59c7b88198a (simple-ipc: add win32 implementation, 2021-03-15).

Do the same for a few other initializers, e.g. STRVEC_INIT and
CACHE_DEF_INIT.

Finally, start incrementally changing the same pattern in
"t/helper/test-run-command.c". This change was part of an earlier
on-list version[1] of c90be786da9 (test-tool run-command: fix
flip-flop init pattern, 2021-09-11).

1. https://lore.kernel.org/git/patch-1.1-0aa4523ab6e-20210909T130849Z-avarab@gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 cache.h                     | 2 +-
 simple-ipc.h                | 6 +-----
 strbuf.h                    | 2 +-
 strvec.h                    | 2 +-
 submodule.h                 | 2 +-
 t/helper/test-run-command.c | 2 +-
 trace.h                     | 2 +-
 7 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/cache.h b/cache.h
index f6295f3b048..25c6b0b1200 100644
--- a/cache.h
+++ b/cache.h
@@ -1668,7 +1668,7 @@ struct cache_def {
 	int track_flags;
 	int prefix_len_stat_func;
 };
-#define CACHE_DEF_INIT { STRBUF_INIT, 0, 0, 0 }
+#define CACHE_DEF_INIT { STRBUF_INIT }
 static inline void cache_def_clear(struct cache_def *cache)
 {
 	strbuf_release(&cache->path);
diff --git a/simple-ipc.h b/simple-ipc.h
index 2c48a5ee004..08b2908d5f8 100644
--- a/simple-ipc.h
+++ b/simple-ipc.h
@@ -65,11 +65,7 @@ struct ipc_client_connect_options {
 	unsigned int uds_disallow_chdir:1;
 };
 
-#define IPC_CLIENT_CONNECT_OPTIONS_INIT { \
-	.wait_if_busy = 0, \
-	.wait_if_not_found = 0, \
-	.uds_disallow_chdir = 0, \
-}
+#define IPC_CLIENT_CONNECT_OPTIONS_INIT { 0 }
 
 /*
  * Determine if a server is listening on this named pipe or socket using
diff --git a/strbuf.h b/strbuf.h
index 5b1113abf8f..3b36bbc49f0 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -70,7 +70,7 @@ struct strbuf {
 };
 
 extern char strbuf_slopbuf[];
-#define STRBUF_INIT  { .alloc = 0, .len = 0, .buf = strbuf_slopbuf }
+#define STRBUF_INIT  { .buf = strbuf_slopbuf }
 
 /*
  * Predeclare this here, since cache.h includes this file before it defines the
diff --git a/strvec.h b/strvec.h
index 6b3cbd67589..a10aad5f645 100644
--- a/strvec.h
+++ b/strvec.h
@@ -33,7 +33,7 @@ struct strvec {
 	size_t alloc;
 };
 
-#define STRVEC_INIT { empty_strvec, 0, 0 }
+#define STRVEC_INIT { empty_strvec }
 
 /**
  * Initialize an array. This is no different than assigning from
diff --git a/submodule.h b/submodule.h
index 4578e501b86..35d18c7868a 100644
--- a/submodule.h
+++ b/submodule.h
@@ -37,7 +37,7 @@ struct submodule_update_strategy {
 	enum submodule_update_type type;
 	const char *command;
 };
-#define SUBMODULE_UPDATE_STRATEGY_INIT {SM_UPDATE_UNSPECIFIED, NULL}
+#define SUBMODULE_UPDATE_STRATEGY_INIT {SM_UPDATE_UNSPECIFIED}
 
 int is_gitmodules_unmerged(struct index_state *istate);
 int is_writing_gitmodules_ok(void);
diff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c
index 14c57365e76..50bb98b7e04 100644
--- a/t/helper/test-run-command.c
+++ b/t/helper/test-run-command.c
@@ -61,7 +61,7 @@ struct testsuite {
 	int quiet, immediate, verbose, verbose_log, trace, write_junit_xml;
 };
 #define TESTSUITE_INIT \
-	{ STRING_LIST_INIT_DUP, STRING_LIST_INIT_DUP, 0, 0, 0, 0, 0, 0, 0 }
+	{ STRING_LIST_INIT_DUP, STRING_LIST_INIT_DUP }
 
 static int next_test(struct child_process *cp, struct strbuf *err, void *cb,
 		     void **task_cb)
diff --git a/trace.h b/trace.h
index 0dbbad0e41c..74f62919c57 100644
--- a/trace.h
+++ b/trace.h
@@ -89,7 +89,7 @@ struct trace_key {
 
 extern struct trace_key trace_default_key;
 
-#define TRACE_KEY_INIT(name) { "GIT_TRACE_" #name, 0, 0, 0 }
+#define TRACE_KEY_INIT(name) { "GIT_TRACE_" #name }
 extern struct trace_key trace_perf_key;
 extern struct trace_key trace_setup_key;
 
-- 
2.33.0.1316.gb2e9b3ba3ae

Re: [PATCH v2 3/5] *.h _INIT macros: don't specify fields equal to 0

[Junio C Hamano]

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

> -#define IPC_CLIENT_CONNECT_OPTIONS_INIT { \
> -	.wait_if_busy = 0, \
> -	.wait_if_not_found = 0, \
> -	.uds_disallow_chdir = 0, \
> -}
> +#define IPC_CLIENT_CONNECT_OPTIONS_INIT { 0 }

The original explicitly initializes the members using designated
init, and the loss somewhat feels like a temporary regression, but
as long as the new norm is

 - If there is *no* member that is initialized to non-zero value, we
   use { 0 }, and

 - Otherwise, we only explicitly initialize members to non-zero
   value using designated initializers

this conversion is perfectly fine.

[PATCH v2 4/5] *.h: move some *_INIT to designated initializers

[Ævar Arnfjörð Bjarmason]

Move various *_INIT macros to use designated initializers. This helps
readability. I've only picked those leftover macros that were not
touched by another in-flight series of mine which changed others, but
also how initialization was done.

In the case of SUBMODULE_ALTERNATE_SETUP_INIT I've left an explicit
initialization of "error_mode", even though
SUBMODULE_ALTERNATE_ERROR_IGNORE itself is defined as "0". Let's not
peek under the hood and assume that enum fields we know the value of
will stay at "0".

The change to "TESTSUITE_INIT" in "t/helper/test-run-command.c" was
part of an earlier on-list version[1] of c90be786da9 (test-tool
run-command: fix flip-flop init pattern, 2021-09-11).

1. https://lore.kernel.org/git/patch-1.1-0aa4523ab6e-20210909T130849Z-avarab@gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 add-interactive.c           | 8 ++++++--
 builtin/submodule--helper.c | 5 +++--
 cache.h                     | 4 +++-
 entry.h                     | 2 +-
 list.h                      | 5 ++++-
 sequencer.h                 | 4 +++-
 shallow.h                   | 4 +++-
 strvec.h                    | 4 +++-
 submodule.c                 | 8 +++++---
 submodule.h                 | 4 +++-
 t/helper/test-run-command.c | 6 ++++--
 trace.h                     | 2 +-
 transport.h                 | 4 +++-
 13 files changed, 42 insertions(+), 18 deletions(-)

diff --git a/add-interactive.c b/add-interactive.c
index 36ebdbdf7e2..6498ae196f1 100644
--- a/add-interactive.c
+++ b/add-interactive.c
@@ -102,8 +102,12 @@ struct prefix_item_list {
 	int *selected; /* for multi-selections */
 	size_t min_length, max_length;
 };
-#define PREFIX_ITEM_LIST_INIT \
-	{ STRING_LIST_INIT_DUP, STRING_LIST_INIT_NODUP, NULL, 1, 4 }
+#define PREFIX_ITEM_LIST_INIT { \
+	.items = STRING_LIST_INIT_DUP, \
+	.sorted = STRING_LIST_INIT_NODUP, \
+	.min_length = 1, \
+	.max_length = 4, \
+}
 
 static void prefix_item_list_clear(struct prefix_item_list *list)
 {
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index deca75c83ee..57f09925157 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1647,8 +1647,9 @@ struct submodule_alternate_setup {
 	} error_mode;
 	struct string_list *reference;
 };
-#define SUBMODULE_ALTERNATE_SETUP_INIT { NULL, \
-	SUBMODULE_ALTERNATE_ERROR_IGNORE, NULL }
+#define SUBMODULE_ALTERNATE_SETUP_INIT { \
+	.error_mode = SUBMODULE_ALTERNATE_ERROR_IGNORE, \
+}
 
 static const char alternate_error_advice[] = N_(
 "An alternate computed from a superproject's alternate is invalid.\n"
diff --git a/cache.h b/cache.h
index 25c6b0b1200..794237515b9 100644
--- a/cache.h
+++ b/cache.h
@@ -1668,7 +1668,9 @@ struct cache_def {
 	int track_flags;
 	int prefix_len_stat_func;
 };
-#define CACHE_DEF_INIT { STRBUF_INIT }
+#define CACHE_DEF_INIT { \
+	.path = STRBUF_INIT, \
+}
 static inline void cache_def_clear(struct cache_def *cache)
 {
 	strbuf_release(&cache->path);
diff --git a/entry.h b/entry.h
index 7c889e58fd7..2254c62727f 100644
--- a/entry.h
+++ b/entry.h
@@ -16,7 +16,7 @@ struct checkout {
 		 clone:1,
 		 refresh_cache:1;
 };
-#define CHECKOUT_INIT { NULL, "" }
+#define CHECKOUT_INIT { .base_dir = "" }
 
 #define TEMPORARY_FILENAME_LENGTH 25
 /*
diff --git a/list.h b/list.h
index eb601192f4c..362a4cd7f5f 100644
--- a/list.h
+++ b/list.h
@@ -46,7 +46,10 @@ struct list_head {
 #define INIT_LIST_HEAD(ptr) \
 	(ptr)->next = (ptr)->prev = (ptr)
 
-#define LIST_HEAD_INIT(name) { &(name), &(name) }
+#define LIST_HEAD_INIT(name) { \
+	.next = &(name), \
+	.prev = &(name), \
+}
 
 /* Add new element at the head of the list. */
 static inline void list_add(struct list_head *newp, struct list_head *head)
diff --git a/sequencer.h b/sequencer.h
index 2088344cb37..259d4eb4a8a 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -119,7 +119,9 @@ struct todo_list {
 	struct stat_data stat;
 };
 
-#define TODO_LIST_INIT { STRBUF_INIT }
+#define TODO_LIST_INIT { \
+	.buf = STRBUF_INIT, \
+}
 
 int todo_list_parse_insn_buffer(struct repository *r, char *buf,
 				struct todo_list *todo_list);
diff --git a/shallow.h b/shallow.h
index 5b4a96dcd69..aba6ff58294 100644
--- a/shallow.h
+++ b/shallow.h
@@ -23,7 +23,9 @@ int is_repository_shallow(struct repository *r);
 struct shallow_lock {
 	struct lock_file lock;
 };
-#define SHALLOW_LOCK_INIT { LOCK_INIT }
+#define SHALLOW_LOCK_INIT { \
+	.lock = LOCK_INIT, \
+}
 
 /* commit $GIT_DIR/shallow and reset stat-validity checks */
 int commit_shallow_file(struct repository *r, struct shallow_lock *lk);
diff --git a/strvec.h b/strvec.h
index a10aad5f645..9f55c8766ba 100644
--- a/strvec.h
+++ b/strvec.h
@@ -33,7 +33,9 @@ struct strvec {
 	size_t alloc;
 };
 
-#define STRVEC_INIT { empty_strvec }
+#define STRVEC_INIT { \
+	.v = empty_strvec, \
+}
 
 /**
  * Initialize an array. This is no different than assigning from
diff --git a/submodule.c b/submodule.c
index 78aed03d928..03cf36707ae 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1321,9 +1321,11 @@ struct submodule_parallel_fetch {
 
 	struct strbuf submodules_with_errors;
 };
-#define SPF_INIT {0, STRVEC_INIT, NULL, NULL, 0, 0, 0, 0, \
-		  STRING_LIST_INIT_DUP, \
-		  NULL, 0, 0, STRBUF_INIT}
+#define SPF_INIT { \
+	.args = STRVEC_INIT, \
+	.changed_submodule_names = STRING_LIST_INIT_DUP, \
+	.submodules_with_errors = STRBUF_INIT, \
+}
 
 static int get_fetch_recurse_config(const struct submodule *submodule,
 				    struct submodule_parallel_fetch *spf)
diff --git a/submodule.h b/submodule.h
index 35d18c7868a..6bd2c99fd99 100644
--- a/submodule.h
+++ b/submodule.h
@@ -37,7 +37,9 @@ struct submodule_update_strategy {
 	enum submodule_update_type type;
 	const char *command;
 };
-#define SUBMODULE_UPDATE_STRATEGY_INIT {SM_UPDATE_UNSPECIFIED}
+#define SUBMODULE_UPDATE_STRATEGY_INIT { \
+	.type = SM_UPDATE_UNSPECIFIED, \
+}
 
 int is_gitmodules_unmerged(struct index_state *istate);
 int is_writing_gitmodules_ok(void);
diff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c
index 50bb98b7e04..3c4fb862234 100644
--- a/t/helper/test-run-command.c
+++ b/t/helper/test-run-command.c
@@ -60,8 +60,10 @@ struct testsuite {
 	int next;
 	int quiet, immediate, verbose, verbose_log, trace, write_junit_xml;
 };
-#define TESTSUITE_INIT \
-	{ STRING_LIST_INIT_DUP, STRING_LIST_INIT_DUP }
+#define TESTSUITE_INIT { \
+	.tests = STRING_LIST_INIT_DUP, \
+	.failed = STRING_LIST_INIT_DUP, \
+}
 
 static int next_test(struct child_process *cp, struct strbuf *err, void *cb,
 		     void **task_cb)
diff --git a/trace.h b/trace.h
index 74f62919c57..e25984051aa 100644
--- a/trace.h
+++ b/trace.h
@@ -89,7 +89,7 @@ struct trace_key {
 
 extern struct trace_key trace_default_key;
 
-#define TRACE_KEY_INIT(name) { "GIT_TRACE_" #name }
+#define TRACE_KEY_INIT(name) { .key = "GIT_TRACE_" #name }
 extern struct trace_key trace_perf_key;
 extern struct trace_key trace_setup_key;
 
diff --git a/transport.h b/transport.h
index 1cbab113730..8bb4c8bbc8c 100644
--- a/transport.h
+++ b/transport.h
@@ -262,7 +262,9 @@ struct transport_ls_refs_options {
 	 */
 	char *unborn_head_target;
 };
-#define TRANSPORT_LS_REFS_OPTIONS_INIT { STRVEC_INIT }
+#define TRANSPORT_LS_REFS_OPTIONS_INIT { \
+	.ref_prefixes = STRVEC_INIT, \
+}
 
 /*
  * Retrieve refs from a remote.
-- 
2.33.0.1316.gb2e9b3ba3ae

Re: [PATCH v2 4/5] *.h: move some *_INIT to designated initializers

[Junio C Hamano]

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

> -#define SUBMODULE_UPDATE_STRATEGY_INIT {SM_UPDATE_UNSPECIFIED}
> +#define SUBMODULE_UPDATE_STRATEGY_INIT { \
> +	.type = SM_UPDATE_UNSPECIFIED, \
> +}

Much better ;-)

[PATCH v2 5/5] cbtree.h: define cb_init() in terms of CBTREE_INIT

[Ævar Arnfjörð Bjarmason]

Use the same pattern for cb_init() as the one established in the
recent refactoring of other such patterns in
5726a6b4012 (*.c *_init(): define in terms of corresponding *_INIT
macro, 2021-07-01).

It has been pointed out[1] that we could perhaps use this C99
replacement of using a compound literal for all of these:

    *t = (struct cb_tree){ 0 };

But let's just stick to the existing pattern established in
5726a6b4012 for now, we can leave another weather balloon for some
other time.

1. http://lore.kernel.org/git/ef724a3a-a4b8-65d3-c928-13a7d78f189a@gmail.com

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 cbtree.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/cbtree.h b/cbtree.h
index a04a312c3f5..dedbb8e2a45 100644
--- a/cbtree.h
+++ b/cbtree.h
@@ -37,11 +37,12 @@ enum cb_next {
 	CB_BREAK = 1
 };
 
-#define CBTREE_INIT { .root = NULL }
+#define CBTREE_INIT { 0 }
 
 static inline void cb_init(struct cb_tree *t)
 {
-	t->root = NULL;
+	struct cb_tree blank = CBTREE_INIT;
+	memcpy(t, &blank, sizeof(*t));
 }
 
 struct cb_node *cb_lookup(struct cb_tree *, const uint8_t *k, size_t klen);
-- 
2.33.0.1316.gb2e9b3ba3ae