threads / patch / 56590

patch, 5 partsDesignated initializer cleanup & conversion

Subject: [PATCH 0/5] Designated initializer cleanup & conversion

## tl;dr

32 messages between Sep 27, 2021 and Sep 30, 2021. Diffs are folded; open one to read it.

replies: 31people: 7as markdown or json

Ævar Arnfjörð Bjarmason· Sep 27, 2021, 00:39 UTC · lore

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
Ævar Arnfjörð Bjarmason· Sep 27, 2021, 00:39 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

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(-)
Show changes to 14 files +22 −25

builtin/submodule--helper.c, checkout.c, contrib/credential/gnome-keyring/git-credential-gnome-keyring.c, contrib/credential/libsecret/git-credential-libsecret.c, diff.c, entry.h, lockfile.h, object-store.h, object.h, oid-array.h, path.h, ref-filter.c, remote.c, revision.c

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
Eric Sunshine· Sep 27, 2021, 02:27 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

On Sun, Sep 26, 2021 at 8:40 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Show 5 quoted lines
> 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/
Show 7 quoted lines
> 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>
Johannes Sixt· Sep 27, 2021, 06:35 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

Am 27.09.21 um 02:39 schrieb Ævar Arnfjörð Bjarmason:
Show 10 quoted lines
> 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
Junio C Hamano· Sep 27, 2021, 20:25 UTC · re: Johannes Sixt · lore

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

Johannes Sixt <j6t@kdbg.org> writes:
Show 13 quoted lines
> 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.

Ævar Arnfjörð Bjarmason· Sep 27, 2021, 00:39 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

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(-)
Show changes to submodule-config.h +0 −4
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
Ævar Arnfjörð Bjarmason· Sep 27, 2021, 00:39 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

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(-)
Show changes to 3 files +3 −7

simple-ipc.h, strbuf.h, t/helper/test-run-command.c

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
Ævar Arnfjörð Bjarmason· Sep 27, 2021, 00:39 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

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(-)
Show changes to cbtree.h +3 −2
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
Johannes Sixt· Sep 27, 2021, 06:37 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

Am 27.09.21 um 02:39 schrieb Ævar Arnfjörð Bjarmason:
Show 13 quoted lines
> --- 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
Ævar Arnfjörð Bjarmason· Sep 27, 2021, 11:02 UTC · re: Johannes Sixt · lore

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

On Mon, Sep 27 2021, Johannes Sixt wrote:
Show 18 quoted lines
> 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*...
Show 8 quoted lines
>> +	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/
Jeff King· Sep 27, 2021, 23:54 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

On Mon, Sep 27, 2021 at 01:02:35PM +0200, Ævar Arnfjörð Bjarmason wrote:
Show 15 quoted lines
> >>  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).

Show 6 quoted lines
> >> +	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
Johannes Sixt· Sep 28, 2021, 06:15 UTC · re: Jeff King · lore

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

Am 28.09.21 um 01:54 schrieb Jeff King:
Show 22 quoted lines
> 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
Junio C Hamano· Sep 28, 2021, 18:32 UTC · re: Jeff King · lore

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

Jeff King <peff@peff.net> writes:
Show 10 quoted lines
>> >> +	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.
Ævar Arnfjörð Bjarmason· Sep 28, 2021, 19:42 UTC · re: Junio C Hamano · lore

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

On Tue, Sep 28 2021, Junio C Hamano wrote:
Show 22 quoted lines
> 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...

Junio C Hamano· Sep 28, 2021, 20:50 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
Show 28 quoted lines
> 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.
Phillip Wood· Sep 27, 2021, 09:13 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

Hi Ævar
On 27/09/2021 01:39, Ævar Arnfjörð Bjarmason wrote:
Show 27 quoted lines
> 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);
> 
Ævar Arnfjörð Bjarmason· Sep 27, 2021, 11:00 UTC · re: Phillip Wood · lore

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

On Mon, Sep 27 2021, Phillip Wood wrote:
Show 34 quoted lines
> 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.

Phillip Wood· Sep 30, 2021, 10:01 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

On 27/09/2021 12:00, Ævar Arnfjörð Bjarmason wrote:
Show 40 quoted lines
> 
> 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
Ævar Arnfjörð Bjarmason· Sep 27, 2021, 00:39 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

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(-)
Show changes to 10 files +36 −15

add-interactive.c, builtin/submodule--helper.c, cache.h, sequencer.h, shallow.h, strvec.h, submodule.c, submodule.h, t/helper/test-run-command.c, transport.h

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
Ævar Arnfjörð Bjarmason· Sep 27, 2021, 12:54 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

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
Ævar Arnfjörð Bjarmason· Sep 27, 2021, 12:54 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

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(-)
Show changes to submodule-config.h +0 −4
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
Jeff King· Sep 27, 2021, 23:34 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

On Mon, Sep 27, 2021 at 02:54:24PM +0200, Ævar Arnfjörð Bjarmason wrote:
Show 20 quoted lines
> 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
Ævar Arnfjörð Bjarmason· Sep 27, 2021, 12:54 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

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(-)
Show changes to 13 files +21 −24

builtin/submodule--helper.c, checkout.c, contrib/credential/gnome-keyring/git-credential-gnome-keyring.c, contrib/credential/libsecret/git-credential-libsecret.c, diff.c, lockfile.h, object-store.h, object.h, oid-array.h, path.h, ref-filter.c, remote.c, revision.c

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
Jeff King· Sep 27, 2021, 23:24 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

On Mon, Sep 27, 2021 at 02:54:25PM +0200, Ævar Arnfjörð Bjarmason wrote:
Show 10 quoted lines
> 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
Ævar Arnfjörð Bjarmason· Sep 28, 2021, 00:25 UTC · re: Jeff King · lore

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

On Mon, Sep 27 2021, Jeff King wrote:
Show 26 quoted lines
> 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.

Jeff King· Sep 28, 2021, 00:46 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

On Tue, Sep 28, 2021 at 02:25:16AM +0200, Ævar Arnfjörð Bjarmason wrote:
Show 19 quoted lines
> 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
Ramsay Jones· Sep 28, 2021, 01:44 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

On 28/09/2021 01:25, Ævar Arnfjörð Bjarmason wrote:
Show 51 quoted lines
> 
> 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

Ævar Arnfjörð Bjarmason· Sep 27, 2021, 12:54 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

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(-)
Show changes to 7 files +7 −11

cache.h, simple-ipc.h, strbuf.h, strvec.h, submodule.h, t/helper/test-run-command.c, trace.h

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
Junio C Hamano· Sep 27, 2021, 21:54 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
Show 6 quoted lines
> -#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.
Ævar Arnfjörð Bjarmason· Sep 27, 2021, 12:54 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

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(-)
Show changes to 13 files +42 −18

add-interactive.c, builtin/submodule--helper.c, cache.h, entry.h, list.h, sequencer.h, shallow.h, strvec.h, submodule.c, submodule.h, t/helper/test-run-command.c, trace.h, transport.h

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
Junio C Hamano· Sep 27, 2021, 23:57 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

Æ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 ;-)
Ævar Arnfjörð Bjarmason· Sep 27, 2021, 12:54 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

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(-)
Show changes to cbtree.h +3 −2
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

← back to recent threads