From 6e54a32468e209aa724b37496fcc63f4ebe84f60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Mon, 27 Sep 2021 14:58:41 +0200 Subject: [PATCH 1/5] daemon.c: refactor hostinfo_init() to HOSTINFO_INIT macro MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove the hostinfo_init() function added in 01cec54e135 (daemon: deglobalize hostname information, 2015-03-07) and instead initialize the "struct hostinfo" with a macro. This is the more idiomatic pattern in the codebase, and doesn't leave us wondering when we see the *_init() function if this struct needs more complex initialization than a macro can provide. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- daemon.c | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/daemon.c b/daemon.c index 5c4cbad62d..d80d009d1a 100644 --- a/daemon.c +++ b/daemon.c @@ -63,6 +63,12 @@ struct hostinfo { unsigned int hostname_lookup_done:1; unsigned int saw_extended_args:1; }; +#define HOSTINFO_INIT { \ + .hostname = STRBUF_INIT, \ + .canon_hostname = STRBUF_INIT, \ + .ip_address = STRBUF_INIT, \ + .tcp_port = STRBUF_INIT, \ +} static void lookup_hostname(struct hostinfo *hi); @@ -727,15 +733,6 @@ static void lookup_hostname(struct hostinfo *hi) } } -static void hostinfo_init(struct hostinfo *hi) -{ - memset(hi, 0, sizeof(*hi)); - strbuf_init(&hi->hostname, 0); - strbuf_init(&hi->canon_hostname, 0); - strbuf_init(&hi->ip_address, 0); - strbuf_init(&hi->tcp_port, 0); -} - static void hostinfo_clear(struct hostinfo *hi) { strbuf_release(&hi->hostname); @@ -760,11 +757,9 @@ static int execute(void) char *line = packet_buffer; int pktlen, len, i; char *addr = getenv("REMOTE_ADDR"), *port = getenv("REMOTE_PORT"); - struct hostinfo hi; + struct hostinfo hi = HOSTINFO_INIT; struct strvec env = STRVEC_INIT; - hostinfo_init(&hi); - if (addr) loginfo("Connection from %s:%s", addr, port); From 4eb2bfdc92139f04619be21a354c025747e7ceea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Mon, 27 Sep 2021 14:58:42 +0200 Subject: [PATCH 2/5] builtin/blame.c: refactor commit_info_init() to COMMIT_INFO_INIT macro MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove the commit_info_init() function addded in ea02ffa3857 (mailmap: simplify map_user() interface, 2013-01-05) and instead initialize the "struct commit_info" with a macro. This is the more idiomatic pattern in the codebase, and doesn't leave us wondering when we see the *_init() function if this struct needs more complex initialization than a macro can provide. The get_commit_info() function is only called by the three callers being changed here immediately after initializing the struct with the macros, so by moving the initialization to the callers we don't need to do it in get_commit_info() anymore. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/blame.c | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/builtin/blame.c b/builtin/blame.c index 641523ff9a..1c31a99640 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -101,6 +101,16 @@ struct commit_info { struct strbuf summary; }; +#define COMMIT_INFO_INIT { \ + .author = STRBUF_INIT, \ + .author_mail = STRBUF_INIT, \ + .author_tz = STRBUF_INIT, \ + .committer = STRBUF_INIT, \ + .committer_mail = STRBUF_INIT, \ + .committer_tz = STRBUF_INIT, \ + .summary = STRBUF_INIT, \ +} + /* * Parse author/committer line in the commit object buffer */ @@ -160,18 +170,6 @@ static void get_ac_line(const char *inbuf, const char *what, strbuf_add(name, namebuf, namelen); } -static void commit_info_init(struct commit_info *ci) -{ - - strbuf_init(&ci->author, 0); - strbuf_init(&ci->author_mail, 0); - strbuf_init(&ci->author_tz, 0); - strbuf_init(&ci->committer, 0); - strbuf_init(&ci->committer_mail, 0); - strbuf_init(&ci->committer_tz, 0); - strbuf_init(&ci->summary, 0); -} - static void commit_info_destroy(struct commit_info *ci) { @@ -192,8 +190,6 @@ static void get_commit_info(struct commit *commit, const char *subject, *encoding; const char *message; - commit_info_init(ret); - encoding = get_log_output_encoding(); message = logmsg_reencode(commit, NULL, encoding); get_ac_line(message, "\nauthor ", @@ -246,7 +242,7 @@ static void write_filename_info(struct blame_origin *suspect) */ static int emit_one_suspect_detail(struct blame_origin *suspect, int repeat) { - struct commit_info ci; + struct commit_info ci = COMMIT_INFO_INIT; if (!repeat && (suspect->commit->object.flags & METAINFO_SHOWN)) return 0; @@ -440,7 +436,7 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int int cnt; const char *cp; struct blame_origin *suspect = ent->suspect; - struct commit_info ci; + struct commit_info ci = COMMIT_INFO_INIT; char hex[GIT_MAX_HEXSZ + 1]; int show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP); const char *default_color = NULL, *color = NULL, *reset = NULL; @@ -630,7 +626,7 @@ static void find_alignment(struct blame_scoreboard *sb, int *option) if (longest_file < num) longest_file = num; if (!(suspect->commit->object.flags & METAINFO_SHOWN)) { - struct commit_info ci; + struct commit_info ci = COMMIT_INFO_INIT; suspect->commit->object.flags |= METAINFO_SHOWN; get_commit_info(suspect->commit, &ci, 1); if (*option & OUTPUT_SHOW_EMAIL) From 73ee449bbf2918e29d26361e57f35a24f224e3be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Fri, 1 Oct 2021 12:27:33 +0200 Subject: [PATCH 3/5] urlmatch.[ch]: add and use URLMATCH_CONFIG_INIT MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change the initialization pattern of "struct urlmatch_config" to use an *_INIT macro and designated initializers. Right now there's no other "struct" member of "struct urlmatch_config" which would require its own *_INIT, but it's good practice not to assume that. Let's also change this to a designated initializer while we're at it. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/config.c | 2 +- credential.c | 2 +- http.c | 2 +- urlmatch.h | 4 ++++ 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/builtin/config.c b/builtin/config.c index 865fddd6ce..542d8d02b2 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -575,7 +575,7 @@ static int get_urlmatch(const char *var, const char *url) int ret; char *section_tail; struct string_list_item *item; - struct urlmatch_config config = { STRING_LIST_INIT_DUP }; + struct urlmatch_config config = URLMATCH_CONFIG_INIT; struct string_list values = STRING_LIST_INIT_DUP; config.collect_fn = urlmatch_collect_fn; diff --git a/credential.c b/credential.c index 000ac7a8d4..e7240f3f63 100644 --- a/credential.c +++ b/credential.c @@ -105,7 +105,7 @@ static int match_partial_url(const char *url, void *cb) static void credential_apply_config(struct credential *c) { char *normalized_url; - struct urlmatch_config config = { STRING_LIST_INIT_DUP }; + struct urlmatch_config config = URLMATCH_CONFIG_INIT; struct strbuf url = STRBUF_INIT; if (!c->host) diff --git a/http.c b/http.c index d7c20493d7..da12471c24 100644 --- a/http.c +++ b/http.c @@ -990,7 +990,7 @@ void http_init(struct remote *remote, const char *url, int proactive_auth) char *low_speed_limit; char *low_speed_time; char *normalized_url; - struct urlmatch_config config = { STRING_LIST_INIT_DUP }; + struct urlmatch_config config = URLMATCH_CONFIG_INIT; config.section = "http"; config.key = NULL; diff --git a/urlmatch.h b/urlmatch.h index 6ff42f81b0..34a3ba6d19 100644 --- a/urlmatch.h +++ b/urlmatch.h @@ -66,6 +66,10 @@ struct urlmatch_config { int (*fallback_match_fn)(const char *url, void *cb); }; +#define URLMATCH_CONFIG_INIT { \ + .vars = STRING_LIST_INIT_DUP, \ +} + int urlmatch_config_entry(const char *var, const char *value, void *cb); #endif /* URL_MATCH_H */ From 0bc7787ca9ea123e28769f728d77261cad1f1557 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Fri, 1 Oct 2021 12:27:34 +0200 Subject: [PATCH 4/5] builtin/remote.c: add and use a REF_STATES_INIT MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use a new REF_STATES_INIT designated initializer instead of assigning to the "strdup_strings" member of the previously memzero()'d version of this struct. The pattern of assigning to "strdup_strings" dates back to 211c89682ee (Make git-remote a builtin, 2008-02-29) (when it was "strdup_paths"), i.e. long before we used anything like our current established *_INIT patterns consistently. Then in e61e0cc6b70 (builtin-remote: teach show to display remote HEAD, 2009-02-25) and e5dcbfd9ab7 (builtin-remote: new show output style for push refspecs, 2009-02-25) we added some more of these. As it turns out we only initialized this struct three times, all the other uses were of pointers to those initialized structs. So let's initialize it in those three places, skip the memset(), and pass those structs down appropriately. This would be a behavior change if we had codepaths that relied say on implicitly having had "new_refs" initialized to STRING_LIST_INIT_NODUP with the memset(), but only set the "strdup_strings" on some other struct, but then called string_list_append() on "new_refs". There isn't any such codepath, all of the late assignments to "strdup_strings" assigned to those structs that we'd use for those codepaths. So just initializing them all up-front makes for easier to understand code, i.e. in the pre-image it looked as though we had that tricky edge case, but we didn't. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/remote.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/builtin/remote.c b/builtin/remote.c index 7f88e6ce9d..160dd954f7 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -344,6 +344,14 @@ struct ref_states { int queried; }; +#define REF_STATES_INIT { \ + .new_refs = STRING_LIST_INIT_DUP, \ + .stale = STRING_LIST_INIT_DUP, \ + .tracked = STRING_LIST_INIT_DUP, \ + .heads = STRING_LIST_INIT_DUP, \ + .push = STRING_LIST_INIT_DUP, \ +} + static int get_ref_states(const struct ref *remote_refs, struct ref_states *states) { struct ref *fetch_map = NULL, **tail = &fetch_map; @@ -355,9 +363,6 @@ static int get_ref_states(const struct ref *remote_refs, struct ref_states *stat die(_("Could not get fetch map for refspec %s"), states->remote->fetch.raw[i]); - states->new_refs.strdup_strings = 1; - states->tracked.strdup_strings = 1; - states->stale.strdup_strings = 1; for (ref = fetch_map; ref; ref = ref->next) { if (!ref->peer_ref || !ref_exists(ref->peer_ref->name)) string_list_append(&states->new_refs, abbrev_branch(ref->name)); @@ -406,7 +411,6 @@ static int get_push_ref_states(const struct ref *remote_refs, match_push_refs(local_refs, &push_map, &remote->push, MATCH_REFS_NONE); - states->push.strdup_strings = 1; for (ref = push_map; ref; ref = ref->next) { struct string_list_item *item; struct push_info *info; @@ -449,7 +453,6 @@ static int get_push_ref_states_noquery(struct ref_states *states) if (remote->mirror) return 0; - states->push.strdup_strings = 1; if (!remote->push.nr) { item = string_list_append(&states->push, _("(matching)")); info = item->util = xcalloc(1, sizeof(struct push_info)); @@ -483,7 +486,6 @@ static int get_head_names(const struct ref *remote_refs, struct ref_states *stat refspec.force = 0; refspec.pattern = 1; refspec.src = refspec.dst = "refs/heads/*"; - states->heads.strdup_strings = 1; get_fetch_map(remote_refs, &refspec, &fetch_map_tail, 0); matches = guess_remote_head(find_ref_by_name(remote_refs, "HEAD"), fetch_map, 1); @@ -1212,7 +1214,7 @@ static int show(int argc, const char **argv) OPT_BOOL('n', NULL, &no_query, N_("do not query remotes")), OPT_END() }; - struct ref_states states; + struct ref_states states = REF_STATES_INIT; struct string_list info_list = STRING_LIST_INIT_NODUP; struct show_info info; @@ -1225,7 +1227,6 @@ static int show(int argc, const char **argv) if (!no_query) query_flag = (GET_REF_STATES | GET_HEAD_NAMES | GET_PUSH_REF_STATES); - memset(&states, 0, sizeof(states)); memset(&info, 0, sizeof(info)); info.states = &states; info.list = &info_list; @@ -1334,8 +1335,7 @@ static int set_head(int argc, const char **argv) if (!opt_a && !opt_d && argc == 2) { head_name = xstrdup(argv[1]); } else if (opt_a && !opt_d && argc == 1) { - struct ref_states states; - memset(&states, 0, sizeof(states)); + struct ref_states states = REF_STATES_INIT; get_remote_ref_states(argv[0], &states, GET_HEAD_NAMES); if (!states.heads.nr) result |= error(_("Cannot determine remote HEAD")); @@ -1374,14 +1374,13 @@ static int set_head(int argc, const char **argv) static int prune_remote(const char *remote, int dry_run) { int result = 0; - struct ref_states states; + struct ref_states states = REF_STATES_INIT; struct string_list refs_to_prune = STRING_LIST_INIT_NODUP; struct string_list_item *item; const char *dangling_msg = dry_run ? _(" %s will become dangling!") : _(" %s has become dangling!"); - memset(&states, 0, sizeof(states)); get_remote_ref_states(remote, &states, GET_REF_STATES); if (!states.stale.nr) { From 0000e81811bcbdc44339d03ae772650b98c26ed9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Fri, 1 Oct 2021 12:27:35 +0200 Subject: [PATCH 5/5] builtin/remote.c: add and use SHOW_INFO_INIT MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the preceding commit we introduced REF_STATES_INIT, but did not change the "struct show_info" to have a corresponding initializer. Let's do that, and make it use "REF_STATES_INIT" and "STRING_LIST_INIT_DUP", doing that requires changing "list" and "states" away from being pointers. The resulting end-state is simpler since we omit the local "info_list" and "states" variables in show() as well as the memset(). Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/remote.c | 90 ++++++++++++++++++++++++------------------------ 1 file changed, 45 insertions(+), 45 deletions(-) diff --git a/builtin/remote.c b/builtin/remote.c index 160dd954f7..deb48772ac 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -972,26 +972,31 @@ static int get_remote_ref_states(const char *name, } struct show_info { - struct string_list *list; - struct ref_states *states; + struct string_list list; + struct ref_states states; int width, width2; int any_rebase; }; +#define SHOW_INFO_INIT { \ + .list = STRING_LIST_INIT_DUP, \ + .states = REF_STATES_INIT, \ +} + static int add_remote_to_show_info(struct string_list_item *item, void *cb_data) { struct show_info *info = cb_data; int n = strlen(item->string); if (n > info->width) info->width = n; - string_list_insert(info->list, item->string); + string_list_insert(&info->list, item->string); return 0; } static int show_remote_info_item(struct string_list_item *item, void *cb_data) { struct show_info *info = cb_data; - struct ref_states *states = info->states; + struct ref_states *states = &info->states; const char *name = item->string; if (states->queried) { @@ -1018,7 +1023,7 @@ static int show_remote_info_item(struct string_list_item *item, void *cb_data) static int add_local_to_show_info(struct string_list_item *branch_item, void *cb_data) { struct show_info *show_info = cb_data; - struct ref_states *states = show_info->states; + struct ref_states *states = &show_info->states; struct branch_info *branch_info = branch_item->util; struct string_list_item *item; int n; @@ -1031,7 +1036,7 @@ static int add_local_to_show_info(struct string_list_item *branch_item, void *cb if (branch_info->rebase >= REBASE_TRUE) show_info->any_rebase = 1; - item = string_list_insert(show_info->list, branch_item->string); + item = string_list_insert(&show_info->list, branch_item->string); item->util = branch_info; return 0; @@ -1086,7 +1091,7 @@ static int add_push_to_show_info(struct string_list_item *push_item, void *cb_da show_info->width = n; if ((n = strlen(push_info->dest)) > show_info->width2) show_info->width2 = n; - item = string_list_append(show_info->list, push_item->string); + item = string_list_append(&show_info->list, push_item->string); item->util = push_item->util; return 0; } @@ -1214,9 +1219,7 @@ static int show(int argc, const char **argv) OPT_BOOL('n', NULL, &no_query, N_("do not query remotes")), OPT_END() }; - struct ref_states states = REF_STATES_INIT; - struct string_list info_list = STRING_LIST_INIT_NODUP; - struct show_info info; + struct show_info info = SHOW_INFO_INIT; argc = parse_options(argc, argv, NULL, options, builtin_remote_show_usage, 0); @@ -1227,25 +1230,22 @@ static int show(int argc, const char **argv) if (!no_query) query_flag = (GET_REF_STATES | GET_HEAD_NAMES | GET_PUSH_REF_STATES); - memset(&info, 0, sizeof(info)); - info.states = &states; - info.list = &info_list; for (; argc; argc--, argv++) { int i; const char **url; int url_nr; - get_remote_ref_states(*argv, &states, query_flag); + get_remote_ref_states(*argv, &info.states, query_flag); printf_ln(_("* remote %s"), *argv); - printf_ln(_(" Fetch URL: %s"), states.remote->url_nr > 0 ? - states.remote->url[0] : _("(no URL)")); - if (states.remote->pushurl_nr) { - url = states.remote->pushurl; - url_nr = states.remote->pushurl_nr; + printf_ln(_(" Fetch URL: %s"), info.states.remote->url_nr > 0 ? + info.states.remote->url[0] : _("(no URL)")); + if (info.states.remote->pushurl_nr) { + url = info.states.remote->pushurl; + url_nr = info.states.remote->pushurl_nr; } else { - url = states.remote->url; - url_nr = states.remote->url_nr; + url = info.states.remote->url; + url_nr = info.states.remote->url_nr; } for (i = 0; i < url_nr; i++) /* @@ -1258,57 +1258,57 @@ static int show(int argc, const char **argv) printf_ln(_(" Push URL: %s"), _("(no URL)")); if (no_query) printf_ln(_(" HEAD branch: %s"), _("(not queried)")); - else if (!states.heads.nr) + else if (!info.states.heads.nr) printf_ln(_(" HEAD branch: %s"), _("(unknown)")); - else if (states.heads.nr == 1) - printf_ln(_(" HEAD branch: %s"), states.heads.items[0].string); + else if (info.states.heads.nr == 1) + printf_ln(_(" HEAD branch: %s"), info.states.heads.items[0].string); else { printf(_(" HEAD branch (remote HEAD is ambiguous," " may be one of the following):\n")); - for (i = 0; i < states.heads.nr; i++) - printf(" %s\n", states.heads.items[i].string); + for (i = 0; i < info.states.heads.nr; i++) + printf(" %s\n", info.states.heads.items[i].string); } /* remote branch info */ info.width = 0; - for_each_string_list(&states.new_refs, add_remote_to_show_info, &info); - for_each_string_list(&states.tracked, add_remote_to_show_info, &info); - for_each_string_list(&states.stale, add_remote_to_show_info, &info); - if (info.list->nr) + for_each_string_list(&info.states.new_refs, add_remote_to_show_info, &info); + for_each_string_list(&info.states.tracked, add_remote_to_show_info, &info); + for_each_string_list(&info.states.stale, add_remote_to_show_info, &info); + if (info.list.nr) printf_ln(Q_(" Remote branch:%s", " Remote branches:%s", - info.list->nr), + info.list.nr), no_query ? _(" (status not queried)") : ""); - for_each_string_list(info.list, show_remote_info_item, &info); - string_list_clear(info.list, 0); + for_each_string_list(&info.list, show_remote_info_item, &info); + string_list_clear(&info.list, 0); /* git pull info */ info.width = 0; info.any_rebase = 0; for_each_string_list(&branch_list, add_local_to_show_info, &info); - if (info.list->nr) + if (info.list.nr) printf_ln(Q_(" Local branch configured for 'git pull':", " Local branches configured for 'git pull':", - info.list->nr)); - for_each_string_list(info.list, show_local_info_item, &info); - string_list_clear(info.list, 0); + info.list.nr)); + for_each_string_list(&info.list, show_local_info_item, &info); + string_list_clear(&info.list, 0); /* git push info */ - if (states.remote->mirror) + if (info.states.remote->mirror) printf_ln(_(" Local refs will be mirrored by 'git push'")); info.width = info.width2 = 0; - for_each_string_list(&states.push, add_push_to_show_info, &info); - QSORT(info.list->items, info.list->nr, cmp_string_with_push); - if (info.list->nr) + for_each_string_list(&info.states.push, add_push_to_show_info, &info); + QSORT(info.list.items, info.list.nr, cmp_string_with_push); + if (info.list.nr) printf_ln(Q_(" Local ref configured for 'git push'%s:", " Local refs configured for 'git push'%s:", - info.list->nr), + info.list.nr), no_query ? _(" (status not queried)") : ""); - for_each_string_list(info.list, show_push_info_item, &info); - string_list_clear(info.list, 0); + for_each_string_list(&info.list, show_push_info_item, &info); + string_list_clear(&info.list, 0); - free_remote_ref_states(&states); + free_remote_ref_states(&info.states); } return result;